Comments (6)
Perhaps a fix could be to do something like what was done for lambdas and collection expressions in getArgumentForMethodTypeInference
?
roslyn/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs
Lines 7776 to 7797 in 0cd65d1
from roslyn.
The problem goes further than nullability. An error type is getting created by the method type inferrer for the ToDictionary call. This can even be seen in the IDE:
Versus for exact name matches on the tuple elements:
There are no diagnostics or emit issues in the final compilation, though. This is the cause of the detection of a "nullability mismatch."
from roslyn.
I looked into this further, and found that the nullability phase might be getting something right. I'm not sure this code should be compiling. If you insert an object cast, the code stops compiling because the tuple element names conflict and are discarded, and tuple.Name
and tuple.Value
can't be found:
#nullable disable
class C
{
void M((string Name, object Value)[] parameters)
{
_ = parameters.Append(("A", (object)"A")).ToDictionary(
tuple => tuple.Name, // '(string, object)' does not contain a definition for 'Name' [...]
tuple => tuple.Value); // '(string, object)' does not contain a definition for 'Value' [...]
}
}
Alternatively to adding the object cast, you could change the parameters to (string Name, string Value)[]
.
So, potentially, I don't think the original example should be compiling at all. The sad part is I know I've taken a dependency on this "only consider names of the receiver's tuple" behavior, over and over, with these LINQ methods.
What's happening is that when the tuple types aren't equal ((string, object)
versus (string, string)
), the Append method type inference takes (string, object)
as an exact bounds for T
via the T[]
expression being passed as the receiver parameter, because the spec says that the array element type is an exact bound if the element type is a value type (impl). Whereas the (string, string)
expression being passed as Append's second parameter becomes a lower bound on Append's T.
Then, when there is an exact bound, only the exact bounds become candidates for T (impl). Then the other bounds are merged with the candidates or removed (impl).
When it comes time to merge and remove, it all comes down to which way this line evaluates:
https://github.com/dotnet/roslyn/blob/main/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/MethodTypeInference.cs#L3299
(string, object)
is not equal to (string, string)
, so it never calls MergeAndReplaceIfStillCandidate with the lower bound. The lower bound ends up being totally ignored. This means that the conflicting tuple element names are never taken into consideration, and it infers all tuple element names from Append's receiver parameter and ignores the tuple element names from Append's second argument.
When the nullability pass gets here, it does call MergeAndReplaceIfStillCandidate, which removes tuple element names which conflict between the two arguments to Append (impl, impl).
from roslyn.
@jcouv Is this connected to #27961?
roslyn/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs
Lines 7748 to 7754 in 0cd65d1
from roslyn.
"Inference should be based on unconverted arguments" seems like it sums up the exact discrepancy that this issue was about. The arguments being passed back in to method type inference have already been target-typed using the result of the prior inference for the same type parameter.
This explains why the inference invoked by the nullability pass is evaluating to true at https://github.com/dotnet/roslyn/blob/main/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/MethodTypeInference.cs#L3299 and is therefore removing conflicting tuple element names.
from roslyn.
Perhaps a fix could be to do something like what was done for lambdas and collection expressions in
getArgumentForMethodTypeInference
?
Yes, that sounds right. The infrastructure for target-typed expressions has evolved over time and I think that collection expressions are the latest example, so may be a good example to follow.
from roslyn.
Related Issues (20)
- Reference highlighting doesn't work for some types from constructor expression
- Reference highlighting doesn't work from primary constructor base call constructor call
- `InvalidOperationException` in `SemanticTokensHelpers` when applying embedded classification on verbatim string
- Should collapse separate comment blocks separately
- Anonymous-Params-delegate is generated incorrectly with AllowRefStructFlag HOT 3
- Flaky test: Microsoft.CodeAnalysis.MSBuild.UnitTests.NewlyCreatedProjectsFromDotNetNew.ValidateVisualBasicTemplateProjects HOT 1
- Diagnostics from C# interactive window won't go away
- [Automated] PRs inserted in VS build main-35220.189
- Emission of NullabilityAttribute(2) on unconstrained generic type parameters causes incorrect runtime reporting of nullability in .NET 9
- When executing "Format document" of "Code cleanup" on save, it does not honor editorconfig, if source file build action is set to "None"
- For loops autofill is different when inside a loop
- IncrementalGenerator stopped working in VS 17.11 HOT 1
- Deconstruct works inconsistently in patterns and in deconstruction into tuple
- CS9203 False Positive HOT 1
- Microsoft.Cci.PdbWriter.GetAssemblyReferenceAlias throws Null Reference exception HOT 2
- Error reporting stops working HOT 3
- [Automated] PRs inserted in VS build feature.debugger.main-35223.06
- Collection expressions may result in unnecessary tail increment HOT 2
- CSC : error CS8034: Unable to load Analyzer assembly
- Collection expression spread optimization using `List<T>.AddRange(IEnumerable<T>)` skipped for `ICollection<U>` when `T` and `U` are distinct types at compile-time only HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from roslyn.