Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revise NullableWalker to use a two-state solution #33648

Merged
merged 13 commits into from
Mar 1, 2019
Merged
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);
gafter marked this conversation as resolved.
Show resolved Hide resolved
}

// 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,
Copy link
Member

@jaredpar jaredpar Feb 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why make this the 0 value here vs. adding it to the end of the enum? #ByDesign

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it represents the value of a ConversionKind field that hasn't yet been set. It must be 0 for that to work.


In reply to: 260403775 [](ancestors = 260403775)

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