disable do{...}while(false) optimisation#1534
Conversation
| } | ||
| } else { | ||
| // self instanceof AST_Do | ||
| return self.body; |
There was a problem hiding this comment.
I prefer being more explicit and keeping this else and have it return self;. At the very least it avoids the if below.
There was a problem hiding this comment.
But removing those lines would be a clean revert. That return make_node(AST_For, self, self).transform(compressor); part from down below has always been there, just tucked away in OPT(AST_While) instead of here instead. So adding return self; would actually alter the flow.
See also #1534 (comment)
There was a problem hiding this comment.
How would } else { /* self instanceof AST_Do */ return self; } alter the flow? The following if (self instanceof AST_While) would not be triggered and then return self; would be called.
There was a problem hiding this comment.
... good point. Sorry for being thick tonight 😅
There was a problem hiding this comment.
If you want to leave this as-is, that's fine. I was trying to avoid an if for do-whiles but the cost is insignificant.
There was a problem hiding this comment.
Too late, I've done it your way 👻
|
This would fix the do-while case, but how much confidence do you have in the |
|
I was referring to: #1452 (comment) If you think it's okay, no problem. |
|
Ah, I see - sorry for the misunderstanding. I can just add that line back in if that makes everybody warm and fuzzy 😉 |
|
|
I think we should either revert #1452 entirely or just fix the do-while-false case. If you are confident that |
I was when I looked at it, and currently I'm certain it does not cause #1532. Will leave it out of this PR then. |
- fails to handle `break` in body fixes mishoo#1532
|
So this ends up being an one-liner (excluding the new tests obviously) |
|
@kzc btw, I just checked my email and I noticed this:
Did I miss that or did you delete it before I had a chance to read? 😅 |
|
Nothing gets passed you. :-) I deleted it when I realized github doesn't support that. I was thinking of a pre-git source control system I was using. |
fixes #1532
/cc @kzc