"panic: runtime error: invalid memory address or nil pointer dereference" on `Subshell()` call

[zikaeroh]

Reproduction: https://play.golang.org/p/XOOqXzF6VGV

package main

import (
    "mvdan.cc/sh/v3/interp"
)

func main() {
    runner, err := interp.New()
    if err != nil {
        panic(err)
    }
    runner.Subshell()
}

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x52697f]

goroutine 1 [running]:
mvdan.cc/sh/v3/interp.(*Runner).Subshell(0xc000196000, 0x0)
    /tmp/gopath238957503/pkg/mod/mvdan.cc/sh/v3@v3.3.0/interp/api.go:570 +0x27f
main.main()
    /tmp/sandbox184729568/prog.go:12 +0x52

Hit this in a benchmark of embedded languages; I call Subshell to obtain clones of the original runner.

[mvdan]

Thanks - it looks like the method assumes that Run has been called at least once, which isn't documented that way. I'll fix that.

It's a bit of an edge case to call Subshell on a runner that hasn't been used for anything, though. It's practically equivalent to just creating a new runner.

[zikaeroh]

Thanks for the fix.

In my case, I found that calling subshell on the unused runner was twice as fast as making a new runner each time with the same setup; this change unfortunately appears to remove most of that performance benefit (though not all of it).

[mvdan]

The syntax parser and printer were pretty heavily optimized years ago, but the runner has only had fairly basic work done. For a while it didn't make sense to heavily optimize the code given that it lacked features and would likely be refactored, but we're at a point now where it probably makes sense.

If you have any suggestions in mind, or a benchmark/pprof that points at a culprit, I'm happy to look into it.