-
Notifications
You must be signed in to change notification settings - Fork 4.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Surprising System.ValueTuple requirement for deconstruction #18629
Comments
CC @jcouv |
(Do let me know if filing this sort of strange picky issue isn't useful, btw. When writing, I tend to explore all kinds of corner cases. This one probably isn't as corner-case-like as most, admittedly - but some really will be. If it's useful to see what surprises more investigative users, that's great - but I certainly don't want to cause churn to no-one's benefit.) |
@jskeet No problem with filing issues that may be questions. Please tag me on tuples and deconstruction questions for a faster triage. The compiler doesn't emit any The requirement of ValueTuple even when the returned tuple is not used/emitted was discussed with LDM. I'm not sure if it was explicitly recorded. I did record it in the feature notes (which may serve you for inspiration/quirks): #11299
|
If this ever does come up for discussion again, I'd certainly like to urge the team to consider it as a useful change. It probably doesn't matter too much for application authors, but I spend all my time writing libraries - I try to keep a wide range of targets (e.g. down to netstandard 1.3) and I avoid adding dependencies as far as possible. At the moment, I can add |
Re-opening the issue to track the request. |
I hit this too. I was using deconstruction in my Roslyn analyzer library (Microsoft/vs-threading) which still supports VS2015. I was disappointed to see that I have to use System.ValueTuple, which isn't supported on VS2015's version of Roslyn. So I changed all my ValueTuple use to Tuple and wrote my own Deconstruct extension methods, and the compilation fails because of the missing reference. Yet when I give it the reference, the resulting DLL doesn't retain that reference. So it's obviously not really needed, yet the compiler requiring it as input means that my project is super-fragile in that if anyone accidentally takes a real dependency on ValueTuple, the compiler will be happy, but VS2015 scenarios will break. |
Due to dotnet/roslyn#18629, we have to reference it to compile, but it compiles away provided we never actually use it.
@AArnott I didn't understand. |
@jcouv: Roslyn 1.x does not guarantee that the I build my analyzer with Roslyn 2.x, so I'm allowed to use deconstruction within it. And in fact my current workaround is to go ahead and reference System.ValueTuple during compilation of my analyzer, and then I have a unit test to ensure that the assembly reference is not retained in the compiled assembly so that it still works in Roslyn 1.x environments. |
@AArnott Thanks for the clarification. Makes sense. |
📝 From discussion with @VSadov, I'm exploring how to bind the deconstruction to have a |
@jcouv would the semantic model still follow the language spec and give a tuple type? |
Fix is in-progress (#20295). With that change, the returned tuple value is recognized as unused in an expression-statement and some positions of a for-statement. @gafter Yes, the semantic model will still return the proper type in all cases. The only caveat is that the tuple type for the expression may have an error type as its underlying type instead of a proper ValueTuple (when it was missing). |
Fixed by #20295 (in C# 7.2 and Visual Studio 2017 version 15.5). |
Now I get to moan and say this has given me extra work to write about it in C# in Depth ;) Seriously though - many thanks! |
Nice!! Now we should be able to deconstruct into spans. Specifically, spans are stack-only types and as such cannot be used to construct arrays/generics/tuples, etc... The following should work: Span<int> s;
(s, s) = SomethingDeconstructableIntoTwoSpans(); The following should not work though: Span<int> s;
var wouldHaveForbiddenType = (s, s) = SomethingDeconstructableIntoTwoSpans(); |
Version Used: 2.1.0.61520
Steps to Reproduce:
Try to compile
without a reference to
System.ValueTuple.dll
Expected Behavior:
Compilation with no errors. No tuple literals or tuple types are involved.
Actual Behavior:
Error:
When compiling with a reference to
System.ValueTuple.dll
, the resulting IL has no reference toSystem.ValueTuple.dll
, so why is it required at compile-time? This is very surprising.There mention of this in #11299:
I haven't found any mention of that in the LDM meetings.
The text was updated successfully, but these errors were encountered: