Implement binding and lowering for the with expression #43249
Conversation
agocke
force-pushed
the
with-binding
branch
3 times, most recently
from
f523e34
to
08e8600
Compare
A with expression binds to a parameterless, accessible, instance method called Clone on the receiver type. Then, all arguments are bound against record properties with the same name, and the expressions are assigned to the properties' backing fields.
| <data name="WRN_GeneratorFailedDuringGeneration" xml:space="preserve"> | ||
| <value>Generator '{0}' failed to generate source. It will not contribute to the output and compilation errors may occur as a result.</value> | ||
| <data name="ERR_InvalidWithReceiverType" xml:space="preserve"> | ||
| <value>The receiver of a `with` expression must have a valid non-void type.</value> |
There was a problem hiding this comment.
valid [](start = 59, length = 5)
What does "valid" mean here? I suggest dropping the word. If it were an error type we presumably would not report this. #Resolved
|
There are a handful of parts of the specification that don't make sense to me, but I don't know where to put comments on it. I can't really review the implementation against a spec that I don't understand. #Resolved |
|
I created a spec PR that we can use for discussion here: dotnet/csharplang#3353 #Resolved |
|
Done review pass (commit 2) |
src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_WithExpression.cs
Show resolved
Hide resolved
src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_WithExpression.cs
Show resolved
Hide resolved
|
Finished reviewing Iteration 4. #Resolved |
|
@gafter I think I've addressed your comments |
| if (symbol is MethodSymbol { ParameterCount: 0 } m) | ||
| { | ||
| cloneMethod = m; | ||
| break; |
There was a problem hiding this comment.
How does this handle multiple results? That can happen for example when Clone is overridden or hides another. This technique does not correspond to how invocations are handled, as there is no contract for the order of symbols in a lookup result. #Resolved
| originalBinder: this, | ||
| diagnose: false, | ||
| useSiteDiagnostics: ref useSiteDiagnostics); | ||
| if (lookupResult.IsSingleViable && |
There was a problem hiding this comment.
IsSingleViable [](start = 37, length = 14)
How does this handle properties that are overridden or hidden (more than one symbol in the lookup result)? This does not correspond to how expressions are bound. #Closed
|
Finished reviewing (Iteration 6). I think there are four active comments from me. #Resolved |
|
@gafter Thanks, updated. Regarding the lookup that will be adjusted soon in PRs unifying init-only and generating record Clone, so I've added prototype comments so I can include that in later PRs. |
A with expression binds to a parameterless, accessible, instance method
called Clone on the receiver type.
Then, all arguments are bound against record properties with the same
name, and the expressions are assigned to the properties' backing
fields.
Relates to #40726 (test plan for records)