Support consumption of unary extension operators. #78499
Conversation
AlekseyTs
added
Area-Compilers
Feature - Extension Everything
ghost
added
the
untriaged
| resultKind = LookupResultKind.Empty; | ||
| originalUserDefinedOperators = []; | ||
|
|
||
| if (operand.IsLiteralDefault() || // Reported not being able to target-type `default` elsewhere, so we can doing more work |
There was a problem hiding this comment.
nit: Do we need this line? (seems the second condition subsumes that) #Closed
There was a problem hiding this comment.
ReportUnaryOperatorError checks for this, and I do not think we we want to do any work in this case as well.
| } | ||
| } | ||
|
|
||
| diagnostics.Add(node, useSiteInfo); |
There was a problem hiding this comment.
Do we have a test for this? #Resolved
There was a problem hiding this comment.
Do we have a test for this?
No. And I didn't plan to add one.
| static void getDeclaredUserDefinedUnaryOperators(ArrayBuilder<NamedTypeSymbol> extensionDeclarationsInSingleScope, UnaryOperatorKind kind, string name, ArrayBuilder<UnaryOperatorSignature> operators) | ||
| { | ||
| foreach (NamedTypeSymbol extensionDeclaration in extensionDeclarationsInSingleScope) | ||
| { |
There was a problem hiding this comment.
nit: consider asserting extensionDeclaration.IsExtension #Resolved
| { | ||
| Debug.Assert(operand.Type.IsNullableType()); | ||
|
|
||
| if (!candidate.Method.ContainingType.ExtensionParameter.Type.IsValidNullableTypeArgument()) |
There was a problem hiding this comment.
Do we have a test where this check matters? #Resolved
There was a problem hiding this comment.
Do we have a test where this check matters?
No we don't, but we generally do not call MakeNullable if this check fails.
| resultKind = LookupResultKind.Empty; | ||
| originalUserDefinedOperators = []; | ||
|
|
||
| if (operand.IsLiteralDefault() || // Reported not being able to target-type `default` elsewhere, so we can doing more work |
There was a problem hiding this comment.
Actually "avoid" is missing
| // _ = +s1; | ||
| Diagnostic(ErrorCode.ERR_BadUnaryOp, "+s1").WithArguments("+", "object").WithLocation(17, 13) | ||
| ); | ||
| } |
There was a problem hiding this comment.
Consider testing consumption of operator defined in metadata image (I think I only saw compilation references) #Resolved
|
@jjonescz, @dotnet/roslyn-compiler For the second review |
| } | ||
| """; | ||
| var comp3 = CreateCompilation(src3, references: [comp1Ref], options: TestOptions.DebugExe); | ||
| CompileAndVerify(comp3, expectedOutput: "operator1").VerifyDiagnostics(); |
There was a problem hiding this comment.
Consider testing this with LangVersion=13 as well. #Resolved
There was a problem hiding this comment.
Consider testing this with LangVersion=13 as well.
I do not expect language version to have any effect on this code, and the scenario doesn't go through the modified/new code paths. One might say it wouldn't hurt, but the same could be said for every scenario we ever test. We have to draw a line somewhere.
| UnaryOperatorKind kind, | ||
| bool isChecked, | ||
| string name1, | ||
| string name2Opt, |
There was a problem hiding this comment.
Consider adding nullable annotations to new code. #Resolved
|
|
||
| if (isApplicableToReceiver(inferredCandidate, operand, ref useSiteInfo)) | ||
| { | ||
| operators[i] = inferredCandidate; |
There was a problem hiding this comment.
Nit: the method is called removeInapplicableToReceiverType but it's also replacing candidates which might be unexpected when reading the call site. Consider renaming the method. #Resolved
3 participants
Relates to test plan #76130