Fatal error when using `itN` resulting from a bind

[byorgey]

Describe the bug Sometimes, using one of the auto-generated itN variables can trigger a fatal "bad application of execConst" error, because the itN contains a VResult which the execConst code is not expecting to see.

To Reproduce

• Start a creative game.
• create "tree"
• def mk = r <- build {}; return r end
• mk
• give it0 "tree"

Expected behavior This ought to work. Probably we just need to strip VResult before storing the value of an itN variable.

Screenshots

[byorgey]

I have a PR with a fix (#978), but overall I'm not happy with the way we handle VResult. It is a fertile source of bugs (#977 , #858, #327, #328) and will probably continue to be so because of some subtle invariants. It's necessary internally in order to pass along intermediate results that also carry an environment of bound names, but there are also many places where we don't expect to see it and don't want to have to worry about dealing with it. A few options for improving the situation that come to mind:

1. Remove VResult from Value, and create a wrapper type for "values with environment" (EValue?) which can be either a Value or a VResult, and carefully decide which instances of the Value type should change to EValue.
2. Add a type parameter to Value that indicates whether it is allowed to contain VResult or not.
3. Get rid of VResult somehow??
4. Something else???

[xsebek]

@byorgey I like option 1) because it will force us to use the values correctly in all places.

[byorgey]

@xsebek What I had in mind for option 2) should do that as well, though I guess I did not explain it well. I had in mind a GADT where the type parameter actually enforced whether there could be VResult, and then we have to choose which type parameter to use where, just like with option 1) we have to choose which type to use where. However, option 1) definitely has simplicity going for it.