From c9da43141cc2dae4ae0303381458bb842f1ebadb Mon Sep 17 00:00:00 2001 From: Neal Gafter Date: Tue, 13 Nov 2018 15:20:12 -0800 Subject: [PATCH] Prevent type and constant value on a constant pattern See also https://github.com/dotnet/roslyn/issues/31150 --- .../Operations/CSharpOperationFactory.cs | 9 +-- .../IOperationTests_ISwitchOperation.cs | 16 ++--- .../Operations/ControlFlowGraphBuilder.cs | 68 ++++++++----------- .../Portable/Operations/OperationCloner.cs | 4 +- .../Portable/Operations/OperationNodes.cs | 28 +++----- .../Operations/VisualBasicOperationFactory.vb | 2 +- 6 files changed, 53 insertions(+), 74 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Operations/CSharpOperationFactory.cs b/src/Compilers/CSharp/Portable/Operations/CSharpOperationFactory.cs index c2b403d13a826..3083a534bce1f 100644 --- a/src/Compilers/CSharp/Portable/Operations/CSharpOperationFactory.cs +++ b/src/Compilers/CSharp/Portable/Operations/CSharpOperationFactory.cs @@ -1964,10 +1964,8 @@ private IConstantPatternOperation CreateBoundConstantPatternOperation(BoundConst { Lazy value = new Lazy(() => Create(boundConstantPattern.Value)); SyntaxNode syntax = boundConstantPattern.Syntax; - ITypeSymbol type = null; - Optional constantValue = default(Optional); bool isImplicit = boundConstantPattern.WasCompilerGenerated; - return new LazyConstantPattern(value, _semanticModel, syntax, type, constantValue, isImplicit); + return new LazyConstantPattern(value, _semanticModel, syntax, isImplicit); } private IDeclarationPatternOperation CreateBoundDeclarationPatternOperation(BoundDeclarationPattern boundDeclarationPattern) @@ -2005,8 +2003,7 @@ private ISwitchOperation CreateBoundPatternSwitchStatementOperation(BoundSwitchS ITypeSymbol type = null; Optional constantValue = default(Optional); bool isImplicit = boundPatternSwitchStatement.WasCompilerGenerated; - bool wasPatternSwitch = boundPatternSwitchStatement.Expression.Type?.IsValidV6SwitchGoverningType() == false; - return new LazySwitchStatement(locals, value, cases, exitLabel, wasPatternSwitch, _semanticModel, syntax, type, constantValue, isImplicit); + return new LazySwitchStatement(locals, value, cases, exitLabel, _semanticModel, syntax, type, constantValue, isImplicit); } private ICaseClauseOperation CreateBoundPatternSwitchLabelOperation(BoundPatternSwitchLabel boundPatternSwitchLabel) @@ -2025,7 +2022,7 @@ private ICaseClauseOperation CreateBoundPatternSwitchLabelOperation(BoundPattern else if (boundPatternSwitchLabel.WhenClause == null && boundPatternSwitchLabel.Pattern.Kind == BoundKind.ConstantPattern && boundPatternSwitchLabel.Pattern is BoundConstantPattern cp && - cp.Value.Type.IsValidV6SwitchGoverningType()) + cp.InputType.IsValidV6SwitchGoverningType()) { Lazy value = new Lazy(() => Create(cp.Value)); return new LazySingleValueCaseClause(label, value, _semanticModel, syntax, type, constantValue, isImplicit); diff --git a/src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_ISwitchOperation.cs b/src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_ISwitchOperation.cs index f38a7c3ac4aee..1f947ca29695c 100644 --- a/src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_ISwitchOperation.cs +++ b/src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_ISwitchOperation.cs @@ -1972,11 +1972,11 @@ void M(bool result, dynamic input) IParameterReferenceOperation: input (OperationKind.ParameterReference, Type: dynamic) (Syntax: 'input') Jump if False (Regular) to Block[B3] - IIsPatternOperation (OperationKind.IsPattern, Type: System.Boolean, IsImplicit) (Syntax: '1') + IIsPatternOperation (OperationKind.IsPattern, Type: System.Boolean, IsImplicit) (Syntax: 'case 1:') Expression: IFlowCaptureReferenceOperation: 0 (OperationKind.FlowCaptureReference, Type: dynamic, IsImplicit) (Syntax: 'input') Pattern: - IConstantPatternOperation (OperationKind.ConstantPattern, Type: System.Int32, Constant: 1, IsImplicit) (Syntax: '1') + IConstantPatternOperation (OperationKind.ConstantPattern, Type: null, IsImplicit) (Syntax: 'case 1:') Value: ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 1) (Syntax: '1') Leaving: {R1} @@ -2424,11 +2424,11 @@ void M(bool result, object input) IParameterReferenceOperation: input (OperationKind.ParameterReference, Type: System.Object) (Syntax: 'input') Jump if False (Regular) to Block[B3] - IIsPatternOperation (OperationKind.IsPattern, Type: System.Boolean, IsImplicit) (Syntax: '1') + IIsPatternOperation (OperationKind.IsPattern, Type: System.Boolean, IsImplicit) (Syntax: 'case 1:') Expression: IFlowCaptureReferenceOperation: 0 (OperationKind.FlowCaptureReference, Type: System.Object, IsImplicit) (Syntax: 'input') Pattern: - IConstantPatternOperation (OperationKind.ConstantPattern, Type: System.Int32, Constant: 1, IsImplicit) (Syntax: '1') + IConstantPatternOperation (OperationKind.ConstantPattern, Type: null, IsImplicit) (Syntax: 'case 1:') Value: ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 1) (Syntax: '1') Leaving: {R1} @@ -2801,11 +2801,11 @@ void M(bool result, object input) IParameterReferenceOperation: input (OperationKind.ParameterReference, Type: System.Object) (Syntax: 'input') Jump if False (Regular) to Block[B3] - IIsPatternOperation (OperationKind.IsPattern, Type: System.Boolean, IsImplicit) (Syntax: '1') + IIsPatternOperation (OperationKind.IsPattern, Type: System.Boolean, IsImplicit) (Syntax: 'case 1:') Expression: IFlowCaptureReferenceOperation: 0 (OperationKind.FlowCaptureReference, Type: System.Object, IsImplicit) (Syntax: 'input') Pattern: - IConstantPatternOperation (OperationKind.ConstantPattern, Type: System.Int32, Constant: 1, IsImplicit) (Syntax: '1') + IConstantPatternOperation (OperationKind.ConstantPattern, Type: null, IsImplicit) (Syntax: 'case 1:') Value: ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 1) (Syntax: '1') @@ -2827,11 +2827,11 @@ void M(bool result, object input) Predecessors: [B1] Statements (0) Jump if False (Regular) to Block[B5] - IIsPatternOperation (OperationKind.IsPattern, Type: System.Boolean, IsImplicit) (Syntax: '2') + IIsPatternOperation (OperationKind.IsPattern, Type: System.Boolean, IsImplicit) (Syntax: 'case 2:') Expression: IFlowCaptureReferenceOperation: 0 (OperationKind.FlowCaptureReference, Type: System.Object, IsImplicit) (Syntax: 'input') Pattern: - IConstantPatternOperation (OperationKind.ConstantPattern, Type: System.Int32, Constant: 2, IsImplicit) (Syntax: '2') + IConstantPatternOperation (OperationKind.ConstantPattern, Type: null, IsImplicit) (Syntax: 'case 2:') Value: ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 2) (Syntax: '2') Leaving: {R1} diff --git a/src/Compilers/Core/Portable/Operations/ControlFlowGraphBuilder.cs b/src/Compilers/Core/Portable/Operations/ControlFlowGraphBuilder.cs index 2928f981de106..538e39d409ba2 100644 --- a/src/Compilers/Core/Portable/Operations/ControlFlowGraphBuilder.cs +++ b/src/Compilers/Core/Portable/Operations/ControlFlowGraphBuilder.cs @@ -4881,8 +4881,6 @@ public override IOperation VisitSwitch(ISwitchOperation operation, int? captureI INamedTypeSymbol booleanType = _compilation.GetSpecialType(SpecialType.System_Boolean); IOperation switchValue = VisitAndCapture(operation.Value); - bool patternMatchingOnly = ((BaseSwitchStatement)operation).PatternMatchingOnly; - ImmutableArray locals = getLocals(); var switchRegion = new RegionBuilder(ControlFlowRegionKind.LocalLifetime, locals: locals); EnterRegion(switchRegion); @@ -4962,59 +4960,49 @@ void handleCase(ICaseClauseOperation caseClause, BasicBlockBuilder body, BasicBl switch (caseClause.CaseKind) { case CaseKind.SingleValue: - handleEqualityCheck(((ISingleValueCaseClauseOperation)caseClause).Value); + handleEqualityCheck(((ISingleValueCaseClauseOperation)caseClause).Value); break; void handleEqualityCheck(IOperation compareWith) { + bool leftIsNullable = ITypeSymbolHelpers.IsNullableType(operation.Value.Type); + bool rightIsNullable = ITypeSymbolHelpers.IsNullableType(compareWith.Type); + bool isLifted = leftIsNullable || rightIsNullable; + EvalStackFrame frame = PushStackFrame(); PushOperand(OperationCloner.CloneOperation(switchValue)); IOperation rightOperand = Visit(compareWith); IOperation leftOperand = PopOperand(); - if (patternMatchingOnly) + if (isLifted) { - var pattern = (IPatternOperation)new ConstantPattern(rightOperand, semanticModel: null, - compareWith.Syntax, compareWith.Type, compareWith.ConstantValue, isImplicit: true); - condition = new IsPatternExpression(leftOperand, pattern, semanticModel: null, - compareWith.Syntax, booleanType, constantValue: default, isImplicit: true); - } - else - { - bool leftIsNullable = ITypeSymbolHelpers.IsNullableType(operation.Value.Type); - bool rightIsNullable = ITypeSymbolHelpers.IsNullableType(compareWith.Type); - bool isLifted = leftIsNullable || rightIsNullable; - - if (isLifted) + if (!leftIsNullable) { - if (!leftIsNullable) + if (leftOperand.Type != null) { - if (leftOperand.Type != null) - { - leftOperand = MakeNullable(leftOperand, compareWith.Type); - } - } - else if (!rightIsNullable && rightOperand.Type != null) - { - rightOperand = MakeNullable(rightOperand, operation.Value.Type); + leftOperand = MakeNullable(leftOperand, compareWith.Type); } } - - condition = new BinaryOperatorExpression(BinaryOperatorKind.Equals, - leftOperand, - rightOperand, - isLifted, - isChecked: false, - isCompareText: false, - operatorMethod: null, - unaryOperatorMethod: null, - semanticModel: null, - compareWith.Syntax, - booleanType, - constantValue: default, - isImplicit: true); + else if (!rightIsNullable && rightOperand.Type != null) + { + rightOperand = MakeNullable(rightOperand, operation.Value.Type); + } } + condition = new BinaryOperatorExpression(BinaryOperatorKind.Equals, + leftOperand, + rightOperand, + isLifted, + isChecked: false, + isCompareText: false, + operatorMethod: null, + unaryOperatorMethod: null, + semanticModel: null, + compareWith.Syntax, + booleanType, + constantValue: default, + isImplicit: true); + condition = Operation.SetParentOperation(condition, null); LinkBlocks(CurrentBasicBlock, condition, jumpIfTrue: false, RegularBranch(nextCase)); @@ -6558,7 +6546,7 @@ public override IOperation VisitTranslatedQuery(ITranslatedQueryOperation operat public override IOperation VisitConstantPattern(IConstantPatternOperation operation, int? captureIdForResult) { return new ConstantPattern(Visit(operation.Value), semanticModel: null, - operation.Syntax, operation.Type, operation.ConstantValue, IsImplicit(operation)); + syntax: operation.Syntax, isImplicit: IsImplicit(operation)); } public override IOperation VisitDeclarationPattern(IDeclarationPatternOperation operation, int? captureIdForResult) diff --git a/src/Compilers/Core/Portable/Operations/OperationCloner.cs b/src/Compilers/Core/Portable/Operations/OperationCloner.cs index 9d2edc4c4c957..bbece9661305f 100644 --- a/src/Compilers/Core/Portable/Operations/OperationCloner.cs +++ b/src/Compilers/Core/Portable/Operations/OperationCloner.cs @@ -77,7 +77,7 @@ public override IOperation VisitConversion(IConversionOperation operation, objec public override IOperation VisitSwitch(ISwitchOperation operation, object argument) { - return new SwitchStatement(operation.Locals, Visit(operation.Value), VisitArray(operation.Cases), operation.ExitLabel, ((BaseSwitchStatement)operation).PatternMatchingOnly, ((Operation)operation).OwningSemanticModel, operation.Syntax, operation.Type, operation.ConstantValue, operation.IsImplicit); + return new SwitchStatement(operation.Locals, Visit(operation.Value), VisitArray(operation.Cases), operation.ExitLabel, ((Operation)operation).OwningSemanticModel, operation.Syntax, operation.Type, operation.ConstantValue, operation.IsImplicit); } public override IOperation VisitSwitchCase(ISwitchCaseOperation operation, object argument) @@ -519,7 +519,7 @@ public override IOperation VisitIsPattern(IIsPatternOperation operation, object public override IOperation VisitConstantPattern(IConstantPatternOperation operation, object argument) { - return new ConstantPattern(Visit(operation.Value), ((Operation)operation).OwningSemanticModel, operation.Syntax, operation.Type, operation.ConstantValue, operation.IsImplicit); + return new ConstantPattern(Visit(operation.Value), ((Operation)operation).OwningSemanticModel, operation.Syntax, operation.IsImplicit); } public override IOperation VisitDeclarationPattern(IDeclarationPatternOperation operation, object argument) diff --git a/src/Compilers/Core/Portable/Operations/OperationNodes.cs b/src/Compilers/Core/Portable/Operations/OperationNodes.cs index 7f0dc1358914e..7cab0b696ec4e 100644 --- a/src/Compilers/Core/Portable/Operations/OperationNodes.cs +++ b/src/Compilers/Core/Portable/Operations/OperationNodes.cs @@ -4855,12 +4855,11 @@ public LazySwitchCase(ImmutableArray locals, Lazy cond /// internal abstract partial class BaseSwitchStatement : Operation, ISwitchOperation { - protected BaseSwitchStatement(ImmutableArray locals, ILabelSymbol exitLabel, bool patternMatchingOnly, SemanticModel semanticModel, SyntaxNode syntax, ITypeSymbol type, Optional constantValue, bool isImplicit) : + protected BaseSwitchStatement(ImmutableArray locals, ILabelSymbol exitLabel, SemanticModel semanticModel, SyntaxNode syntax, ITypeSymbol type, Optional constantValue, bool isImplicit) : base(OperationKind.Switch, semanticModel, syntax, type, constantValue, isImplicit) { Locals = locals; ExitLabel = exitLabel; - PatternMatchingOnly = patternMatchingOnly; } public ImmutableArray Locals { get; } @@ -4891,11 +4890,6 @@ public override IEnumerable Children /// Cases of the switch. /// public abstract ImmutableArray Cases { get; } - /// - /// True if the switch expression in C# is not valid as a C# 6 switch expression, implying that the switch - /// statement must be considered a pattern-matching (C# 7 and later) switch statement. - /// - public bool PatternMatchingOnly { get; } public override void Accept(OperationVisitor visitor) { visitor.VisitSwitch(this); @@ -4911,8 +4905,8 @@ public override TResult Accept(OperationVisitor internal sealed partial class SwitchStatement : BaseSwitchStatement, ISwitchOperation { - public SwitchStatement(ImmutableArray locals, IOperation value, ImmutableArray cases, ILabelSymbol exitLabel, bool patternMatchingOnly, SemanticModel semanticModel, SyntaxNode syntax, ITypeSymbol type, Optional constantValue, bool isImplicit) : - base(locals, exitLabel, patternMatchingOnly, semanticModel, syntax, type, constantValue, isImplicit) + public SwitchStatement(ImmutableArray locals, IOperation value, ImmutableArray cases, ILabelSymbol exitLabel, SemanticModel semanticModel, SyntaxNode syntax, ITypeSymbol type, Optional constantValue, bool isImplicit) : + base(locals, exitLabel, semanticModel, syntax, type, constantValue, isImplicit) { Value = SetParentOperation(value, this); Cases = SetParentOperation(cases, this); @@ -4930,8 +4924,8 @@ internal sealed partial class LazySwitchStatement : BaseSwitchStatement, ISwitch private readonly Lazy _lazyValue; private readonly Lazy> _lazyCases; - public LazySwitchStatement(ImmutableArray locals, Lazy value, Lazy> cases, ILabelSymbol exitLabel, bool patternMatchingOnly, SemanticModel semanticModel, SyntaxNode syntax, ITypeSymbol type, Optional constantValue, bool isImplicit) : - base(locals, exitLabel, patternMatchingOnly, semanticModel, syntax, type, constantValue, isImplicit) + public LazySwitchStatement(ImmutableArray locals, Lazy value, Lazy> cases, ILabelSymbol exitLabel, SemanticModel semanticModel, SyntaxNode syntax, ITypeSymbol type, Optional constantValue, bool isImplicit) : + base(locals, exitLabel, semanticModel, syntax, type, constantValue, isImplicit) { _lazyValue = value ?? throw new System.ArgumentNullException(nameof(value)); _lazyCases = cases; @@ -6128,8 +6122,8 @@ public LazyLocalFunctionStatement(IMethodSymbol symbol, Lazy bo /// internal abstract partial class BaseConstantPattern : Operation, IConstantPatternOperation { - protected BaseConstantPattern(SemanticModel semanticModel, SyntaxNode syntax, ITypeSymbol type, Optional constantValue, bool isImplicit) : - base(OperationKind.ConstantPattern, semanticModel, syntax, type, constantValue, isImplicit) + protected BaseConstantPattern(SemanticModel semanticModel, SyntaxNode syntax, bool isImplicit) : + base(OperationKind.ConstantPattern, semanticModel, syntax, type: null, constantValue: default, isImplicit) { } @@ -6162,8 +6156,8 @@ public override TResult Accept(OperationVisitor internal sealed partial class ConstantPattern : BaseConstantPattern, IConstantPatternOperation { - public ConstantPattern(IOperation value, SemanticModel semanticModel, SyntaxNode syntax, ITypeSymbol type, Optional constantValue, bool isImplicit) : - base(semanticModel, syntax, type, constantValue, isImplicit) + public ConstantPattern(IOperation value, SemanticModel semanticModel, SyntaxNode syntax, bool isImplicit) : + base(semanticModel, syntax, isImplicit) { Value = SetParentOperation(value, this); } @@ -6178,8 +6172,8 @@ internal sealed partial class LazyConstantPattern : BaseConstantPattern, IConsta { private readonly Lazy _lazyValue; - public LazyConstantPattern(Lazy value, SemanticModel semanticModel, SyntaxNode syntax, ITypeSymbol type, Optional constantValue, bool isImplicit) : - base(semanticModel, syntax, type, constantValue, isImplicit) + public LazyConstantPattern(Lazy value, SemanticModel semanticModel, SyntaxNode syntax, bool isImplicit) : + base(semanticModel, syntax, isImplicit) { _lazyValue = value ?? throw new System.ArgumentNullException(nameof(value)); } diff --git a/src/Compilers/VisualBasic/Portable/Operations/VisualBasicOperationFactory.vb b/src/Compilers/VisualBasic/Portable/Operations/VisualBasicOperationFactory.vb index a4673da965ffd..4cc5852c32ee5 100644 --- a/src/Compilers/VisualBasic/Portable/Operations/VisualBasicOperationFactory.vb +++ b/src/Compilers/VisualBasic/Portable/Operations/VisualBasicOperationFactory.vb @@ -1057,7 +1057,7 @@ Namespace Microsoft.CodeAnalysis.Operations Dim type As ITypeSymbol = Nothing Dim constantValue As [Optional](Of Object) = New [Optional](Of Object)() Dim isImplicit As Boolean = boundSelectStatement.WasCompilerGenerated - Return New LazySwitchStatement(ImmutableArray(Of ILocalSymbol).Empty, value, cases, exitLabel, patternMatchingOnly:=False, _semanticModel, syntax, type, constantValue, isImplicit) + Return New LazySwitchStatement(ImmutableArray(Of ILocalSymbol).Empty, value, cases, exitLabel, _semanticModel, syntax, type, constantValue, isImplicit) End Function Private Function CreateBoundCaseBlockOperation(boundCaseBlock As BoundCaseBlock) As ISwitchCaseOperation