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

[js] analyzer-optimize can re-order api (extern) calls what can cause a mess #9943

Closed
Klug76 opened this issue Nov 7, 2020 · 4 comments · Fixed by #10432
Closed

[js] analyzer-optimize can re-order api (extern) calls what can cause a mess #9943

Klug76 opened this issue Nov 7, 2020 · 4 comments · Fixed by #10432
Labels
feature-analyzer Static analyzer

Comments

@Klug76
Copy link

Klug76 commented Nov 7, 2020

Haxe ver: 4.1.4, 3.4.4
Sample code:
https://try.haxe.org/index.php#8888E

class Test {
    static function main() {
                var document = js.Browser.window.document;
		var e_div_ = document.createElement("div");
		e_div_.style.position = "absolute";
		e_div_.style.left = "0";
		e_div_.style.top = "0";
		e_div_.style.width = "512px";
		e_div_.style.height = "256px";
		e_div_.style.visibility = "hidden";
		e_div_.style.overflow = 'hidden';
		var e_span_ = document.createElement("span");
		e_span_.innerText = "|Yy";
		var e_block_ = document.createElement("div");
		e_block_.style.width = "1px";
		e_block_.style.height = "0";
		e_block_.style.display = "inline-block";
		e_div_.appendChild(e_span_);
		e_div_.appendChild(e_block_);
                document.body.appendChild(e_div_);
		e_block_.style.verticalAlign = "baseline";
		var a = (e_block_.offsetTop - e_span_.offsetTop);
		e_block_.style.verticalAlign = "bottom";
		var h = (e_block_.offsetTop - e_span_.offsetTop);
		e_block_.style.verticalAlign = "middle";
		var m = (e_block_.offsetTop - e_span_.offsetTop);
                trace('a=$a, h=$h, m=$m');
    }
}

Bad result with analyzer-optimize:
a=10, h=10, m=10
Part of js code:

	e_block_.style.verticalAlign = "baseline";
	e_block_.style.verticalAlign = "bottom";
	e_block_.style.verticalAlign = "middle";
	console.log("a=" + (e_block_.offsetTop - e_span_.offsetTop) + ", h=" + (e_block_.offsetTop - e_span_.offsetTop) + ", m=" + (e_block_.offsetTop - e_span_.offsetTop));

Good result w/o analyzer-optimize:
a=14, h=18, m=10
Part of js code:

	e_block_.style.verticalAlign = "baseline";
	var a = e_block_.offsetTop - e_span_.offsetTop;
	e_block_.style.verticalAlign = "bottom";
	var h = e_block_.offsetTop - e_span_.offsetTop;
	e_block_.style.verticalAlign = "middle";
	var m = e_block_.offsetTop - e_span_.offsetTop;
	console.log("a=" + a + ", h=" + h + ", m=" + m);
@RealyUniqueName
Copy link
Member

This happens because both style and it's properties are declared as physical fields without getters/setters. Analyzer assumes such fields as free of side effects.

I think we need to support @:pure(false) on physical fields for such cases.

@RealyUniqueName RealyUniqueName added this to the Bugs milestone Nov 9, 2020
@RealyUniqueName RealyUniqueName added the feature-analyzer Static analyzer label Nov 9, 2020
@nadako
Copy link
Member

nadako commented Nov 9, 2020

I think we need to support @:pure(false) on physical fields for such cases.

Maybe even with support for separate get and set purity setting, because also in DOM API there is some "interesting" tricks to trigger layout recalculation by reading some specific fields.

@Simn
Copy link
Member

Simn commented Nov 13, 2020

Not sure how to express that though. I think it's fair to be pessimistic in such cases and just consider both access kinds impure.

@AdrianV
Copy link
Contributor

AdrianV commented Nov 13, 2020

IMHO access to external fields should always considered as impure by default.

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

Successfully merging a pull request may close this issue.

5 participants