Temporary assignment to nonexistent variable leaves it equal to ''

[tw4452852]

~/go/src/github.com/elves/elvish> put $y
Compilation error: variable $y not found
  [interactive], line 1:
    put $y
~/go/src/github.com/elves/elvish> y=1 echo hi
hi
~/go/src/github.com/elves/elvish> put $y
▶ ''
~/go/src/github.com/elves/elvish>       

Is this a known issue? I think y should be undefined as before.

[zzamboni]

I can confirm this bug. If the variable already had a value (like in the example in the docs), then the old value is preserved correctly after the temporary assignment, but if the variable was undefined, then it gets set to an empty string after the temporary assignment.

[xiaq]

Yup, it's a known bug. Whoever implemented it must have been too lazy to write correct code :(

[tw4452852]

@xiaq
I think the culprit is this:

elvish/eval/compile_op.go

Lines 246 to 251 in 4df00b0

	if val == nil {
	// XXX Old value is nonexistent. We should delete the
	// variable. However, since the compiler now doesn't delete
	// it, we don't delete it in the evaler either.
	val = String("")
	}

Is there any reason behind this? Maybe we should just skip nil value.

[krader1961]

It seems to me that the core challenge in fixing this is modifying the compiler to recognize that a sequence such as this:

x=y put $x
put $x

Should result in this compilation error:

compilation error: variable $x not found
/Users/krader/x.elv, line 2: put $x
Exception: elvish exited with 2

Note that I created that error by commenting out the first line. From the compiler's perspective the temporary assignment should be equivalent to commenting out the first line. The fix for this issue is trivial if you don't care that the compiler does not recognize $x does not exist after the temporary assignment.

The borderline trivial, incomplete, fix would mean that dereferencing an undefined var is caught at run-time rather than compile-time. Is that good enough? My knowledge of the Elvish compiler is almost zero so it's possible the fully correct fix is also borderline trivial.

[krader1961]

FYI, Anyone tempted to work on this should read the feedback in P.R. #802.

@xiaq, If you've started working on a fix for this issue it would be a good idea to make note of that fact so no one wastes time taking a stab at the problem.

[xiaq]

This will no longer be irrelevant when the temp assignment form gets deprecated and eventually removed (#1114). The tmp command requires the variable to exist.