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

Improve inline constructor handling of 'if', 'try' and 'while' expressions. #11356

Merged
merged 1 commit into from Nov 7, 2023

Conversation

basro
Copy link
Contributor

@basro basro commented Nov 4, 2023

This PR adds special cases for if try and while expressions in the constructor inliner logic allowing for the body of those expressions to result in an inlined object with the condition that the expressions themselves are ignored (not assigned to a variable or passed as argument to some other function)

Example of code that used to cancel inlining but now works:

class ExternInlineClass {
	public var a = 1;
	public extern inline function new() {
	}
}

class Test {
	static var condition = false;
	static function main() {
		var a = new ExternInlineClass();

		if ( condition ) a; else a;

		while ( condition ) a;

		try { a; } catch(_) { a; };
		
		return a.a;
	}
}

A scenario where you'd encounter this problem is when you have a function that does some side effect and then returns an inline object. Even if the inline object was not used, just by being present at the end of an if or while block it would cause the inlining to cancel.

if (condition) doSomethingAndReturnInlinedObject();

This could be worked around by changing the body of the if to result in a noop:

if (condition) {doSomethingAndReturnInlinedObject(); null; }

With this PR that work around is no longer needed.

@Simn
Copy link
Member

Simn commented Nov 7, 2023

Please rebase to make the mac build failures go away!

This commit changes the captured argument of analyze_aliases from a
boolean into a three value enum.
What used to be captured = false is now IEHNotHandled.
What used to be captured = true is now split between IEHCaptured and
IEHIgnored.
Knowing that the value will be ignored allows if and try expressions to
not cancel the inlining of their bodies.
@basro basro force-pushed the inline-ctors-ignored-expressions branch from 07f4791 to 4592f41 Compare November 7, 2023 11:33
@Simn Simn merged commit 01dde74 into development Nov 7, 2023
122 checks passed
@Simn Simn deleted the inline-ctors-ignored-expressions branch November 7, 2023 12:30
@skial skial mentioned this pull request Nov 13, 2023
1 task
Simn added a commit that referenced this pull request Nov 19, 2023
@Simn
Copy link
Member

Simn commented Nov 19, 2023

@basro Please check if the linked commit makes sense to you! It seemed wrong to consider IEHIgnored as captured = true, so I flipped that. See the added test case for how it went wrong.

Edit: Looking at the example again, this might not be correct... My thinking was that this doesn't fail without the inline constructor algorithm, but that's only because the analyzer eliminates block-level array declarations and replaces them with their individual values. It could be argued that this should fail either way, so yeah, please take a look some time and feel free to revert my change!

0b1kn00b pushed a commit to 0b1kn00b/haxe that referenced this pull request Jan 25, 2024
This commit changes the captured argument of analyze_aliases from a
boolean into a three value enum.
What used to be captured = false is now IEHNotHandled.
What used to be captured = true is now split between IEHCaptured and
IEHIgnored.
Knowing that the value will be ignored allows if and try expressions to
not cancel the inlining of their bodies.
0b1kn00b pushed a commit to 0b1kn00b/haxe that referenced this pull request Jan 25, 2024
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.

None yet

2 participants