未验证 提交 abe0db95 编写于 作者: J Julien Couvreur 提交者: GitHub

Fix nullability check in override/implementation (#49723)

上级 08a2543b
......@@ -750,7 +750,7 @@ private static ImmutableArray<Symbol> PerformCrefOverloadResolution(ArrayBuilder
// CONSIDER: we might want to reuse this method symbol (as long as the MethodKind and Vararg-ness match).
signatureMember = new SignatureOnlyMethodSymbol(
methodKind: candidateMethodKind,
typeParameters: IndexedTypeParameterSymbol.Take(signatureMemberArity),
typeParameters: IndexedTypeParameterSymbol.TakeSymbols(signatureMemberArity),
parameters: parameterSymbols,
// This specific comparer only looks for varargs.
callingConvention: candidateMethodIsVararg ? Microsoft.Cci.CallingConvention.ExtraArguments : Microsoft.Cci.CallingConvention.HasThis,
......
......@@ -743,7 +743,7 @@ private static MethodSymbol SubstituteTypeParameters(MethodSymbol method)
return method;
}
return method.Construct(IndexedTypeParameterSymbol.Take(n).Cast<TypeParameterSymbol, TypeSymbol>());
return method.Construct(IndexedTypeParameterSymbol.Take(n));
}
private bool AreNamedTypesEqual(NamedTypeSymbol type, NamedTypeSymbol other)
......
......@@ -32,5 +32,16 @@ internal enum NullableAnnotation : byte
/// Type is annotated with '?' - string?, T?.
/// </summary>
Annotated,
/// <summary>
/// Used for indexed type parameters and used locally in override/implementation checks.
/// When substituting a type parameter with Ignored annotation into some original type parameter
/// with some other annotation, the result is the annotation from the original symbol.
///
/// T annotated + (T -> U ignored) = U annotated
/// T oblivious + (T -> U ignored) = U oblivious
/// T not-annotated + (T -> U ignored) = U not-annotated
/// </summary>
Ignored,
}
}
......@@ -5,6 +5,7 @@
#nullable disable
using System.Collections.Immutable;
using System.Diagnostics;
using Microsoft.CodeAnalysis.CSharp.Symbols;
using Roslyn.Utilities;
......@@ -25,37 +26,55 @@ internal static class NullableAnnotationExtensions
/// Join nullable annotations from the set of lower bounds for fixing a type parameter.
/// This uses the covariant merging rules. (Annotated wins over Oblivious which wins over NotAnnotated)
/// </summary>
public static NullableAnnotation Join(this NullableAnnotation a, NullableAnnotation b) => (a < b) ? b : a;
public static NullableAnnotation Join(this NullableAnnotation a, NullableAnnotation b)
{
Debug.Assert(a != NullableAnnotation.Ignored);
Debug.Assert(b != NullableAnnotation.Ignored);
return (a < b) ? b : a;
}
/// <summary>
/// Meet two nullable annotations for computing the nullable annotation of a type parameter from upper bounds.
/// This uses the contravariant merging rules. (NotAnnotated wins over Oblivious which wins over Annotated)
/// </summary>
public static NullableAnnotation Meet(this NullableAnnotation a, NullableAnnotation b) => (a < b) ? a : b;
public static NullableAnnotation Meet(this NullableAnnotation a, NullableAnnotation b)
{
Debug.Assert(a != NullableAnnotation.Ignored);
Debug.Assert(b != NullableAnnotation.Ignored);
return (a < b) ? a : b;
}
/// <summary>
/// Return the nullable annotation to use when two annotations are expected to be "compatible", which means
/// they could be the same. These are the "invariant" merging rules. (NotAnnotated wins over Annotated which wins over Oblivious)
/// </summary>
public static NullableAnnotation EnsureCompatible(this NullableAnnotation a, NullableAnnotation b) =>
(a, b) switch
public static NullableAnnotation EnsureCompatible(this NullableAnnotation a, NullableAnnotation b)
{
Debug.Assert(a != NullableAnnotation.Ignored);
Debug.Assert(b != NullableAnnotation.Ignored);
return (a, b) switch
{
(NullableAnnotation.Oblivious, _) => b,
(_, NullableAnnotation.Oblivious) => a,
_ => a < b ? a : b,
};
}
/// <summary>
/// Merges nullability.
/// </summary>
public static NullableAnnotation MergeNullableAnnotation(this NullableAnnotation a, NullableAnnotation b, VarianceKind variance) =>
variance switch
public static NullableAnnotation MergeNullableAnnotation(this NullableAnnotation a, NullableAnnotation b, VarianceKind variance)
{
Debug.Assert(a != NullableAnnotation.Ignored);
Debug.Assert(b != NullableAnnotation.Ignored);
return variance switch
{
VarianceKind.In => a.Meet(b),
VarianceKind.Out => a.Join(b),
VarianceKind.None => a.EnsureCompatible(b),
_ => throw ExceptionUtilities.UnexpectedValue(variance)
};
}
/// <summary>
/// The attribute (metadata) representation of <see cref="NullableAnnotation.NotAnnotated"/>.
......@@ -105,18 +124,25 @@ internal static ImmutableArray<ITypeSymbol> GetPublicSymbols(this ImmutableArray
internal static ImmutableArray<CodeAnalysis.NullableAnnotation> ToPublicAnnotations(this ImmutableArray<TypeWithAnnotations> types) =>
types.SelectAsArray(t => t.ToPublicAnnotation());
internal static CodeAnalysis.NullableAnnotation ToPublicAnnotation(TypeSymbol type, NullableAnnotation annotation) =>
annotation switch
internal static CodeAnalysis.NullableAnnotation ToPublicAnnotation(TypeSymbol type, NullableAnnotation annotation)
{
Debug.Assert(annotation != NullableAnnotation.Ignored);
return annotation switch
{
CSharp.NullableAnnotation.Annotated => CodeAnalysis.NullableAnnotation.Annotated,
CSharp.NullableAnnotation.NotAnnotated => CodeAnalysis.NullableAnnotation.NotAnnotated,
NullableAnnotation.Annotated => CodeAnalysis.NullableAnnotation.Annotated,
NullableAnnotation.NotAnnotated => CodeAnalysis.NullableAnnotation.NotAnnotated,
// A value type may be oblivious or not annotated depending on whether the type reference
// is from source or metadata. (Binding using the #nullable context only when setting the annotation
// to avoid checking IsValueType early.) The annotation is normalized here in the public API.
CSharp.NullableAnnotation.Oblivious when type.IsValueType => CodeAnalysis.NullableAnnotation.NotAnnotated,
CSharp.NullableAnnotation.Oblivious => CodeAnalysis.NullableAnnotation.None,
NullableAnnotation.Oblivious when type.IsValueType => CodeAnalysis.NullableAnnotation.NotAnnotated,
NullableAnnotation.Oblivious => CodeAnalysis.NullableAnnotation.None,
NullableAnnotation.Ignored => CodeAnalysis.NullableAnnotation.None,
_ => throw ExceptionUtilities.UnexpectedValue(annotation)
};
}
internal static CSharp.NullableAnnotation ToInternalAnnotation(this CodeAnalysis.NullableAnnotation annotation) =>
annotation switch
......
......@@ -989,7 +989,7 @@ IEqualityComparer<MethodSymbol> retargetedMethodComparer
retargetedType,
method.MethodKind,
method.CallingConvention,
IndexedTypeParameterSymbol.Take(method.Arity),
IndexedTypeParameterSymbol.TakeSymbols(method.Arity),
targetParamsBuilder.ToImmutableAndFree(),
method.RefKind,
method.IsInitOnly,
......
......@@ -82,7 +82,7 @@ private static void GrowPool(int count)
/// </summary>
/// <param name="count"></param>
/// <returns></returns>
internal static ImmutableArray<TypeParameterSymbol> Take(int count)
internal static ImmutableArray<TypeParameterSymbol> TakeSymbols(int count)
{
if (count > s_parameterPool.Length)
{
......@@ -99,6 +99,23 @@ internal static ImmutableArray<TypeParameterSymbol> Take(int count)
return builder.ToImmutableAndFree();
}
internal static ImmutableArray<TypeWithAnnotations> Take(int count)
{
if (count > s_parameterPool.Length)
{
GrowPool(count);
}
var builder = ArrayBuilder<TypeWithAnnotations>.GetInstance();
for (int i = 0; i < count; i++)
{
builder.Add(TypeWithAnnotations.Create(GetTypeParameter(i), NullableAnnotation.Ignored));
}
return builder.ToImmutableAndFree();
}
public override int Ordinal
{
get { return _index; }
......
......@@ -944,7 +944,7 @@ void checkSingleOverriddenMember(Symbol overridingMember, Symbol overriddenMembe
if (overridingMethod.IsGenericMethod)
{
overriddenMethod = overriddenMethod.Construct(overridingMethod.TypeArgumentsWithAnnotations);
overriddenMethod = overriddenMethod.Construct(TypeMap.TypeParametersAsTypeSymbolsWithIgnoredAnnotations(overridingMethod.TypeParameters));
}
// Check for mismatched byref returns and return type. Ignore custom modifiers, because this diagnostic is based on the C# semantics.
......
......@@ -632,7 +632,7 @@ private static void PartialMethodChecks(SourceOrdinaryMethodSymbol definition, S
{
Debug.Assert(!ReferenceEquals(definition, implementation));
MethodSymbol constructedDefinition = definition.ConstructIfGeneric(implementation.TypeArgumentsWithAnnotations);
MethodSymbol constructedDefinition = definition.ConstructIfGeneric(TypeMap.TypeParametersAsTypeSymbolsWithIgnoredAnnotations(implementation.TypeParameters));
bool returnTypesEqual = constructedDefinition.ReturnTypeWithAnnotations.Equals(implementation.ReturnTypeWithAnnotations, TypeCompareKind.AllIgnoreOptions);
if (!returnTypesEqual
&& !SourceMemberContainerTypeSymbol.IsOrContainsErrorType(implementation.ReturnType)
......
......@@ -20,11 +20,16 @@ namespace Microsoft.CodeAnalysis.CSharp.Symbols
/// </summary>
internal sealed class TypeMap : AbstractTypeParameterMap
{
public static readonly System.Func<TypeWithAnnotations, TypeSymbol> AsTypeSymbol = t => t.Type;
public static readonly Func<TypeWithAnnotations, TypeSymbol> AsTypeSymbol = t => t.Type;
internal static ImmutableArray<TypeWithAnnotations> TypeParametersAsTypeSymbolsWithAnnotations(ImmutableArray<TypeParameterSymbol> typeParameters)
{
return typeParameters.SelectAsArray((tp) => TypeWithAnnotations.Create(tp));
return typeParameters.SelectAsArray(static (tp) => TypeWithAnnotations.Create(tp));
}
internal static ImmutableArray<TypeWithAnnotations> TypeParametersAsTypeSymbolsWithIgnoredAnnotations(ImmutableArray<TypeParameterSymbol> typeParameters)
{
return typeParameters.SelectAsArray(static (tp) => TypeWithAnnotations.Create(tp, NullableAnnotation.Ignored));
}
internal static ImmutableArray<TypeSymbol> AsTypeSymbols(ImmutableArray<TypeWithAnnotations> typesOpt)
......
......@@ -1690,10 +1690,12 @@ internal static void CheckNullableReferenceTypeMismatchOnImplementingMember(Type
{
if (arg.isExplicit)
{
// We use ConstructedFrom symbols here and below to not leak methods with Ignored annotations in type arguments
// into diagnostics
diagnostics.Add(topLevel ?
ErrorCode.WRN_TopLevelNullabilityMismatchInReturnTypeOnExplicitImplementation :
ErrorCode.WRN_NullabilityMismatchInReturnTypeOnExplicitImplementation,
implementingMethod.Locations[0], new FormattedSymbol(implementedMethod, SymbolDisplayFormat.MinimallyQualifiedFormat));
implementingMethod.Locations[0], new FormattedSymbol(implementedMethod.ConstructedFrom, SymbolDisplayFormat.MinimallyQualifiedFormat));
}
else
{
......@@ -1702,7 +1704,7 @@ internal static void CheckNullableReferenceTypeMismatchOnImplementingMember(Type
ErrorCode.WRN_NullabilityMismatchInReturnTypeOnImplicitImplementation,
GetImplicitImplementationDiagnosticLocation(implementedMethod, arg.implementingType, implementingMethod),
new FormattedSymbol(implementingMethod, SymbolDisplayFormat.MinimallyQualifiedFormat),
new FormattedSymbol(implementedMethod, SymbolDisplayFormat.MinimallyQualifiedFormat));
new FormattedSymbol(implementedMethod.ConstructedFrom, SymbolDisplayFormat.MinimallyQualifiedFormat));
}
};
......@@ -1716,7 +1718,7 @@ internal static void CheckNullableReferenceTypeMismatchOnImplementingMember(Type
ErrorCode.WRN_NullabilityMismatchInParameterTypeOnExplicitImplementation,
implementingMethod.Locations[0],
new FormattedSymbol(implementingParameter, SymbolDisplayFormat.ShortFormat),
new FormattedSymbol(implementedMethod, SymbolDisplayFormat.MinimallyQualifiedFormat));
new FormattedSymbol(implementedMethod.ConstructedFrom, SymbolDisplayFormat.MinimallyQualifiedFormat));
}
else
{
......@@ -1726,7 +1728,7 @@ internal static void CheckNullableReferenceTypeMismatchOnImplementingMember(Type
GetImplicitImplementationDiagnosticLocation(implementedMethod, arg.implementingType, implementingMethod),
new FormattedSymbol(implementingParameter, SymbolDisplayFormat.ShortFormat),
new FormattedSymbol(implementingMethod, SymbolDisplayFormat.MinimallyQualifiedFormat),
new FormattedSymbol(implementedMethod, SymbolDisplayFormat.MinimallyQualifiedFormat));
new FormattedSymbol(implementedMethod.ConstructedFrom, SymbolDisplayFormat.MinimallyQualifiedFormat));
}
};
......@@ -1770,7 +1772,7 @@ internal static void CheckNullableReferenceTypeMismatchOnImplementingMember(Type
if (implementedMethod.IsGenericMethod)
{
implementedMethod = implementedMethod.Construct(implementingMethod.TypeArgumentsWithAnnotations);
implementedMethod = implementedMethod.Construct(TypeMap.TypeParametersAsTypeSymbolsWithIgnoredAnnotations(implementingMethod.TypeParameters));
}
SourceMemberContainerTypeSymbol.CheckValidNullableMethodOverride(
......
......@@ -86,6 +86,7 @@ internal static TypeWithAnnotations Create(TypeSymbol typeSymbol, NullableAnnota
return default;
}
Debug.Assert(nullableAnnotation != NullableAnnotation.Ignored || typeSymbol.IsTypeParameter());
switch (nullableAnnotation)
{
case NullableAnnotation.Oblivious:
......@@ -161,11 +162,6 @@ internal bool CanBeAssignedNull
}
}
private static bool IsIndexedTypeParameter(TypeSymbol typeSymbol)
{
return typeSymbol is IndexedTypeParameterSymbol;
}
private static TypeWithAnnotations CreateNonLazyType(TypeSymbol typeSymbol, NullableAnnotation nullableAnnotation, ImmutableArray<CustomModifier> customModifiers)
{
return new TypeWithAnnotations(typeSymbol, nullableAnnotation, Extensions.Create(customModifiers));
......@@ -429,6 +425,9 @@ public bool IsAtLeastAsVisibleAs(Symbol sym, ref HashSet<DiagnosticInfo> useSite
internal TypeWithAnnotations SubstituteTypeCore(AbstractTypeMap typeMap)
{
// Ignored may only appear on a replacement type and will not survive the substitution (ie. the original annotation wins over Ignored)
Debug.Assert(NullableAnnotation != NullableAnnotation.Ignored);
var newCustomModifiers = typeMap.SubstituteCustomModifiers(this.CustomModifiers);
TypeSymbol typeSymbol = this.Type;
var newTypeWithModifiers = typeMap.SubstituteType(typeSymbol);
......@@ -437,6 +436,7 @@ internal TypeWithAnnotations SubstituteTypeCore(AbstractTypeMap typeMap)
{
Debug.Assert(newTypeWithModifiers.NullableAnnotation.IsOblivious() || (typeSymbol.IsNullableType() && newTypeWithModifiers.NullableAnnotation.IsAnnotated()));
Debug.Assert(newTypeWithModifiers.CustomModifiers.IsEmpty);
Debug.Assert(NullableAnnotation != NullableAnnotation.Ignored);
if (typeSymbol.Equals(newTypeWithModifiers.Type, TypeCompareKind.ConsiderEverything) &&
newCustomModifiers == CustomModifiers)
......@@ -457,7 +457,7 @@ internal TypeWithAnnotations SubstituteTypeCore(AbstractTypeMap typeMap)
{
return this; // substitution had no effect on the type or modifiers
}
else if (Is((TypeParameterSymbol)typeSymbol))
else if (Is((TypeParameterSymbol)typeSymbol) && newTypeWithModifiers.NullableAnnotation != NullableAnnotation.Ignored)
{
return newTypeWithModifiers;
}
......@@ -468,14 +468,13 @@ internal TypeWithAnnotations SubstituteTypeCore(AbstractTypeMap typeMap)
}
NullableAnnotation newAnnotation;
Debug.Assert(!IsIndexedTypeParameter(newTypeWithModifiers.Type) || newTypeWithModifiers.NullableAnnotation.IsOblivious());
Debug.Assert(newTypeWithModifiers.Type is not IndexedTypeParameterSymbol || newTypeWithModifiers.NullableAnnotation == NullableAnnotation.Ignored);
if (NullableAnnotation.IsAnnotated() || newTypeWithModifiers.NullableAnnotation.IsAnnotated())
{
newAnnotation = NullableAnnotation.Annotated;
}
else if (IsIndexedTypeParameter(newTypeWithModifiers.Type))
else if (newTypeWithModifiers.NullableAnnotation == NullableAnnotation.Ignored)
{
newAnnotation = NullableAnnotation;
}
......
......@@ -18,10 +18,10 @@ public class IndexedTypeParameterTests
[Fact]
public void TestTake()
{
var zero = IndexedTypeParameterSymbol.Take(0);
var zero = IndexedTypeParameterSymbol.TakeSymbols(0);
Assert.Equal(0, zero.Length);
var five = IndexedTypeParameterSymbol.Take(5);
var five = IndexedTypeParameterSymbol.TakeSymbols(5);
Assert.Equal(5, five.Length);
Assert.Equal(five[0], IndexedTypeParameterSymbol.GetTypeParameter(0));
Assert.Equal(five[1], IndexedTypeParameterSymbol.GetTypeParameter(1));
......@@ -29,14 +29,14 @@ public void TestTake()
Assert.Equal(five[3], IndexedTypeParameterSymbol.GetTypeParameter(3));
Assert.Equal(five[4], IndexedTypeParameterSymbol.GetTypeParameter(4));
var fifty = IndexedTypeParameterSymbol.Take(50);
var fifty = IndexedTypeParameterSymbol.TakeSymbols(50);
Assert.Equal(50, fifty.Length);
// prove they are all unique
var set = new HashSet<TypeParameterSymbol>(fifty);
Assert.Equal(50, set.Count);
var fiveHundred = IndexedTypeParameterSymbol.Take(500);
var fiveHundred = IndexedTypeParameterSymbol.TakeSymbols(500);
Assert.Equal(500, fiveHundred.Length);
}
}
......
......@@ -203,7 +203,7 @@ public interface ISymbol : IEquatable<ISymbol?>
ISymbol OriginalDefinition { get; }
void Accept(SymbolVisitor visitor);
TResult Accept<TResult>(SymbolVisitor<TResult> visitor);
TResult? Accept<TResult>(SymbolVisitor<TResult> visitor);
/// <summary>
/// Returns the Documentation Comment ID for the symbol, or null if the symbol doesn't
......
......@@ -19,7 +19,7 @@ private ExpressionSyntaxGeneratorVisitor()
}
public override ExpressionSyntax DefaultVisit(ISymbol symbol)
=> symbol.Accept(TypeSyntaxGeneratorVisitor.Create());
=> symbol.Accept(TypeSyntaxGeneratorVisitor.Create())!;
private static TExpressionSyntax AddInformationTo<TExpressionSyntax>(TExpressionSyntax syntax, ISymbol symbol)
where TExpressionSyntax : ExpressionSyntax
......@@ -48,7 +48,7 @@ public override ExpressionSyntax VisitNamedType(INamedTypeSymbol symbol)
}
else
{
var container = symbol.ContainingType.Accept(this);
var container = symbol.ContainingType.Accept(this)!;
return CreateMemberAccessExpression(symbol, container, simpleNameSyntax);
}
}
......@@ -66,7 +66,7 @@ public override ExpressionSyntax VisitNamedType(INamedTypeSymbol symbol)
}
else
{
var container = symbol.ContainingNamespace.Accept(this);
var container = symbol.ContainingNamespace.Accept(this)!;
return CreateMemberAccessExpression(symbol, container, simpleNameSyntax);
}
}
......@@ -91,7 +91,7 @@ public override ExpressionSyntax VisitNamespace(INamespaceSymbol symbol)
}
else
{
var container = symbol.ContainingNamespace.Accept(this);
var container = symbol.ContainingNamespace.Accept(this)!;
return CreateMemberAccessExpression(symbol, container, syntax);
}
}
......
......@@ -293,7 +293,7 @@ public override TypeSyntax VisitNamedType(INamedTypeSymbol symbol)
}
else
{
var container = symbol.ContainingNamespace.Accept(this);
var container = symbol.ContainingNamespace.Accept(this)!;
typeSyntax = AddInformationTo(SyntaxFactory.QualifiedName(
(NameSyntax)container,
simpleNameSyntax), symbol);
......@@ -323,7 +323,7 @@ public override TypeSyntax VisitNamespace(INamespaceSymbol symbol)
}
else
{
var container = symbol.ContainingNamespace.Accept(this);
var container = symbol.ContainingNamespace.Accept(this)!;
return AddInformationTo(SyntaxFactory.QualifiedName(
(NameSyntax)container,
syntax), symbol);
......
......@@ -22,7 +22,7 @@ internal static partial class ITypeSymbolExtensions
public static ExpressionSyntax GenerateExpressionSyntax(
this ITypeSymbol typeSymbol)
{
return typeSymbol.Accept(ExpressionSyntaxGeneratorVisitor.Instance).WithAdditionalAnnotations(Simplifier.Annotation);
return typeSymbol.Accept(ExpressionSyntaxGeneratorVisitor.Instance)!.WithAdditionalAnnotations(Simplifier.Annotation);
}
public static NameSyntax GenerateNameSyntax(
......@@ -48,7 +48,7 @@ internal static partial class ITypeSymbolExtensions
return SyntaxFactory.IdentifierName("var");
}
var syntax = symbol.Accept(TypeSyntaxGeneratorVisitor.Create(nameSyntax))
var syntax = symbol.Accept(TypeSyntaxGeneratorVisitor.Create(nameSyntax))!
.WithAdditionalAnnotations(Simplifier.Annotation);
if (!allowVar)
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册