diff --git a/ICSharpCode.Decompiler.Tests/Semantics/TypeInferenceTests.cs b/ICSharpCode.Decompiler.Tests/Semantics/TypeInferenceTests.cs index 0770355eb..e70ab6579 100644 --- a/ICSharpCode.Decompiler.Tests/Semantics/TypeInferenceTests.cs +++ b/ICSharpCode.Decompiler.Tests/Semantics/TypeInferenceTests.cs @@ -854,25 +854,24 @@ namespace ICSharpCode.Decompiler.Tests.Semantics } [Test] - public void FixingDoesNotMergeBoundsThatDifferInNullability() + public void FixingMergesBoundsThatDifferInNullability() { // Signature: M(T x, T y) // Invocation: M(nullableArrayOfString, arrayOfString); - // Merging nullability requires the variance of the position, which this - // implementation does not track, so such bounds stay distinct and fixing fails - // (csc infers string[]?). + // Merging nullability in this covariant position should result in T=string[]? (the nullable array type). var comp = RefAssemblyCompilation.Instance; var T = new DefaultTypeParameter(comp, SymbolKind.Method, 0, "T"); IType stringType = comp.FindType(KnownTypeCode.String); - new TypeInference(comp).InferTypeArguments(new ITypeParameter[] { T }, - new[] { + var result = new TypeInference(comp).InferTypeArguments([T], + [ new ResolveResult(new ArrayType(comp, stringType, 1, Nullability.Nullable)), new ResolveResult(new ArrayType(comp, stringType)) - }, - new IType[] { T, T }, + ], + [T, T], out bool success); - Assert.That(success, Is.False); + Assert.That(success, Is.True); + Assert.That(result, Is.EqualTo([new ArrayType(comp, stringType, 1, Nullability.Nullable)])); } [Test] @@ -1257,6 +1256,18 @@ namespace ICSharpCode.Decompiler.Tests.Semantics Is.EqualTo(SpecialType.Dynamic)); Assert.That(success); } + + [Test] + public void BestCommonTypeObjectAndNullableObject() + { + Assert.That( + ti.GetBestCommonType(new[] { + new ResolveResult(compilation.FindType(KnownTypeCode.Object).ChangeNullability(Nullability.NotNullable)), + new ResolveResult(compilation.FindType(KnownTypeCode.Object).ChangeNullability(Nullability.Nullable)) + }, out bool success), + Is.EqualTo(compilation.FindType(KnownTypeCode.Object).ChangeNullability(Nullability.Nullable))); + Assert.That(success); + } #endregion #region FindTypeInBounds @@ -1399,8 +1410,12 @@ namespace ICSharpCode.Decompiler.Tests.Semantics // ReadOnlyCollectionBuilder appears because the test compilation includes // System.Core, which declares it as another public implementation of both // IList and IList. + var typesInBounds = FindAllTypesInBounds(Resolve(), Resolve(typeof(IEnumerable), typeof(IEnumerable), typeof(IList))); + // As this finds all derived types, the result set contains compiler-generated types like <>z__ReadOnlyArray`1. + // We filter those out to make the test more robust against changes. + typesInBounds = typesInBounds.Where(t => !t.GetDefinition().IsCompilerGenerated()).ToArray(); Assert.That( - FindAllTypesInBounds(Resolve(), Resolve(typeof(IEnumerable), typeof(IEnumerable), typeof(IList))), + typesInBounds, Is.EqualTo(Resolve(typeof(List), typeof(List), typeof(Collection), typeof(Collection), typeof(ReadOnlyCollection), typeof(ReadOnlyCollection), typeof(System.Runtime.CompilerServices.ReadOnlyCollectionBuilder), typeof(System.Runtime.CompilerServices.ReadOnlyCollectionBuilder)))); } #endregion diff --git a/ICSharpCode.Decompiler/CSharp/Resolver/TypeInference.cs b/ICSharpCode.Decompiler/CSharp/Resolver/TypeInference.cs index 41aa50324..959982ec9 100644 --- a/ICSharpCode.Decompiler/CSharp/Resolver/TypeInference.cs +++ b/ICSharpCode.Decompiler/CSharp/Resolver/TypeInference.cs @@ -251,7 +251,7 @@ namespace ICSharpCode.Decompiler.CSharp.Resolver return; // Two exact bounds that differ only in tuple element names are not conflicting; // their names are merged instead (kept where both agree, dropped otherwise). - IType merged = MergeSimilarTypes(ExactBound, type); + IType merged = MergeSimilarTypes(ExactBound, type, VarianceModifier.Invariant); if (merged != null) ExactBound = merged; else @@ -986,8 +986,14 @@ namespace ICSharpCode.Decompiler.CSharp.Resolver // merged - kept where both sides agree, dropped where they conflict. See // MergeTupleNames in Roslyn's MethodTypeInference.cs. IType fixedTo = tp.ExactBound; - foreach (var b in tp.LowerBounds.Concat(tp.UpperBounds)) - fixedTo = MergeSimilarTypes(fixedTo, b) ?? fixedTo; + foreach (var b in tp.LowerBounds) + { + fixedTo = MergeSimilarTypes(fixedTo, b, VarianceModifier.Covariant) ?? fixedTo; + } + foreach (var b in tp.UpperBounds) + { + fixedTo = MergeSimilarTypes(fixedTo, b, VarianceModifier.Contravariant) ?? fixedTo; + } // the exact bound determines the result, up to the merged element names tp.FixedTo = fixedTo; // check validity @@ -1016,23 +1022,23 @@ namespace ICSharpCode.Decompiler.CSharp.Resolver } /// - /// Merges similar types that differ only in tuple element names and/or object/dynamic, recursively. - /// * for tuple element names, a name is kept where both sides agree and dropped where they conflict. - /// * for object/dynamic, dynamic is preferred over object. + /// Merges similar types that differ only in any of these aspects. + /// * tuple element names: a name is kept where both sides agree and dropped where they conflict. + /// * object/dynamic: dynamic is preferred over object. + /// * nullability: depends on the variance of the position. /// Returns null if the types differ in any other aspects. /// - static IType MergeSimilarTypes(IType a, IType b) + static IType MergeSimilarTypes(IType a, IType b, VarianceModifier variance) { if (a.Equals(b)) return a; - // Roslyn merges differing nullability based on the variance of the position; this - // implementation does not track that, so differently annotated types are left alone. - if (a.Nullability != b.Nullability) - return null; - if (a is NullabilityAnnotatedType na && b is NullabilityAnnotatedType nb) + if (a is NullabilityAnnotatedType || b is NullabilityAnnotatedType) { - return MergeSimilarTypes(na.TypeWithoutAnnotation, nb.TypeWithoutAnnotation) - ?.ChangeNullability(a.Nullability); + var nullability = MergeNullability(a.Nullability, b.Nullability, variance); + var merged = MergeSimilarTypes(a.WithoutNullability(), b.WithoutNullability(), variance); + if (merged == null) + return null; + return merged.ChangeNullability(nullability); } if (a.Kind == TypeKind.Dynamic && b.IsKnownType(KnownTypeCode.Object)) { @@ -1048,7 +1054,11 @@ namespace ICSharpCode.Decompiler.CSharp.Resolver var mergedElements = ImmutableArray.CreateBuilder(ta.ElementTypes.Length); for (int i = 0; i < ta.ElementTypes.Length; i++) { - var merged = MergeSimilarTypes(ta.ElementTypes[i], tb.ElementTypes[i]); + // Note: even though ValueTuple has invariant type parameters, + // Roslyn merges tuple element types in a covariant manner. + var merged = MergeSimilarTypes( + ta.ElementTypes[i], tb.ElementTypes[i], + variance.Combine(VarianceModifier.Covariant)); if (merged == null) return null; mergedElements.Add(merged); @@ -1062,28 +1072,75 @@ namespace ICSharpCode.Decompiler.CSharp.Resolver ta.GetDefinition()?.ParentModule); } if (a is ParameterizedType pa && b is ParameterizedType pb - && pa.GenericType.Equals(pb.GenericType) && pa.TypeArguments.Count == pb.TypeArguments.Count) { + var genericType = MergeSimilarTypes(pa.GenericType, pb.GenericType, variance); + if (genericType == null) + return null; var mergedArgs = new IType[pa.TypeArguments.Count]; for (int i = 0; i < pa.TypeArguments.Count; i++) { - var merged = MergeSimilarTypes(pa.TypeArguments[i], pb.TypeArguments[i]); + var merged = MergeSimilarTypes( + pa.TypeArguments[i], pb.TypeArguments[i], + variance.Combine(pa.TypeParameters[i].Variance)); if (merged == null) return null; mergedArgs[i] = merged; } - return new ParameterizedType(pa.GenericType, mergedArgs); + return new ParameterizedType(genericType, mergedArgs); } if (a is ArrayType arrA && b is ArrayType arrB && arrA.Dimensions == arrB.Dimensions) { - var mergedElem = MergeSimilarTypes(arrA.ElementType, arrB.ElementType); + // Roslyn ArrayTypeSymbol merges in a covariant manner. + var mergedElem = MergeSimilarTypes( + arrA.ElementType, arrB.ElementType, + variance.Combine(VarianceModifier.Covariant)); if (mergedElem == null) return null; - return new ArrayType(arrA.Compilation, mergedElem, arrA.Dimensions, arrA.Nullability); + var nullability = MergeNullability(arrA.Nullability, arrB.Nullability, variance); + return new ArrayType(arrA.Compilation, mergedElem, arrA.Dimensions, nullability); + } + if (a is PointerType ptrA && b is PointerType ptrB) + { + var mergedElem = MergeSimilarTypes( + ptrA.ElementType, ptrB.ElementType, + variance.Combine(VarianceModifier.Invariant)); + if (mergedElem == null) + return null; + return new PointerType(mergedElem); } return null; } + + static Nullability MergeNullability(Nullability a, Nullability b, VarianceModifier variance) + { + // Like Roslyn's MergeNullableAnnotation() + return (variance, a, b) switch { + // Covariant merging rules: Nullable wins over Oblivious which wins over NotNullable. + (VarianceModifier.Covariant, Nullability.Nullable, _) => Nullability.Nullable, + (VarianceModifier.Covariant, _, Nullability.Nullable) => Nullability.Nullable, + (VarianceModifier.Covariant, Nullability.Oblivious, _) => Nullability.Oblivious, + (VarianceModifier.Covariant, _, Nullability.Oblivious) => Nullability.Oblivious, + (VarianceModifier.Covariant, Nullability.NotNullable, Nullability.NotNullable) => Nullability.NotNullable, + // Contravariant merging rules: NotNullable wins over Oblivious which wins over Nullable. + (VarianceModifier.Contravariant, Nullability.NotNullable, _) => Nullability.NotNullable, + (VarianceModifier.Contravariant, _, Nullability.NotNullable) => Nullability.NotNullable, + (VarianceModifier.Contravariant, Nullability.Oblivious, _) => Nullability.Oblivious, + (VarianceModifier.Contravariant, _, Nullability.Oblivious) => Nullability.Oblivious, + (VarianceModifier.Contravariant, Nullability.Nullable, Nullability.Nullable) => Nullability.Nullable, + // Invariant merging rules: NotNullable wins over Nullable which wins over Oblivious. + // Weird but that's what Roslyn does: + // static T M(ref T x, ref T y) => x; + // M(ref nullableArray, nonNullableArray); + // T is inferred as int[] and then the first argument reports a "possible null reference assignment" warning. + (VarianceModifier.Invariant, Nullability.NotNullable, _) => Nullability.NotNullable, + (VarianceModifier.Invariant, _, Nullability.NotNullable) => Nullability.NotNullable, + (VarianceModifier.Invariant, Nullability.Nullable, _) => Nullability.Nullable, + (VarianceModifier.Invariant, _, Nullability.Nullable) => Nullability.Nullable, + (VarianceModifier.Invariant, Nullability.Oblivious, Nullability.Oblivious) => Nullability.Oblivious, + _ => throw new NotSupportedException("Unexpected nullability combination: " + a + ", " + b + " with variance " + variance) + }; + } #endregion #region Finding the best common type of a set of expressions @@ -1166,28 +1223,33 @@ namespace ICSharpCode.Decompiler.CSharp.Resolver // Deduplicate types. This also merges types that differ only in tuple // element names and/or object/dynamic. var candidateMergeDict = new Dictionary(); - foreach (var candidate in lowerBounds.Concat(upperBounds)) + void AddCandidates(IEnumerable candidates, VarianceModifier variance) { - var key = candidate.AcceptVisitor(NormalizeTypeVisitor.KeyForTypeMerging); - if (candidateMergeDict.TryGetValue(key, out var existing)) + foreach (var candidate in candidates) { - var merged = MergeSimilarTypes(existing, candidate); - Log.WriteLine(" Merged similar types " + existing + " and " + candidate + " into " + merged); - if (merged != null) + var key = candidate.AcceptVisitor(NormalizeTypeVisitor.KeyForTypeMerging); + if (candidateMergeDict.TryGetValue(key, out var existing)) { - candidateMergeDict[key] = merged; + var merged = MergeSimilarTypes(existing, candidate, variance); + Log.WriteLine(" Merged similar types " + existing + " and " + candidate + " into " + merged); + if (merged != null) + { + candidateMergeDict[key] = merged; + } + else + { + Debug.Fail("MergeSimilarTypes should always be able to merge;" + + " is the KeyForTypeMerging visitor misconfigured?"); + } } else { - Debug.Fail("MergeSimilarTypes should always be able to merge;" - + " is the KeyForTypeMerging visitor misconfigured?"); + candidateMergeDict.Add(key, candidate); } } - else - { - candidateMergeDict.Add(key, candidate); - } } + AddCandidates(lowerBounds, VarianceModifier.Covariant); + AddCandidates(upperBounds, VarianceModifier.Contravariant); Log.WriteCollection("FindTypesInBound, Merged types from bounds=", candidateMergeDict.Values); diff --git a/ICSharpCode.Decompiler/TypeSystem/ITypeParameter.cs b/ICSharpCode.Decompiler/TypeSystem/ITypeParameter.cs index c32ff410f..4a8d9d7dc 100644 --- a/ICSharpCode.Decompiler/TypeSystem/ITypeParameter.cs +++ b/ICSharpCode.Decompiler/TypeSystem/ITypeParameter.cs @@ -129,19 +129,32 @@ namespace ICSharpCode.Decompiler.TypeSystem /// /// Represents the variance of a type parameter. /// - public enum VarianceModifier : byte + public enum VarianceModifier : sbyte { /// /// The type parameter is not variant. /// - Invariant, + Invariant = 0, /// /// The type parameter is covariant (used in output position). /// - Covariant, + Covariant = 1, /// /// The type parameter is contravariant (used in input position). /// - Contravariant + Contravariant = -1 }; + + static class VarianceExtensions + { + /// + /// Combines variance modifiers. + /// It's like a multiplication where Invariant is 0, Covariant is +1 and Contravariant is -1. + /// + public static VarianceModifier Combine(this VarianceModifier a, VarianceModifier b) + { + // By picking matching enum values, we can actually implement this as multiplication. + return (VarianceModifier)((sbyte)a * (sbyte)b); + } + } } diff --git a/ICSharpCode.Decompiler/TypeSystem/NormalizeTypeVisitor.cs b/ICSharpCode.Decompiler/TypeSystem/NormalizeTypeVisitor.cs index 14c4fe91a..6d25464e7 100644 --- a/ICSharpCode.Decompiler/TypeSystem/NormalizeTypeVisitor.cs +++ b/ICSharpCode.Decompiler/TypeSystem/NormalizeTypeVisitor.cs @@ -65,7 +65,7 @@ namespace ICSharpCode.Decompiler.TypeSystem RemoveTupleElementNames = true, RemoveModOpt = false, RemoveModReq = false, - RemoveNullability = false, + RemoveNullability = true, }; internal static readonly NormalizeTypeVisitor IgnoreNullability = new NormalizeTypeVisitor {