Skip to content

Commit

Permalink
Merge pull request #33648 from gafter/nullable-states
Browse files Browse the repository at this point in the history
Revise NullableWalker to use a two-state solution
  • Loading branch information
jcouv authored Mar 1, 2019
2 parents 5d70e9b + 2b6ee0d commit 2566316
Show file tree
Hide file tree
Showing 87 changed files with 4,112 additions and 4,542 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ internal struct NamespaceOrTypeOrAliasSymbolWithAnnotations

private NamespaceOrTypeOrAliasSymbolWithAnnotations(TypeSymbolWithAnnotations type)
{
Debug.Assert(!type.IsNull);
Debug.Assert(type.HasType);
_type = type;
_symbol = null;
_isNullableEnabled = false; // Not meaningful for a TypeSymbolWithAnnotations, it already baked the fact into its content.
Expand All @@ -31,10 +31,10 @@ private NamespaceOrTypeOrAliasSymbolWithAnnotations(Symbol symbol, bool isNullab

internal TypeSymbolWithAnnotations Type => _type;
internal Symbol Symbol => _symbol ?? Type.TypeSymbol;
internal bool IsType => !_type.IsNull;
internal bool IsType => !_type.IsDefault;
internal bool IsAlias => _symbol?.Kind == SymbolKind.Alias;
internal NamespaceOrTypeSymbol NamespaceOrTypeSymbol => Symbol as NamespaceOrTypeSymbol;
internal bool IsDefault => _type.IsNull && _symbol is null;
internal bool IsDefault => !_type.HasType && _symbol is null;

internal bool IsNullableEnabled
{
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs
Original file line number Diff line number Diff line change
Expand Up @@ -954,7 +954,7 @@ private bool CheckPropertyValueKind(SyntaxNode node, BoundExpression expr, BindV
// change from Dev10 which reports this error for struct types only,
// not for type parameters constrained to "struct".

Debug.Assert(!propertySymbol.Type.IsNull);
Debug.Assert(propertySymbol.Type.HasType);
Error(diagnostics, ErrorCode.ERR_ReturnNotLValue, expr.Syntax, propertySymbol);
}
else
Expand Down Expand Up @@ -1665,7 +1665,7 @@ private static void ReportReadOnlyFieldError(FieldSymbol field, SyntaxNode node,
{
Debug.Assert((object)field != null);
Debug.Assert(RequiresAssignableVariable(kind));
Debug.Assert(!field.Type.IsNull);
Debug.Assert(field.Type.HasType);

// It's clearer to say that the address can't be taken than to say that the field can't be modified
// (even though the latter message gives more explanation of why).
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Attributes.cs
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ private void ValidateTypeForAttributeParameters(ImmutableArray<ParameterSymbol>
foreach (var parameter in parameters)
{
var paramType = parameter.Type;
Debug.Assert(!paramType.IsNull);
Debug.Assert(paramType.HasType);

if (!paramType.TypeSymbol.IsValidAttributeParameterType(Compilation))
{
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ protected BoundExpression CreateConversion(
sourceTuple.Type,
sourceTuple.Arguments,
sourceTuple.Type, // same type to keep original element names
sourceTuple.HasErrors);
sourceTuple.HasErrors).WithSuppression(sourceTuple.IsSuppressed);
}

// We need to preserve any conversion that changes the type (even identity conversions, like object->dynamic),
Expand Down
8 changes: 4 additions & 4 deletions src/Compilers/CSharp/Portable/Binder/Binder_Deconstruct.cs
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,7 @@ private static TypeSymbol MakeMergedTupleType(ArrayBuilder<DeconstructionVariabl
locationsBuilder.Add(element.Syntax.Location);
}

if (typesBuilder.Any(t => t.IsNull))
if (typesBuilder.Any(t => !t.HasType))
{
typesBuilder.Free();
locationsBuilder.Free();
Expand Down Expand Up @@ -721,7 +721,7 @@ private DeconstructionVariable BindDeconstructionVariables(
bool isConst = false;
AliasSymbol alias;
var declType = BindVariableType(component.Designation, diagnostics, component.Type, ref isConst, out isVar, out alias);
Debug.Assert(isVar == declType.IsNull);
Debug.Assert(isVar == !declType.HasType);
if (component.Designation.Kind() == SyntaxKind.ParenthesizedVariableDesignation && !isVar)
{
// An explicit is not allowed with a parenthesized designation
Expand Down Expand Up @@ -819,7 +819,7 @@ private BoundExpression BindDeconstructionVariable(
// might own nested scope.
var hasErrors = localSymbol.ScopeBinder.ValidateDeclarationNameConflictsInScope(localSymbol, diagnostics);

if (!declType.IsNull)
if (declType.HasType)
{
return new BoundLocal(syntax, localSymbol, BoundLocalDeclarationKind.WithExplicitType, constantValueOpt: null, isNullableUnknown: false, type: declType.TypeSymbol, hasErrors: hasErrors);
}
Expand All @@ -839,7 +839,7 @@ private BoundExpression BindDeconstructionVariable(
BoundThisReference receiver = ThisReference(designation, this.ContainingType, hasErrors: false,
wasCompilerGenerated: true);

if (!declType.IsNull)
if (declType.HasType)
{
var fieldType = field.GetFieldType(this.FieldsBeingBound);
Debug.Assert(TypeSymbol.Equals(declType.TypeSymbol, fieldType.TypeSymbol, TypeCompareKind.ConsiderEverything2));
Expand Down
10 changes: 5 additions & 5 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -692,7 +692,7 @@ private BoundExpression BindDeclarationExpression(DeclarationExpressionSyntax no
/// </summary>
private BoundExpression BindDeclarationVariables(TypeSymbolWithAnnotations declType, VariableDesignationSyntax node, CSharpSyntaxNode syntax, DiagnosticBag diagnostics)
{
declType = declType.IsNull ? TypeSymbolWithAnnotations.Create(CreateErrorType("var")) : declType;
declType = declType.HasType ? declType : TypeSymbolWithAnnotations.Create(CreateErrorType("var"));
switch (node.Kind())
{
case SyntaxKind.SingleVariableDesignation:
Expand Down Expand Up @@ -799,7 +799,7 @@ private BoundExpression BindTupleExpression(TupleExpressionSyntax node, Diagnost
var elementType = TypeSymbolWithAnnotations.Create(boundArgument.Type);
elementTypes.Add(elementType);

if (elementType.IsNull)
if (!elementType.HasType)
{
hasNaturalType = false;
}
Expand Down Expand Up @@ -2237,7 +2237,7 @@ private void GenerateExplicitConversionErrorsForTupleLiteralArguments(
/// </summary>
private BoundExpression BindExplicitNullableCastFromNonNullable(ExpressionSyntax node, BoundExpression operand, TypeSymbolWithAnnotations targetType, DiagnosticBag diagnostics)
{
Debug.Assert(!targetType.IsNull && targetType.IsNullableType());
Debug.Assert(targetType.HasType && targetType.IsNullableType());
Debug.Assert((object)operand.Type != null && !operand.Type.IsNullableType());

// Section 6.2.3 of the spec only applies when the non-null version of the types involved have a
Expand Down Expand Up @@ -2491,7 +2491,7 @@ private BoundExpression BindOutDeclarationArgument(DeclarationExpressionSyntax d
bool isConst = false;
AliasSymbol alias;
var declType = BindVariableType(designation, diagnostics, typeSyntax, ref isConst, out isVar, out alias);
Debug.Assert(isVar == declType.IsNull);
Debug.Assert(isVar != declType.HasType);

return new BoundDiscardExpression(declarationExpression, declType.TypeSymbol);
}
Expand Down Expand Up @@ -2744,7 +2744,7 @@ private void CoerceArguments<TMember>(
else if (argument.Kind == BoundKind.DiscardExpression && !argument.HasExpressionType())
{
TypeSymbolWithAnnotations parameterType = GetCorrespondingParameterType(ref result, parameters, arg);
Debug.Assert(!parameterType.IsNull);
Debug.Assert(parameterType.HasType);
arguments[arg] = ((BoundDiscardExpression)argument).SetInferredType(parameterType);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Lambda.cs
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ private UnboundLambda BindAnonymousFunction(CSharpSyntaxNode syntax, DiagnosticB
foreach (var type in types)
{
// UNDONE: Where do we report improper use of pointer types?
if (!type.IsNull && type.IsStatic)
if (type.HasType && type.IsStatic)
{
Error(diagnostics, ErrorCode.ERR_ParameterIsStaticClass, syntax, type.TypeSymbol);
}
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Patterns.cs
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ private BoundTypeExpression BindPatternType(
Debug.Assert(inputType != (object)null);
Debug.Assert(!typeSyntax.IsVar); // if the syntax had `var`, it would have been parsed as a var pattern.
TypeSymbolWithAnnotations declType = BindType(typeSyntax, diagnostics, out AliasSymbol aliasOpt);
Debug.Assert(!declType.IsNull);
Debug.Assert(declType.HasType);
Debug.Assert(typeSyntax.Kind() != SyntaxKind.NullableType); // the syntax does not permit nullable annotations
BoundTypeExpression boundDeclType = new BoundTypeExpression(typeSyntax, aliasOpt, inferredType: false, type: declType.TypeSymbol);
hasErrors |= CheckValidPatternType(typeSyntax, inputType, declType.TypeSymbol, patternTypeWasInSource: true, diagnostics: diagnostics);
Expand Down
10 changes: 5 additions & 5 deletions src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs
Original file line number Diff line number Diff line change
Expand Up @@ -728,7 +728,7 @@ declarationNode is VariableDesignationSyntax ||
// we want to treat the declaration as an explicitly typed declaration.

TypeSymbolWithAnnotations declType = BindTypeOrVarKeyword(typeSyntax.SkipRef(out _), diagnostics, out isVar, out alias);
Debug.Assert(!declType.IsNull || isVar);
Debug.Assert(declType.HasType || isVar);

if (isVar)
{
Expand Down Expand Up @@ -915,7 +915,7 @@ protected BoundLocalDeclaration BindVariableDeclaration(
CSharpSyntaxNode associatedSyntaxNode = null)
{
Debug.Assert(declarator != null);
Debug.Assert(!declTypeOpt.IsNull || isVar);
Debug.Assert(declTypeOpt.HasType || isVar);
Debug.Assert(typeSyntax != null);

var localDiagnostics = DiagnosticBag.GetInstance();
Expand Down Expand Up @@ -999,7 +999,7 @@ protected BoundLocalDeclaration BindVariableDeclaration(
}
}

Debug.Assert(!declTypeOpt.IsNull);
Debug.Assert(declTypeOpt.HasType);

if (kind == LocalDeclarationKind.FixedVariable)
{
Expand Down Expand Up @@ -2344,7 +2344,7 @@ internal BoundStatement BindForOrUsingOrFixedDeclarations(VariableDeclarationSyn
bool isVar;
TypeSymbolWithAnnotations declType = BindTypeOrVarKeyword(typeSyntax, diagnostics, out isVar, out alias);

Debug.Assert(!declType.IsNull || isVar);
Debug.Assert(declType.HasType || isVar);

var variables = nodeOpt.Variables;
int count = variables.Count;
Expand Down Expand Up @@ -2495,7 +2495,7 @@ protected virtual TypeSymbol GetCurrentReturnType(out RefKind refKind)

TypeSymbolWithAnnotations returnType = symbol.ReturnType;

if (returnType.IsNull || (object)returnType.TypeSymbol == LambdaSymbol.ReturnTypeIsBeingInferred)
if ((object)returnType.TypeSymbol == LambdaSymbol.ReturnTypeIsBeingInferred)
{
return null;
}
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Symbols.cs
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,7 @@ void reportNullableReferenceTypesIfNeeded(SyntaxToken questionToken, TypeSymbolW
var location = questionToken.GetLocation();

// Inside a method body or other executable code, we can question IsValueType without causing cycles.
if (!typeArgument.IsNull && !ShouldCheckConstraints)
if (typeArgument.HasType && !ShouldCheckConstraints)
{
LazyMissingNonNullTypesContextDiagnosticInfo.AddAll(isNullableEnabled, typeArgument, location, diagnostics);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ private ForEachEnumeratorInfo(
BinderFlags location)
{
Debug.Assert((object)collectionType != null, "Field 'collectionType' cannot be null");
Debug.Assert(!elementType.IsNull, "Field 'elementType' cannot be null");
Debug.Assert(elementType.HasType, "Field 'elementType' cannot be null");
Debug.Assert((object)getEnumeratorMethod != null, "Field 'getEnumeratorMethod' cannot be null");
Debug.Assert((object)currentPropertyGetter != null, "Field 'currentPropertyGetter' cannot be null");
Debug.Assert((object)moveNextMethod != null, "Field 'moveNextMethod' cannot be null");
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/Binder/ForEachLoopBinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -244,11 +244,11 @@ private BoundForEachStatement BindForEachPartsWorker(DiagnosticBag diagnostics,

if (isVar)
{
declType = inferredType.IsNull ? TypeSymbolWithAnnotations.Create(CreateErrorType("var")) : inferredType;
declType = inferredType.HasType ? inferredType : TypeSymbolWithAnnotations.Create(CreateErrorType("var"));
}
else
{
Debug.Assert(!declType.IsNull);
Debug.Assert(declType.HasType);
}

iterationVariableType = declType.TypeSymbol;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public static NullableAnnotation GetNullableAnnotation(ArrayBuilder<TypeSymbolWi
NullableAnnotation result = NullableAnnotation.NotAnnotated;
foreach (var type in types)
{
Debug.Assert(!type.IsNull);
Debug.Assert(type.HasType);
Debug.Assert(type.Equals(types[0], TypeCompareKind.AllIgnoreOptions));
// This uses the covariant merging rules.
result = result.JoinForFixingLowerBounds(type.AsSpeakable().NullableAnnotation);
Expand All @@ -24,6 +24,18 @@ public static NullableAnnotation GetNullableAnnotation(ArrayBuilder<TypeSymbolWi
return result;
}

public static NullableAnnotation GetNullableAnnotation(ArrayBuilder<TypeWithState> types)
{
ArrayBuilder<TypeSymbolWithAnnotations> builder = ArrayBuilder<TypeSymbolWithAnnotations>.GetInstance();
foreach (var type in types)
{
builder.Add(type.ToTypeSymbolWithAnnotations());
}
var result = GetNullableAnnotation(builder);
builder.Free();
return result;
}

/// <remarks>
/// This method finds the best common type of a set of expressions as per section 7.5.2.14 of the specification.
/// NOTE: If some or all of the expressions have error types, we return error type as the inference result.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ internal static Conversion GetTrivialConversion(ConversionKind kind)
return new Conversion(kind);
}

internal static Conversion UnsetConversion => new Conversion(ConversionKind.UnsetConversionKind);
internal static Conversion NoConversion => new Conversion(ConversionKind.NoConversion);
internal static Conversion Identity => new Conversion(ConversionKind.Identity);
internal static Conversion ImplicitConstant => new Conversion(ConversionKind.ImplicitConstant);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ namespace Microsoft.CodeAnalysis.CSharp
{
internal enum ConversionKind : byte
{
UnsetConversionKind = 0,
NoConversion,
Identity,
ImplicitNumeric,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1399,12 +1399,12 @@ internal bool HasTopLevelNullabilityIdentityConversion(TypeSymbolWithAnnotations
return true;
}

if (source.IsPossiblyNullableReferenceTypeTypeParameter() && !destination.IsPossiblyNullableReferenceTypeTypeParameter())
if (source.IsPossiblyNullableTypeTypeParameter() && !destination.IsPossiblyNullableTypeTypeParameter())
{
return destination.NullableAnnotation.IsAnyNullable();
}

if (destination.IsPossiblyNullableReferenceTypeTypeParameter() && !source.IsPossiblyNullableReferenceTypeTypeParameter())
if (destination.IsPossiblyNullableTypeTypeParameter() && !source.IsPossiblyNullableTypeTypeParameter())
{
return source.NullableAnnotation.IsAnyNullable();
}
Expand All @@ -1431,7 +1431,7 @@ internal bool HasTopLevelNullabilityImplicitConversion(TypeSymbolWithAnnotations
return true;
}

if (source.IsPossiblyNullableReferenceTypeTypeParameter() && !destination.IsPossiblyNullableReferenceTypeTypeParameter())
if (source.IsPossiblyNullableTypeTypeParameter() && !destination.IsPossiblyNullableTypeTypeParameter())
{
return false;
}
Expand Down
Loading

0 comments on commit 2566316

Please sign in to comment.