CR feedback on fixing constructor lookup when tuple created via "new" #10875
Conversation
|
@dotnet/roslyn-compiler - please review |
| { | ||
| static void Main() | ||
| { | ||
| var x = new (int a, int b)(1, 2); |
There was a problem hiding this comment.
Would it also be useful to test constructor initializer? var x = new (int a, int b)(1, 2) { a = 3; b = 4 };
There was a problem hiding this comment.
Turns out we did not support constructor initializers for tuples at all.
Added positive/negative tests
|
LGTM |
|
|
||
| var comp = CreateCompilationWithMscorlib(source, parseOptions: TestOptions.Regular.WithTuplesFeature()); | ||
| comp.VerifyDiagnostics( | ||
| // (6,22): error CS1729: 'ValueTuple<int, int>' does not contain a constructor that takes 3 arguments |
There was a problem hiding this comment.
Should we be using tuple notation for the type in the error message?
There was a problem hiding this comment.
Possibly.
Technically ctor belongs to the underlying type, so the error is correct - it tells exactly what type is missing a constructor.
I can see, however, how a custom error "this tuple cannot be constructed with N elements" could be more helpful.
Captured in #10874
|
@dotnet-bot test prtest/win/dbg/unit32 please |
|
@dotnet-bot test prtest/win/dbg/unit32 please |
gafter
changed the title
|
Recognizing this is a temporary solution that prevents a crash and this area is being reworked, LGTM. I suspect we may want to change the expected behavior of these tests after LDM review. |
CR feedback on fixing constructoir lookup when tuple created via "new"
see also
#10874
#10891