Bytecode to AST cache is broken with hoisted conflicting variables

[guillep]

When two hoisted variables (to:do:, ifNotNil: ...) reuse the same variable name, the bytecode to ast cache seems to be broken.

For example, consider the following method:

AdditionalMethodState >> copyWithout: aPropertyOrPragma "<Association|Pragma>"
	"Answer a copy of the receiver which no longer includes aPropertyOrPragma"
	| bs copy offset |
	"no need to initialize here; we're copying all inst vars"
	copy := self class basicNew: (bs := self basicSize) - ((self includes: aPropertyOrPragma)
															ifTrue: [1]
															ifFalse: [0]).
	offset := 0.
	1 to: bs do:
		[:i|
		(self basicAt: i) = aPropertyOrPragma
			ifTrue: [offset := 1]
			ifFalse: [copy basicAt: i - offset put: (self basicAt: i)]].
	1 to: self class instSize do:
		[:i| copy instVarAt: i put: (self instVarAt: i)].
	^copy

Then debug that method with the following statement:

ms := AdditionalMethodState new
	copyWith: 17 -> 25.
ms properties.
ms copyWithout: ms properties associations anyOne.

When you arrive inside the second loop, the debugger shows a renaming of the i variable AND the selection is shifted.

Some extra details

We tracked down the issue to an incorrect bytecode to AST cache

(AdditionalMethodState >> #copyWithout:) ast bcToASTCache nodeForPC: 186
	"OCMessageNode(copy instVarAt: i_587955712 put: (self instVarAt: i_587955712))"

Looks like some internal compiler transformation (renaming of hoisted variables to avoid clashing) creeps up to the AST and thus to the bytecode to AST cache.

Scope of the issue

We have seen this issue appear when in cases like:

• Same ifNotNil: argument

a ifNotNil: [:b | ... ].
...
a ifNotNil: [:b | ... ].

• Same to:do: argument

a to: x do: [:b | ... ].
...
a to: x do: [:b | ... ].

• Variables defined in the scope of an optimized closure, even in conditionals

a ifNil: [ | conflict | ... ].
...
a ifTrue: [ | conflict | ... ].
...
1 to: 19 do: [ :conflict | ... ].

• Affects also shadowed variables

testInlineBlockShadowVariableAssignedInside
	| a |
	a := 23.
	1 to: 17 do: [ :i | | a |
		a := 7 ].
	self assert: a equals: 23

To get the entire list of miscompiled/mapped methods

problems := OrderedCollection new.
Smalltalk allClassesDo: [ :c |
	problems addAll: (c methods select: [ :e | 
		[e debugInfo asOfflineDebugInfo. false]
			on: Error do: [ true ] ]) ].
problems