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

fix: move return to inside TryCatchRegion, if possible #560

Merged
merged 2 commits into from
Apr 8, 2019
Merged

fix: move return to inside TryCatchRegion, if possible #560

merged 2 commits into from
Apr 8, 2019

Conversation

asashour
Copy link
Contributor

@asashour asashour commented Apr 4, 2019

No description provided.

@skylot
Copy link
Owner

skylot commented Apr 4, 2019

Sorry, but to resolve this issue we need to fix the root cause and in this case, it is in CodeShrinkVisitor.
The purpose of a CodeShrinkVisitor is to inline variables with one-time usage, but for doing this there are a lot of checks like: we can't reorder insns with side-effects (invoke for example). And in your test, there is such invoke instruction from finally block which incorrectly prevents inlining because it moved and marked as DONT_GENERATE and CodeShrinkVisitor don't handle this case well (inline canceled here)
CFG of the test method (problem insn in block 3):
cfg-560

To fix that we can change canReorder method in InsnNode like this:

	public boolean canReorder() {
		if (contains(AFlag.DONT_GENERATE)) {
			if (getType() == InsnType.MONITOR_EXIT) {
				return false;
			}
			return true;
		}
		switch (getType()) {
...

@asashour
Copy link
Contributor Author

asashour commented Apr 4, 2019

Very nice, it is a complex logic, but I learn gradually, thanks a lot for your help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants