Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong source for CompiledBlocks due to incorrect bytecode to AST nodes mapping #16495

Open
chisandrei opened this issue Apr 19, 2024 · 1 comment · May be fixed by #16664
Open

Wrong source for CompiledBlocks due to incorrect bytecode to AST nodes mapping #16495

chisandrei opened this issue Apr 19, 2024 · 1 comment · May be fixed by #16664

Comments

@chisandrei
Copy link
Contributor

In this method all blocks seem to be printed wrongly:

image

This code is wrong as the sourceNode for a CompiledBloc resolves to a wrong node. We got several methods like the one below in Pharo 11 where the source code of all the blocks is wrong. Recompiling the method fixes the issue, and all blocks have the right source code, but not sure how initially it got in that state.

This was encountered in the latest Pharo 11 + VW on Windows 11.

In the above case this is the bytecode for the method and block:

Screenshot 2024-04-19 at 20 55 15

Also this is the bytecode to AST nodes mapping:

Screenshot 2024-04-19 at 21 00 28

This is after recompiling the method:

Screenshot 2024-04-19 at 21 03 36 Screenshot 2024-04-19 at 21 04 55 Screenshot 2024-04-19 at 21 05 17

Not sure if related, but we seem to be getting also wrong sources for blocks in Pharo-12.0.0+SNAPSHOT.build.1488.sha.c55c586763b889b5e3b0c2749a987fac5ae1f7a7 (64 Bit) in some cases.
For example executing the first time this code in Pharo 12 returns the wrong result

blocks := OrderedCollection  new.
(AbsolutePath>>#asZnUrl) innerCompiledBlocksDo: [ :aBlock | 
		blocks add: aBlock ].
	
blocks first sourceNode sourceCode.
Screenshot 2024-04-19 at 21 11 50 Screenshot 2024-04-19 at 21 14 23

Executing the code the second time returns the correct result:

Screenshot 2024-04-19 at 21 13 24 Screenshot 2024-04-19 at 21 13 50
@carolahp
Copy link
Contributor

The problem is caused when the method containing the block references a variable that was undeclared at the time of its compilation. The image below shows that blocks first outerCode returns an outdated version of the method's code, where the bytecode 67 (send:runtimeUndeclaredReadInContext:) is stacked.
image

A solution would be to recompile all the methods referencing a given UndeclaredVariable when the variable is no longer Undeclared, and do the same when a Global is deleted and left as Undeclared.

Additionally, we should add some tests to ReleaseTest to ensure that a fresh Pharo image does not contain methods with this problem.

@MarcusDenker do you think this solution is heading in the right direction?

tesonep added a commit to tesonep/pharo that referenced this issue May 16, 2024
When removing an undeclared variable we recompile the methods that are accessing through a message send.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants