x == default is valid object equality for reference types#38853
x == default is valid object equality for reference types#38853jcouv merged 2 commits intodotnet:masterfrom
Conversation
b1a0aa8 to
87cf923
Compare
87cf923 to
f765887
Compare
| // SPEC: operand to the type of the other operand. Or: | ||
| // SPEC: (2) One operand is a value of type T where T is a type-parameter and the other operand is | ||
| // SPEC: the literal null. Furthermore T does not have the value type constraint. | ||
| // SPEC: (3) One operand is the literal default and the other operand is a reference-type. |
There was a problem hiding this comment.
Did the spec actually change?
There was a problem hiding this comment.
It hasn't yet, but I want to record the proposed spec change. I don't know a better way to keep the code and comment in sync...
| return false; | ||
| } | ||
|
|
||
| // If at least one side is null or default then clearly a conversion exists. |
There was a problem hiding this comment.
// If at least one side is null or default then clearly a conversion exists. [](start = 12, length = 76)
Is there a benefit in allowing null == default? Is it always true? #Resolved
There was a problem hiding this comment.
Good catch. Thanks
I'll keep as disallowed as it doesn't seem useful.
In reply to: 328337637 [](ancestors = 328337637)
| "; | ||
|
|
||
| var comp = CreateCompilation(source, parseOptions: TestOptions.Regular7_1); | ||
| comp.VerifyDiagnostics(); |
There was a problem hiding this comment.
comp.VerifyDiagnostics(); [](start = 12, length = 25)
For success cases, I think we would want to run the code and observe that they behaves per spec. #Resolved
| // || default != x; | ||
| Diagnostic(ErrorCode.ERR_BadBinaryOps, "default != x").WithArguments("!=", "default", "C").WithLocation(9, 16) | ||
| ); | ||
| comp.VerifyDiagnostics(); |
There was a problem hiding this comment.
comp.VerifyDiagnostics(); [](start = 12, length = 25)
For success cases, I think we would want to run the code and observe that they behaves per spec. #Resolved
AlekseyTs
left a comment
There was a problem hiding this comment.
LGTM (iteration 1), modulo suggested tests strengthening and pending successful CI result.
Since we've received significant feedback on part of the compat break we took in 16.4p1, we're making a refinement to reduce the negative impact.
t == defaultfor unconstrained types will remain an error (introduced in 16.4p1), butx == defaultwill be re-allowed for reference types (could bind as object equality).Fixes #38643
Relates to #37596 (16.4p1 breaking changes)