Skip to content

Commit

Permalink
Include nullability in CheckConstraints (#28959)
Browse files Browse the repository at this point in the history
  • Loading branch information
cston authored Aug 3, 2018
1 parent 14efbde commit 0ece26c
Show file tree
Hide file tree
Showing 30 changed files with 736 additions and 85 deletions.
3 changes: 2 additions & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Symbols.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1166,7 +1166,8 @@ private NamedTypeSymbol ConstructNamedType(

if (ShouldCheckConstraints)
{
type.CheckConstraintsForNonTuple(this.Conversions, typeSyntax, typeArgumentsSyntax, this.Compilation, basesBeingResolved, diagnostics);
bool includeNullability = Compilation.IsFeatureEnabled(MessageID.IDS_FeatureStaticNullChecking);
type.CheckConstraintsForNonTuple(this.Conversions.WithNullability(includeNullability), typeSyntax, typeArgumentsSyntax, this.Compilation, basesBeingResolved, diagnostics);
}

type = (NamedTypeSymbol)TupleTypeSymbol.TransformToTupleIfCompatible(type);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,26 +13,27 @@ internal sealed class Conversions : ConversionsBase
private readonly Binder _binder;

public Conversions(Binder binder)
: this(binder, currentRecursionDepth: 0, includeNullability: false)
: this(binder, currentRecursionDepth: 0, includeNullability: false, otherNullabilityOpt: null)
{
}

private Conversions(Binder binder, int currentRecursionDepth, bool includeNullability)
: base(binder.Compilation.Assembly.CorLibrary, currentRecursionDepth, includeNullability)
private Conversions(Binder binder, int currentRecursionDepth, bool includeNullability, Conversions otherNullabilityOpt)
: base(binder.Compilation.Assembly.CorLibrary, currentRecursionDepth, includeNullability, otherNullabilityOpt)
{
_binder = binder;
}

protected override ConversionsBase CreateInstance(int currentRecursionDepth)
{
return new Conversions(_binder, currentRecursionDepth, IncludeNullability);
return new Conversions(_binder, currentRecursionDepth, IncludeNullability, otherNullabilityOpt: null);
}

private CSharpCompilation Compilation { get { return _binder.Compilation; } }

internal Conversions WithNullability(bool includeNullability)
protected override ConversionsBase WithNullabilityCore(bool includeNullability)
{
return (IncludeNullability == includeNullability) ? this : new Conversions(_binder, currentRecursionDepth, includeNullability);
Debug.Assert(IncludeNullability != includeNullability);
return new Conversions(_binder, currentRecursionDepth, includeNullability, this);
}

public override Conversion GetMethodGroupConversion(BoundMethodGroup source, TypeSymbol destination, ref HashSet<DiagnosticInfo> useSiteDiagnostics)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,45 @@ internal abstract partial class ConversionsBase

internal readonly bool IncludeNullability;

protected ConversionsBase(AssemblySymbol corLibrary, int currentRecursionDepth, bool includeNullability)
/// <summary>
/// An optional clone of this instance with distinct IncludeNullability.
/// Used to avoid unnecessary allocations when calling WithNullability() repeatedly.
/// </summary>
private ConversionsBase _lazyOtherNullability;

protected ConversionsBase(AssemblySymbol corLibrary, int currentRecursionDepth, bool includeNullability, ConversionsBase otherNullabilityOpt)
{
Debug.Assert((object)corLibrary != null);
Debug.Assert(otherNullabilityOpt == null || includeNullability != otherNullabilityOpt.IncludeNullability);
Debug.Assert(otherNullabilityOpt == null || currentRecursionDepth == otherNullabilityOpt.currentRecursionDepth);

this.corLibrary = corLibrary;
this.currentRecursionDepth = currentRecursionDepth;
IncludeNullability = includeNullability;
_lazyOtherNullability = otherNullabilityOpt;
}

/// <summary>
/// Returns this instance if includeNullability is correct, and returns a
/// cached clone of this instance with distinct includeNullability otherwise.
/// </summary>
internal ConversionsBase WithNullability(bool includeNullability)
{
if (IncludeNullability == includeNullability)
{
return this;
}
if (_lazyOtherNullability == null)
{
_lazyOtherNullability = WithNullabilityCore(includeNullability);
}
Debug.Assert(_lazyOtherNullability.IncludeNullability == includeNullability);
Debug.Assert(_lazyOtherNullability._lazyOtherNullability == this);
return _lazyOtherNullability;
}

protected abstract ConversionsBase WithNullabilityCore(bool includeNullability);

public abstract Conversion GetMethodGroupConversion(BoundMethodGroup source, TypeSymbol destination, ref HashSet<DiagnosticInfo> useSiteDiagnostics);

public abstract Conversion GetStackAllocConversion(BoundStackAllocArrayCreation sourceExpression, TypeSymbol destination, ref HashSet<DiagnosticInfo> useSiteDiagnostics);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,31 @@
using Roslyn.Utilities;
using System;
using System.Collections.Generic;
using System.Diagnostics;

namespace Microsoft.CodeAnalysis.CSharp
{
internal sealed class TypeConversions : ConversionsBase
{
public TypeConversions(AssemblySymbol corLibrary)
: this(corLibrary, currentRecursionDepth: 0, includeNullability: false)
: this(corLibrary, currentRecursionDepth: 0, includeNullability: false, otherNullabilityOpt: null)
{
}

private TypeConversions(AssemblySymbol corLibrary, int currentRecursionDepth, bool includeNullability)
: base(corLibrary, currentRecursionDepth, includeNullability)
private TypeConversions(AssemblySymbol corLibrary, int currentRecursionDepth, bool includeNullability, TypeConversions otherNullabilityOpt)
: base(corLibrary, currentRecursionDepth, includeNullability, otherNullabilityOpt)
{
}

protected override ConversionsBase CreateInstance(int currentRecursionDepth)
{
return new TypeConversions(this.corLibrary, currentRecursionDepth, IncludeNullability);
return new TypeConversions(this.corLibrary, currentRecursionDepth, IncludeNullability, otherNullabilityOpt: null);
}

protected override ConversionsBase WithNullabilityCore(bool includeNullability)
{
Debug.Assert(IncludeNullability != includeNullability);
return new TypeConversions(corLibrary, currentRecursionDepth, includeNullability, this);
}

public override Conversion GetMethodGroupConversion(BoundMethodGroup source, TypeSymbol destination, ref HashSet<DiagnosticInfo> useSiteDiagnostics)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,7 @@ private bool FailsConstraintChecks(MethodSymbol method, out ArrayBuilder<TypePar
this.Conversions,
this.Compilation,
diagnosticsBuilder,
warningsBuilderOpt: null,
ref useSiteDiagnosticsBuilder);

if (!constraintsSatisfied)
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/BoundTree/UnboundLambda.cs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public TypeSymbolWithAnnotations GetInferredReturnType(ref HashSet<DiagnosticInf
var diagnostics = DiagnosticBag.GetInstance();
var delegateType = Type.GetDelegateType();
var compilation = Binder.Compilation;
var conversions = Binder.Conversions.WithNullability(includeNullability: true);
var conversions = (Conversions)Binder.Conversions.WithNullability(includeNullability: true);
NullableWalker.Analyze(compilation, lambda: this, diagnostics, delegateInvokeMethod: delegateType?.DelegateInvokeMethod, returnTypes: returnTypes, initialState: nullableState);
diagnostics.Free();
var inferredReturnType = InferReturnType(returnTypes, compilation, conversions, delegateType, Symbol.IsAsync);
Expand Down
18 changes: 18 additions & 0 deletions src/Compilers/CSharp/Portable/CSharpResources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -5363,6 +5363,12 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="ERR_AnnotationDisallowedInObjectCreation" xml:space="preserve">
<value>Cannot use a nullable reference type in object creation.</value>
</data>
<data name="WRN_NullabilityMismatchInTypeParameterConstraint" xml:space="preserve">
<value>The type '{3}' cannot be used as type parameter '{2}' in the generic type or method '{0}'. Nullability of type argument '{3}' doesn't match constraint type '{1}'.</value>
</data>
<data name="WRN_NullabilityMismatchInTypeParameterConstraint_Title" xml:space="preserve">
<value>The type cannot be used as type parameter in the generic type or method. Nullability of type argument doesn't match constraint type.</value>
</data>
<data name="ERR_ExplicitNullableAttribute" xml:space="preserve">
<value>Explicit application of 'System.Runtime.CompilerServices.NullableAttribute' is not allowed.</value>
</data>
Expand Down
1 change: 1 addition & 0 deletions src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1615,6 +1615,7 @@ internal enum ErrorCode
ERR_AnnotationDisallowedInObjectCreation = 8628,
WRN_MissingNonNullTypesContext = 8629,
ERR_NonNullTypesNotAvailable = 8630,
WRN_NullabilityMismatchInTypeParameterConstraint = 8631,
}
// Note: you will need to re-generate compiler code after adding warnings (build\scripts\generate-compiler-code.cmd)
}
1 change: 1 addition & 0 deletions src/Compilers/CSharp/Portable/Errors/ErrorFacts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,7 @@ internal static int GetWarningLevel(ErrorCode code)
case ErrorCode.WRN_NullabilityMismatchInParameterTypeOfTargetDelegate:
case ErrorCode.WRN_NullAsNonNullable:
case ErrorCode.WRN_NoBestNullabilityConditionalExpression:
case ErrorCode.WRN_NullabilityMismatchInTypeParameterConstraint:
case ErrorCode.WRN_Experimental:
case ErrorCode.WRN_AttributesOnBackingFieldsNotAvailable:
case ErrorCode.WRN_TupleBinopLiteralNameMismatch:
Expand Down
38 changes: 33 additions & 5 deletions src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ private NullableWalker(
// If so, are we interested in an InMethodBinder specifically?
_binder = compilation.GetBinderFactory(node.SyntaxTree).GetBinder(node.Syntax);
Debug.Assert(!_binder.Conversions.IncludeNullability);
_conversions = _binder.Conversions.WithNullability(true);
_conversions = (Conversions)_binder.Conversions.WithNullability(true);
_useMethodSignatureReturnType = (object)methodSignatureOpt != null && useMethodSignatureReturnType;
_useMethodSignatureParameterTypes = (object)methodSignatureOpt != null && useMethodSignatureParameterTypes;
_methodSignatureOpt = methodSignatureOpt;
Expand Down Expand Up @@ -2047,10 +2047,18 @@ private MethodSymbol VisitArguments(
// We do a first pass to work through the arguments without making any assumptions
ImmutableArray<TypeSymbolWithAnnotations> results = VisitArgumentsEvaluate(arguments, refKindsOpt);

if ((object)method != null && method.IsGenericMethod && HasImplicitTypeArguments(node))
if ((object)method != null && method.IsGenericMethod)
{
method = InferMethodTypeArguments((BoundCall)node, method, GetArgumentsForMethodTypeInference(arguments, results));
parameters = method.Parameters;
if (HasImplicitTypeArguments(node))
{
method = InferMethodTypeArguments((BoundCall)node, method, GetArgumentsForMethodTypeInference(arguments, results));
parameters = method.Parameters;
}
if (!method.IsDefinition)
{
var syntax = node.Syntax;
CheckMethodConstraints((syntax as InvocationExpressionSyntax)?.Expression ?? syntax, method);
}
}

// PROTOTYPE(NullableReferenceTypes): Can we handle some error cases?
Expand Down Expand Up @@ -2489,7 +2497,6 @@ private MethodSymbol InferMethodTypeArguments(BoundCall node, MethodSymbol metho
ref useSiteDiagnostics);
if (result.Success)
{
// PROTOTYPE(NullableReferenceTypes): Check constraints
return definition.Construct(result.InferredTypeArguments);
}
return method;
Expand Down Expand Up @@ -2532,6 +2539,27 @@ BoundExpression getArgumentForMethodTypeInference(BoundExpression argument, Type
}
}

private void CheckMethodConstraints(SyntaxNode syntax, MethodSymbol method)
{
var diagnosticsBuilder = ArrayBuilder<TypeParameterDiagnosticInfo>.GetInstance();
var warningsBuilder = ArrayBuilder<TypeParameterDiagnosticInfo>.GetInstance();
ArrayBuilder<TypeParameterDiagnosticInfo> useSiteDiagnosticsBuilder = null;
ConstraintsHelper.CheckMethodConstraints(
method,
_conversions,
compilation,
diagnosticsBuilder,
warningsBuilder,
ref useSiteDiagnosticsBuilder);
foreach (var pair in warningsBuilder)
{
Diagnostics.Add(pair.DiagnosticInfo, syntax.Location);
}
useSiteDiagnosticsBuilder?.Free();
warningsBuilder.Free();
diagnosticsBuilder.Free();
}

private void ReplayReadsAndWrites(LocalFunctionSymbol localFunc,
SyntaxNode syntax,
bool writes)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ public static bool IsWarning(ErrorCode code)
case ErrorCode.WRN_NullAsNonNullable:
case ErrorCode.WRN_NoBestNullabilityConditionalExpression:
case ErrorCode.WRN_MissingNonNullTypesContext:
case ErrorCode.WRN_NullabilityMismatchInTypeParameterConstraint:
return true;
default:
return false;
Expand Down
Loading

0 comments on commit 0ece26c

Please sign in to comment.