Skip to content

Commit

Permalink
Correct nullability analysis in conditional access (#34973)
Browse files Browse the repository at this point in the history 
Fixes #29956

Also introduce a helper `TypeSymbol.IsVoidType()`
  • Loading branch information
Neal Gafter committed Apr 30, 2019
1 parent 4c6cb7a commit bd5da4b
Show file tree
Hide file tree
Showing 52 changed files with 286 additions and 142 deletions.
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_AnonymousTypes.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ private TypeSymbol GetAnonymousTypeFieldType(BoundExpression expression, CSharpS
{
if (expression.HasExpressionType())
{
if (expressionType.SpecialType == SpecialType.System_Void)
if (expressionType.IsVoidType())
{
errorArg = expressionType;
expressionType = CreateErrorType(SyntaxFacts.GetText(SyntaxKind.VoidKeyword));
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/Binder/Binder_Await.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ private bool CouldBeAwaited(BoundExpression expression)
var type = expression.Type;
if (((object)type == null) ||
type.IsDynamic() ||
(type.SpecialType == SpecialType.System_Void))
(type.IsVoidType()))
{
return false;
}
Expand Down Expand Up @@ -289,7 +289,7 @@ private static bool ValidateAwaitedExpression(BoundExpression expression, Syntax
/// </remarks>
private bool GetGetAwaiterMethod(BoundExpression expression, SyntaxNode node, DiagnosticBag diagnostics, out MethodSymbol getAwaiterMethod, out BoundExpression getAwaiterCall)
{
if (expression.Type.SpecialType == SpecialType.System_Void)
if (expression.Type.IsVoidType())
{
Error(diagnostics, ErrorCode.ERR_BadAwaitArgVoidCall, node);
getAwaiterMethod = null;
Expand Down
16 changes: 9 additions & 7 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2960,7 +2960,7 @@ private BoundExpression BindImplicitArrayCreationExpression(ImplicitArrayCreatio
TypeSymbol bestType = BestTypeInferrer.InferBestType(boundInitializerExpressions, this.Conversions, ref useSiteDiagnostics);
diagnostics.Add(node, useSiteDiagnostics);

if ((object)bestType == null || bestType.SpecialType == SpecialType.System_Void) // Dev10 also reports ERR_ImplicitlyTypedArrayNoBestType for void.
if ((object)bestType == null || bestType.IsVoidType()) // Dev10 also reports ERR_ImplicitlyTypedArrayNoBestType for void.
{
Error(diagnostics, ErrorCode.ERR_ImplicitlyTypedArrayNoBestType, node);
bestType = CreateErrorType();
Expand All @@ -2987,7 +2987,7 @@ private BoundExpression BindImplicitStackAllocArrayCreationExpression(ImplicitSt
TypeSymbol bestType = BestTypeInferrer.InferBestType(boundInitializerExpressions, this.Conversions, ref useSiteDiagnostics);
diagnostics.Add(node, useSiteDiagnostics);

if ((object)bestType == null || bestType.SpecialType == SpecialType.System_Void)
if ((object)bestType == null || bestType.IsVoidType())
{
Error(diagnostics, ErrorCode.ERR_ImplicitlyTypedArrayNoBestType, node);
bestType = CreateErrorType();
Expand Down Expand Up @@ -3638,7 +3638,7 @@ private BoundExpression BindConstructorInitializerCore(
try
{
TypeSymbol constructorReturnType = constructor.ReturnType;
Debug.Assert(constructorReturnType.SpecialType == SpecialType.System_Void); //true of all constructors
Debug.Assert(constructorReturnType.IsVoidType()); //true of all constructors

// Get the bound arguments and the argument names.
// : this(__arglist()) is legal
Expand Down Expand Up @@ -5657,7 +5657,7 @@ private BoundExpression BindMemberAccessWithBoundLeft(
}

// No member accesses on void
if ((object)leftType != null && leftType.SpecialType == SpecialType.System_Void)
if ((object)leftType != null && leftType.IsVoidType())
{
diagnostics.Add(ErrorCode.ERR_BadUnaryOp, operatorToken.GetLocation(), SyntaxFacts.GetText(operatorToken.Kind()), leftType);
return BadExpression(node, boundLeft);
Expand Down Expand Up @@ -7151,7 +7151,7 @@ private BoundExpression BindPointerElementAccess(ExpressionSyntax node, BoundExp
CheckOverflowAtRuntime, pointedAtType, hasErrors: true);
}

if (pointedAtType.SpecialType == SpecialType.System_Void)
if (pointedAtType.IsVoidType())
{
Error(diagnostics, ErrorCode.ERR_VoidError, expr.Syntax);
hasErrors = true;
Expand Down Expand Up @@ -8005,7 +8005,9 @@ private BoundConditionalAccess BindConditionalAccessExpression(ConditionalAccess
}

// if access has value type, the type of the conditional access is nullable of that
if (accessType.IsValueType && !accessType.IsNullableType() && accessType.SpecialType != SpecialType.System_Void)
// https://github.com/dotnet/roslyn/issues/35075: The test `accessType.IsValueType && !accessType.IsNullableType()`
// should probably be `accessType.IsNonNullableValueType()`
if (accessType.IsValueType && !accessType.IsNullableType() && !accessType.IsVoidType())
{
accessType = GetSpecialType(SpecialType.System_Nullable_T, diagnostics, node).Construct(accessType);
}
Expand Down Expand Up @@ -8121,7 +8123,7 @@ private BoundExpression BindConditionalAccessReceiver(ConditionalAccessExpressio
}

// No member accesses on void
if (receiverType.SpecialType == SpecialType.System_Void)
if (receiverType.IsVoidType())
{
Error(diagnostics, ErrorCode.ERR_BadUnaryOp, operatorToken.GetLocation(), operatorToken.Text, receiverType);
return BadExpression(receiverSyntax, receiver);
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Initializers.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ private static BoundInitializer BindGlobalStatement(
// insert an implicit conversion for the submission return type (if needed):
var expression = InitializerRewriter.GetTrailingScriptExpression(statement);
if (expression != null &&
((object)expression.Type == null || expression.Type.SpecialType != SpecialType.System_Void))
((object)expression.Type == null || !expression.Type.IsVoidType()))
{
var submissionResultType = scriptInitializer.ResultType;
expression = binder.GenerateConversionForAssignment(submissionResultType, expression, diagnostics);
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Invocation.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ private BoundExpression BindArgListOperator(InvocationExpressionSyntax node, Dia

analyzedArguments.Arguments[i] = GenerateConversionForAssignment(objType, argument, diagnostics);
}
else if (argument.Type.SpecialType == SpecialType.System_Void)
else if (argument.Type.IsVoidType())
{
Error(diagnostics, ErrorCode.ERR_CantUseVoidInArglist, argument.Syntax);
}
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Lookup.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1666,7 +1666,7 @@ private void AddMemberLookupSymbolsInfoInClass(LookupSymbolsInfo result, TypeSym
PooledHashSet<NamedTypeSymbol> visited = null;
// We need a check for SpecialType.System_Void as its base type is
// ValueType but we don't wish to return any members for void type
while ((object)type != null && type.SpecialType != SpecialType.System_Void)
while ((object)type != null && !type.IsVoidType())
{
AddMemberLookupSymbolsInfoWithoutInheritance(result, type, options, originalBinder, accessThroughType);

Expand Down
8 changes: 4 additions & 4 deletions src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ private static bool IsLegalDynamicOperand(BoundExpression operand)

// Pointer types and very special types are not convertible to object.

return !type.IsPointerType() && !type.IsRestrictedType() && type.SpecialType != SpecialType.System_Void;
return !type.IsPointerType() && !type.IsRestrictedType() && !type.IsVoidType();
}

private BoundExpression BindDynamicBinaryOperator(
Expand Down Expand Up @@ -2111,7 +2111,7 @@ private static void BindPointerIndirectionExpressionInternal(CSharpSyntaxNode no
{
pointedAtType = operandType.PointedAtType;

if (pointedAtType.SpecialType == SpecialType.System_Void)
if (pointedAtType.IsVoidType())
{
pointedAtType = null;

Expand Down Expand Up @@ -2828,7 +2828,7 @@ private BoundExpression BindIsOperator(BinaryExpressionSyntax node, DiagnosticBa

if (operand.ConstantValue == ConstantValue.Null ||
operand.Kind == BoundKind.MethodGroup ||
operand.Type.SpecialType == SpecialType.System_Void)
operand.Type.IsVoidType())
{
// warning for cases where the result is always false:
// (a) "null is TYPE" OR operand evaluates to null
Expand Down Expand Up @@ -3351,7 +3351,7 @@ private static bool ReportAsOperatorConversionDiagnostics(
// Generate an error if there is no possible legal conversion and both the operandType
// and the targetType are closed types OR operandType is void type, otherwise we need a runtime check
if (!operandType.ContainsTypeParameter() && !targetType.ContainsTypeParameter() ||
operandType.SpecialType == SpecialType.System_Void)
operandType.IsVoidType())
{
SymbolDistinguisher distinguisher = new SymbolDistinguisher(compilation, operandType, targetType);
Error(diagnostics, ErrorCode.ERR_NoExplicitBuiltinConv, node, distinguisher.First, distinguisher.Second);
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Patterns.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ private BoundExpression BindIsPatternExpression(IsPatternExpressionSyntax node,
BoundExpression expression = BindValue(node.Expression, diagnostics, BindValueKind.RValue);
bool hasErrors = IsOperandErrors(node, ref expression, diagnostics);
TypeSymbol expressionType = expression.Type;
if ((object)expressionType == null || expressionType.SpecialType == SpecialType.System_Void)
if ((object)expressionType == null || expressionType.IsVoidType())
{
if (!hasErrors)
{
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/Binder/Binder_Query.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,7 @@ private void ReduceLet(LetClauseSyntax let, QueryTranslationState state, Diagnos
Error(d, ErrorCode.ERR_QueryRangeVariableAssignedBadValue, errorLocation, yExpression.Display);
yExpression = new BoundBadExpression(yExpression.Syntax, LookupResultKind.Empty, ImmutableArray<Symbol>.Empty, ImmutableArray.Create(yExpression), CreateErrorType());
}
else if (!yExpression.HasAnyErrors && yExpression.Type.SpecialType == SpecialType.System_Void)
else if (!yExpression.HasAnyErrors && yExpression.Type.IsVoidType())
{
Error(d, ErrorCode.ERR_QueryRangeVariableAssignedBadValue, errorLocation, yExpression.Type);
yExpression = new BoundBadExpression(yExpression.Syntax, LookupResultKind.Empty, ImmutableArray<Symbol>.Empty, ImmutableArray.Create(yExpression), yExpression.Type);
Expand Down Expand Up @@ -761,7 +761,7 @@ protected BoundCall MakeQueryInvocation(CSharpSyntaxNode node, BoundExpression r

receiver = new BoundBadExpression(receiver.Syntax, LookupResultKind.NotAValue, ImmutableArray<Symbol>.Empty, ImmutableArray.Create(receiver), CreateErrorType());
}
else if (receiver.Type.SpecialType == SpecialType.System_Void)
else if (receiver.Type.IsVoidType())
{
if (!receiver.HasAnyErrors && !node.HasErrors)
{
Expand Down
16 changes: 8 additions & 8 deletions src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -960,7 +960,7 @@ protected BoundLocalDeclaration BindVariableDeclaration(
{
declTypeOpt = TypeWithAnnotations.Create(initializerType);

if (declTypeOpt.SpecialType == SpecialType.System_Void)
if (declTypeOpt.IsVoidType())
{
Error(localDiagnostics, ErrorCode.ERR_ImplicitlyTypedVariableAssignedBadValue, declarator, declTypeOpt.Type);
declTypeOpt = TypeWithAnnotations.Create(CreateErrorType("var"));
Expand Down Expand Up @@ -1254,7 +1254,7 @@ private bool IsValidFixedVariableInitializer(TypeSymbol declType, SourceLocalSym

private MethodSymbol GetFixedPatternMethodOpt(BoundExpression initializer, DiagnosticBag additionalDiagnostics)
{
if (initializer.Type.SpecialType == SpecialType.System_Void)
if (initializer.Type.IsVoidType())
{
return null;
}
Expand Down Expand Up @@ -1384,7 +1384,7 @@ private BoundExpression InferTypeForDiscardAssignment(BoundDiscardExpression op1
return op1.FailInference(this, diagnostics);
}

if (inferredType.SpecialType == SpecialType.System_Void)
if (inferredType.IsVoidType())
{
diagnostics.Add(ErrorCode.ERR_VoidAssignment, op1.Syntax.Location);
}
Expand Down Expand Up @@ -2615,7 +2615,7 @@ private BoundStatement BindReturn(ReturnStatementSyntax syntax, DiagnosticBag di
// on a lambda expression of unknown return type.
if ((object)retType != null)
{
if (retType.SpecialType == SpecialType.System_Void || IsTaskReturningAsyncMethod())
if (retType.IsVoidType() || IsTaskReturningAsyncMethod())
{
if (arg != null)
{
Expand All @@ -2624,7 +2624,7 @@ private BoundStatement BindReturn(ReturnStatementSyntax syntax, DiagnosticBag di
if ((object)lambda != null)
{
// Error case: void-returning or async task-returning method or lambda with "return x;"
var errorCode = retType.SpecialType == SpecialType.System_Void
var errorCode = retType.IsVoidType()
? ErrorCode.ERR_RetNoObjectRequiredLambda
: ErrorCode.ERR_TaskRetNoObjectRequiredLambda;

Expand All @@ -2640,7 +2640,7 @@ private BoundStatement BindReturn(ReturnStatementSyntax syntax, DiagnosticBag di
else
{
// Error case: void-returning or async task-returning method or lambda with "return x;"
var errorCode = retType.SpecialType == SpecialType.System_Void
var errorCode = retType.IsVoidType()
? ErrorCode.ERR_RetNoObjectRequired
: ErrorCode.ERR_TaskRetNoObjectRequired;

Expand Down Expand Up @@ -2668,7 +2668,7 @@ private BoundStatement BindReturn(ReturnStatementSyntax syntax, DiagnosticBag di
else
{
// Check that the returned expression is not void.
if ((object)arg?.Type != null && arg.Type.SpecialType == SpecialType.System_Void)
if ((object)arg?.Type != null && arg.Type.IsVoidType())
{
Error(diagnostics, ErrorCode.ERR_CantReturnVoid, expressionSyntax);
}
Expand Down Expand Up @@ -3010,7 +3010,7 @@ internal BoundBlock CreateBlockFromExpression(CSharpSyntaxNode node, ImmutableAr
Error(diagnostics, errorCode, syntax);
statement = new BoundReturnStatement(syntax, RefKind.None, expression) { WasCompilerGenerated = true };
}
else if (returnType.SpecialType == SpecialType.System_Void || IsTaskReturningAsyncMethod())
else if (returnType.IsVoidType() || IsTaskReturningAsyncMethod())
{
// If the return type is void then the expression is required to be a legal
// statement expression.
Expand Down
6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/ConversionsBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -549,7 +549,7 @@ private Conversion ClassifyImplicitBuiltInConversionSlow(TypeSymbol source, Type
Debug.Assert((object)source != null);
Debug.Assert((object)destination != null);

if (source.SpecialType == SpecialType.System_Void || destination.SpecialType == SpecialType.System_Void)
if (source.IsVoidType() || destination.IsVoidType())
{
return Conversion.NoConversion;
}
Expand All @@ -574,7 +574,7 @@ private Conversion ClassifyExplicitBuiltInOnlyConversion(TypeSymbol source, Type
Debug.Assert((object)source != null);
Debug.Assert((object)destination != null);

if (source.SpecialType == SpecialType.System_Void || destination.SpecialType == SpecialType.System_Void)
if (source.IsVoidType() || destination.IsVoidType())
{
return Conversion.NoConversion;
}
Expand Down Expand Up @@ -2801,7 +2801,7 @@ private static bool HasImplicitPointerConversion(TypeSymbol source, TypeSymbol d

PointerTypeSymbol pd = destination as PointerTypeSymbol;
PointerTypeSymbol ps = source as PointerTypeSymbol;
return (object)pd != null && (object)ps != null && pd.PointedAtType.SpecialType == SpecialType.System_Void;
return (object)pd != null && (object)ps != null && pd.PointedAtType.IsVoidType();
}

private bool HasIdentityOrReferenceConversion(TypeSymbol source, TypeSymbol destination, ref HashSet<DiagnosticInfo> useSiteDiagnostics)
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/MethodTypeInference.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1274,7 +1274,7 @@ private bool MethodGroupReturnTypeInference(Binder binder, BoundExpression sourc
}

var returnType = MethodGroupReturnType(binder, (BoundMethodGroup)source, fixedDelegateParameters, delegateInvokeMethod.RefKind, ref useSiteDiagnostics);
if ((object)returnType == null || returnType.SpecialType == SpecialType.System_Void)
if ((object)returnType == null || returnType.IsVoidType())
{
return false;
}
Expand Down Expand Up @@ -2764,7 +2764,7 @@ private static bool IsReallyAType(TypeSymbol type)
{
return (object)type != null &&
!type.IsErrorType() &&
(type.SpecialType != SpecialType.System_Void);
!type.IsVoidType();
}

private static void GetAllCandidates(Dictionary<TypeWithAnnotations, TypeWithAnnotations> candidates, ArrayBuilder<TypeWithAnnotations> builder)
Expand Down
Loading

0 comments on commit bd5da4b

Please sign in to comment.