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
Issue 14621, 14624, 14625 - Fix bugs in array operator overloading semantics #4686
Conversation
@@ -64,6 +66,8 @@ Expression *resolveAliasThis(Scope *sc, Expression *e) | |||
e = e->semantic(sc); | |||
} | |||
e = resolveProperties(sc, e); | |||
|
|||
e = (gag && global.endGagging(olderrors)) ? NULL : e; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (gag && global.endGagging(olderrors))
e = NULL;
would be nicer IMO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, updated.
851037d
to
99760f3
Compare
To @WalterBright and other committers: Please review this to fix regression Issue 14621, toward 2.068-beta2 release. |
I really don't like the gag technique, even though I came up with it, it's been nothing but trouble. There's got to be a better way, and there is - the TOKerror method. Also, at this point, this looks like a very large and disruptive change for this late in the beta cycle, especially since this regression was introduced well over a year ago. |
Did you decide not to fix issue 14621 in 2.068 release? |
…ng fallback Points: - Shorten the length of problematic gagged semantic path. - Improve ArrayExp as the entry point of array operator overloading. - Reduce stack size and the number of heap-allocated AST nodes for recursive semantic analysis. Also fixes unlisted wrong-code issue. See test20x() in runnable/opover2.d
…bal.errors' on line 752 in file 'statement.c'
Right, I think this fix is risky to insert into the 2.068 beta, not worth it because it's an old issue and hasn't caused much problems. |
Ok, make sense. |
If it's an old issue, please downgrade the bug from regression to something else. |
It's not so old. The gagging issue is since 2.066, that introduced when the new operator overloading feature is added. |
This PR has been moved to #4948, for the transplanting to stable branch. |
Issue 14621 - [REG2.066] ICE: Assertion failure: 'global.gaggedErrors || global.errors' on line 752 in file 'statement.c'
Issue 14624 - The array operator overloading fallback is not correct
Issue 14625 - opIndex() doesn't work on foreach container iteration
Shorten the length of gagged semantic path, and improve
ArrayExp
as the entry point of array operator overloading.