make MCS work when left-hand operand is erroneous #1257
Conversation
Eagerly updating the input AST in `builtinFieldAccess` prevents `x.y` succeeding as `y(x)` when `x` is an error. The input AST is now only modified (ideally it shouldn't be at all) when the LHS is not an error, meaning that `y(x)` gets the untyped `x` when `x` turned out to be an error. In addition, `i` is renamed to `rhs`, making what it represents more clear.
`semOverloadedCall` expected the first operand of a call that was originally a dot expression to be typed, but this is now not always the case anymore.
It's gone now. In general, such limitations shouldn't be specified in the manual -- they're bugs!
zerbina
added
bug
| discard | ||
|
|
||
| # must not result in an "undeclared identifier" error | ||
| nonexistentName.f |
There was a problem hiding this comment.
This is susceptible to double evaluation of compile time effects? (static: echo "hi"; nonexistentName).f
Longer term with both untyped overloads being analysed first and perhaps restricting compile time effects/making them transactional maybe that's not an issue? There might be precedence issues with the former. 🤔
There was a problem hiding this comment.
Yep, it is susceptible to the double evaluation, as well as leakage of symbol table changes ((var x = 0; nonexistentName).f).
I figured that, while not ideal, this is an improvement over now, where (static: ...; nonexistentName).f doesn't compile at all.
There was a problem hiding this comment.
I'm trying to think of whether this will allow something to work that never should have. If it doesn't then that's an improvement, otherwise we're accepting known invalid programs, which would be a regression.
Possible rewrite: I thought about this possible rewrite, but I think it leads to a bunch more issues than necessarily all that much better a solution.
An alternative/complementary approach I was thinking of was what if we did some untyped AST rewriting to solve the precedence issues, which I believe this is?
For example, (var x = 0; nonexistentName).f, can be thought of as (var x = 0; nonexistentName.f). Pretty sure we can come up with a rewrite rule(s) over untyped AST that has the desired effect. I guess the only "issue" is that an un/typed leading parameter used in an MCS won't receive the entire parenthetical syntax, but that wouldn't happen now anyways.
The rewrite rules might be:
- If there is only one constituent unwrap
- If there is more than one, wrap the last one in parenthesis, and move the dot operator and RHS in, then apply the rules again
After this change we're saying that an untyped dot operator (pre-rewriting rule) has higher priority than the untyped parenthetical expression.
Once evaluating untyped parameter overloads first rule goes in, we can defer the rewrite until the untyped are handled, meaning we'll be able to see the entire LHS in those cases.
Summary
When using the method-call syntax, templates/macros with
untypedparameters are now properly resolved to for
a.f,a.f(b), anda.f = b, whenais itself not semantically valid. The manual isupdated accordingly.
Fixes #1255.
Details
In
builtinFieldAccess, the typed left-hand expression was directlywritten back to the input AST, meaning that in case of an error, the
resulting
nkDotCallalready contained an error, preventing recoveryvia an
untypedtemplate/macro.The input AST is now only modified if the left-hand expression is not
an error, so that the dot-call rewrite gets the original AST. A
regression test covering the three basic dot expression rewrites is
added.
Since the first operand in a
.(a, b)(originating froma.b) cannow be untyped,
resolveOverloadsnow needs to ensure that saidoperand is typed before attempting to access the expression's type.
Notes for Reviewers
builtinFieldAccessshould not modify the input AST at all, but that requires a larger refactor