diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Crefs.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Crefs.cs index dd151040077a2ffd5c44b22873ab56a7d0e17120..6c2ae93c855f10b5dfbb146e12bc6d40a89e2bee 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Crefs.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Crefs.cs @@ -885,7 +885,7 @@ private ImmutableArray BindCrefParameters(BaseCrefParameterList } /// - /// Keep in sync with CSharpSemanticModel.GetSpeculativelyBoundExpression. + /// Keep in sync with CSharpSemanticModel.GetSpeculativelyBoundExpressionWithoutNullability. /// private TypeSymbol BindCrefParameterOrReturnType(TypeSyntax typeSyntax, MemberCrefSyntax memberCrefSyntax, DiagnosticBag diagnostics) { diff --git a/src/Compilers/CSharp/Portable/Compilation/CSharpSemanticModel.cs b/src/Compilers/CSharp/Portable/Compilation/CSharpSemanticModel.cs index df6973257483d6157c114d12fe740ffe6cfc9bd1..40a0b84af4b23214b8a078eed7baae841b34ea3f 100644 --- a/src/Compilers/CSharp/Portable/Compilation/CSharpSemanticModel.cs +++ b/src/Compilers/CSharp/Portable/Compilation/CSharpSemanticModel.cs @@ -151,6 +151,18 @@ internal static bool CanGetSemanticInfo(CSharpSyntaxNode node, bool allowNamedAr /// The cancellation token. internal abstract CSharpTypeInfo GetTypeInfoWorker(CSharpSyntaxNode node, CancellationToken cancellationToken = default(CancellationToken)); + /// + /// Binds the provided expression in the given context. + /// + /// The position to bind at. + /// The expression to bind + /// How to speculatively bind the given expression. If this is + /// then the provided expression should be a . + /// The binder that was used to bind the given syntax. + /// The symbols used in a cref. If this is not default, then the return is null. + /// The expression that was bound. If is not default, this is null. + internal abstract BoundExpression GetSpeculativelyBoundExpression(int position, ExpressionSyntax expression, SpeculativeBindingOption bindingOption, out Binder binder, out ImmutableArray crefSymbols); + /// /// Gets a list of method or indexed property symbols for a syntax node. This is overridden by various specializations of SemanticModel. /// It can assume that CheckSyntaxNode and CanGetSemanticInfo have already been called, as well as that named @@ -233,7 +245,7 @@ private Binder GetSpeculativeBinderForAttribute(int position, AttributeSyntax at return new ExecutableCodeBinder(attribute, binder.ContainingMemberOrLambda, binder).GetBinder(attribute); } - private static BoundExpression GetSpeculativelyBoundExpressionHelper(Binder binder, ExpressionSyntax expression, SpeculativeBindingOption bindingOption, DiagnosticBag diagnostics) + protected static BoundExpression GetSpeculativelyBoundExpressionHelper(Binder binder, ExpressionSyntax expression, SpeculativeBindingOption bindingOption, DiagnosticBag diagnostics) { Debug.Assert(binder != null); Debug.Assert(binder.IsSemanticModelBinder); @@ -261,7 +273,7 @@ private static BoundExpression GetSpeculativelyBoundExpressionHelper(Binder bind /// /// Keep in sync with Binder.BindCrefParameterOrReturnType. /// - private BoundExpression GetSpeculativelyBoundExpression(int position, ExpressionSyntax expression, SpeculativeBindingOption bindingOption, out Binder binder, out ImmutableArray crefSymbols) + protected BoundExpression GetSpeculativelyBoundExpressionWithoutNullability(int position, ExpressionSyntax expression, SpeculativeBindingOption bindingOption, out Binder binder, out ImmutableArray crefSymbols) { if (expression == null) { diff --git a/src/Compilers/CSharp/Portable/Compilation/MemberSemanticModel.cs b/src/Compilers/CSharp/Portable/Compilation/MemberSemanticModel.cs index 85def57fa6ece0a5541e20042ee063ab0856eebe..10c89f478b99c7e65f2b9bf9b0eb7c62f1ca6f7e 100644 --- a/src/Compilers/CSharp/Portable/Compilation/MemberSemanticModel.cs +++ b/src/Compilers/CSharp/Portable/Compilation/MemberSemanticModel.cs @@ -139,6 +139,9 @@ internal override MemberSemanticModel GetMemberModel(SyntaxNode node) return IsInTree(node) ? this : null; } + // We hide this property, as EnsureRootBoundForNullabilityIfNecessary can cause + // the cache to be populated. + [DebuggerBrowsable(DebuggerBrowsableState.Never)] protected NullableWalker.SnapshotManager SnapshotManager { get @@ -170,6 +173,30 @@ internal sealed override bool TryGetSpeculativeSemanticModelCore(SyntaxTreeSeman return false; } + internal override BoundExpression GetSpeculativelyBoundExpression(int position, ExpressionSyntax expression, SpeculativeBindingOption bindingOption, out Binder binder, out ImmutableArray crefSymbols) + { + if (expression == null) + { + throw new ArgumentNullException(nameof(expression)); + } + + if (!Compilation.NullableSemanticAnalysisEnabled || bindingOption != SpeculativeBindingOption.BindAsExpression) + { + return GetSpeculativelyBoundExpressionWithoutNullability(position, expression, bindingOption, out binder, out crefSymbols); + } + + EnsureRootBoundForNullabilityIfNecessary(); + Debug.Assert(SnapshotManager is object); + + crefSymbols = default; + position = CheckAndAdjustPosition(position); + expression = SyntaxFactory.GetStandaloneExpression(expression); + var bindableNewExpression = GetBindableSyntaxNode(expression); + binder = GetEnclosingBinder(position); + var boundRoot = Bind(binder, bindableNewExpression, _ignoredDiagnostics); + return (BoundExpression)NullableWalker.AnalyzeAndRewriteSpeculation(position, boundRoot, binder, SnapshotManager, takeNewSnapshots: false, out _); + } + private Binder GetEnclosingBinderInternalWithinRoot(SyntaxNode node, int position) { AssertPositionAdjusted(position); @@ -1435,21 +1462,9 @@ protected void GuardedAddBoundTreeForStandaloneSyntax(SyntaxNode syntax, BoundNo NodeMapBuilder.AddToMap(bound, _guardedNodeMap, syntax); } - // If we're a speculative model, then we should never be passed a new manager, as we should not be recording - // new snapshot info, only using what was given when the model was created. If we're not a speculative model, - // then there are 3 possibilities: - // 1. Nullable analysis wasn't enabled, and we should never be passed a manager. - // 2. Nullable analysis is enabled, but we're not adding info for a root node and we shouldn't be passed - // new snapshots. This can occur when we're asked to bind a standalone node that wasn't in the original - // tree, such as aliases. These nodes do not affect analysis, so we should not be attempting to save - // snapshot information for use in speculative models created off this one. - // 3. Nullable analysis is enabled, and we were passed a new manager. If this is the case, we must be adding - // cached nodes for the root syntax node, and the existing snapshot manager must be null. - Debug.Assert((IsSpeculativeSemanticModel && manager is null) || - (!IsSpeculativeSemanticModel && - (manager is null && (!Compilation.NullableSemanticAnalysisEnabled || syntax != Root)) || - (manager is object && syntax == Root && Compilation.NullableSemanticAnalysisEnabled && _lazySnapshotManager is null))); - if (!IsSpeculativeSemanticModel && manager is object) + Debug.Assert((manager is null && (!Compilation.NullableSemanticAnalysisEnabled || syntax != Root)) || + (manager is object && syntax == Root && Compilation.NullableSemanticAnalysisEnabled && _lazySnapshotManager is null)); + if (manager is object) { _lazySnapshotManager = manager; } @@ -1859,10 +1874,10 @@ protected void EnsureRootBoundForNullabilityIfNecessary() #endif } - // If this isn't a speculative model and we have a snapshot manager, - // then we've already done all the work necessary and we should avoid - // taking an unnecessary read lock. - if (!IsSpeculativeSemanticModel && _lazySnapshotManager is object) + // If we have a snapshot manager, then we've already done + // all the work necessary and we should avoid taking an + // unnecessary read lock. + if (_lazySnapshotManager is object) { return; } @@ -1908,7 +1923,7 @@ void ensureSpeculativeNodeBound() return; } - boundRoot = NullableWalker.AnalyzeAndRewriteSpeculation(_speculatedPosition, boundRoot, binder, _parentSnapshotManagerOpt, out var newSnapshots); + boundRoot = NullableWalker.AnalyzeAndRewriteSpeculation(_speculatedPosition, boundRoot, binder, _parentSnapshotManagerOpt, takeNewSnapshots: true, out var newSnapshots); GuardedAddBoundTreeForStandaloneSyntax(bindableRoot, boundRoot, newSnapshots); } diff --git a/src/Compilers/CSharp/Portable/Compilation/SyntaxTreeSemanticModel.cs b/src/Compilers/CSharp/Portable/Compilation/SyntaxTreeSemanticModel.cs index f004248520c87e49f2fd90b372304377e5ee1e47..0960dda8cbdbc8775c8fc0e08cc9302f536c69a9 100644 --- a/src/Compilers/CSharp/Portable/Compilation/SyntaxTreeSemanticModel.cs +++ b/src/Compilers/CSharp/Portable/Compilation/SyntaxTreeSemanticModel.cs @@ -718,6 +718,28 @@ internal sealed override bool TryGetSpeculativeSemanticModelCore(SyntaxTreeSeman return false; } + internal override BoundExpression GetSpeculativelyBoundExpression(int position, ExpressionSyntax expression, SpeculativeBindingOption bindingOption, out Binder binder, out ImmutableArray crefSymbols) + { + if (expression == null) + { + throw new ArgumentNullException(nameof(expression)); + } + + // If the given position is in a member that we can get a semantic model for, we want to defer to that implementation + // of GetSpeculativelyBoundExpression so it can take nullability into account. + if (bindingOption == SpeculativeBindingOption.BindAsExpression) + { + position = CheckAndAdjustPosition(position); + var model = GetMemberModel(position); + if (model is object) + { + return model.GetSpeculativelyBoundExpression(position, expression, bindingOption, out binder, out crefSymbols); + } + } + + return GetSpeculativelyBoundExpressionWithoutNullability(position, expression, bindingOption, out binder, out crefSymbols); + } + private MemberSemanticModel GetMemberModel(int position) { AssertPositionAdjusted(position); diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs index 36b48d53090bff77d394d5a45de9bf6dbde52fa3..007e05fd1f7f04d4f4a6d7e9e4585635b566f01f 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs @@ -488,15 +488,16 @@ protected override ImmutableArray Scan(ref bool badRegion) BoundNode node, Binder binder, SnapshotManager originalSnapshots, + bool takeNewSnapshots, out SnapshotManager newSnapshots) { var analyzedNullabilities = ImmutableDictionary.CreateBuilder(); - var newSnapshotBuilder = new SnapshotManager.Builder(); + var newSnapshotBuilder = takeNewSnapshots ? new SnapshotManager.Builder() : null; var (walker, initialState, symbol) = originalSnapshots.RestoreWalkerToAnalyzeNewNode(position, node, binder, analyzedNullabilities, newSnapshotBuilder); Analyze(walker, symbol, diagnostics: null, initialState, snapshotBuilderOpt: newSnapshotBuilder); var analyzedNullabilitiesMap = analyzedNullabilities.ToImmutable(); - newSnapshots = newSnapshotBuilder.ToManagerAndFree(); + newSnapshots = newSnapshotBuilder?.ToManagerAndFree(); #if DEBUG if (binder.Compilation.NullableSemanticAnalysisEnabled) diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/Source/NullablePublicAPITests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/Source/NullablePublicAPITests.cs index e10929c27d71969ae513b503924db2d706866d79..5c669584d9f64aea7ecb13de3bb49f7265d8aeb6 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/Source/NullablePublicAPITests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/Source/NullablePublicAPITests.cs @@ -913,12 +913,16 @@ void M(string? s1) var conditionalAccessExpression = root.DescendantNodes().OfType().Single(); var ternary = root.DescendantNodes().OfType().Single(); - var newSource = ((ExpressionStatementSyntax)SyntaxFactory.ParseStatement(@"_ = s1 == """" ? s1 : s1;")); - var newTernary = (ConditionalExpressionSyntax)((AssignmentExpressionSyntax)newSource.Expression).Right; + var newSource = (BlockSyntax)SyntaxFactory.ParseStatement(@"{ string? s3 = null; _ = s1 == """" ? s1 : s1; }"); + var newExprStatement = (ExpressionStatementSyntax)newSource.Statements[1]; + var newTernary = (ConditionalExpressionSyntax)((AssignmentExpressionSyntax)newExprStatement.Expression).Right; var inCondition = ((BinaryExpressionSyntax)newTernary.Condition).Left; var whenTrue = newTernary.WhenTrue; var whenFalse = newTernary.WhenFalse; + var newReference = (IdentifierNameSyntax)SyntaxFactory.ParseExpression(@"s1"); + var newCoalesce = (AssignmentExpressionSyntax)SyntaxFactory.ParseExpression(@"s3 ??= s1", options: TestOptions.Regular8); + // Before the if statement verifySpeculativeModel(ifStatement.SpanStart, PublicNullableFlowState.MaybeNull); @@ -940,12 +944,25 @@ void M(string? s1) void verifySpeculativeModel(int spanStart, PublicNullableFlowState conditionFlowState) { Assert.True(model.TryGetSpeculativeSemanticModel(spanStart, newSource, out var speculativeModel)); + var speculativeTypeInfo = speculativeModel.GetTypeInfo(inCondition); Assert.Equal(conditionFlowState, speculativeTypeInfo.Nullability.FlowState); + speculativeTypeInfo = speculativeModel.GetTypeInfo(whenTrue); Assert.Equal(PublicNullableFlowState.NotNull, speculativeTypeInfo.Nullability.FlowState); + + var referenceTypeInfo = speculativeModel.GetSpeculativeTypeInfo(whenTrue.SpanStart, newReference, SpeculativeBindingOption.BindAsExpression); + Assert.Equal(PublicNullableFlowState.NotNull, referenceTypeInfo.Nullability.FlowState); + var coalesceTypeInfo = speculativeModel.GetSpeculativeTypeInfo(whenTrue.SpanStart, newCoalesce, SpeculativeBindingOption.BindAsExpression); + Assert.Equal(PublicNullableFlowState.NotNull, coalesceTypeInfo.Nullability.FlowState); + speculativeTypeInfo = speculativeModel.GetTypeInfo(whenFalse); Assert.Equal(conditionFlowState, speculativeTypeInfo.Nullability.FlowState); + referenceTypeInfo = speculativeModel.GetSpeculativeTypeInfo(whenFalse.SpanStart, newReference, SpeculativeBindingOption.BindAsExpression); + Assert.Equal(conditionFlowState, referenceTypeInfo.Nullability.FlowState); + + coalesceTypeInfo = speculativeModel.GetSpeculativeTypeInfo(whenFalse.SpanStart, newCoalesce, SpeculativeBindingOption.BindAsExpression); + Assert.Equal(conditionFlowState, coalesceTypeInfo.Nullability.FlowState); } } @@ -1011,5 +1028,63 @@ void M(C? x, C x2) Diagnostic(ErrorCode.WRN_NullabilityMismatchInAssignment, "(default(object), default(int))").WithArguments("(object?, int)", "(object a, int b)").WithLocation(11, 32) ); } + + [Fact] + public void SpeculativeGetTypeInfo_Basic() + { + + var source = @" +class C +{ + static object? staticField = null; + object field = staticField is null ? new object() : staticField; + + string M(string? s1) + { + if (s1 != null) + { + s1.ToString(); + } + + s1?.ToString(); + + s1 = """"; + var s2 = s1 == null ? """" : s1; + + return null!; + } +}"; + + var comp = CreateCompilation(source, options: WithNonNullTypesTrue(), parseOptions: TestOptions.Regular8WithNullableAnalysis); + comp.VerifyDiagnostics(); + + var syntaxTree = comp.SyntaxTrees[0]; + var root = syntaxTree.GetRoot(); + var model = comp.GetSemanticModel(syntaxTree); + + var ifStatement = root.DescendantNodes().OfType().Single(); + var conditionalAccessExpression = root.DescendantNodes().OfType().Single(); + var ternary = root.DescendantNodes().OfType().Skip(1).Single(); + + var newReference = (IdentifierNameSyntax)SyntaxFactory.ParseExpression(@"s1"); + var newCoalesce = (AssignmentExpressionSyntax)SyntaxFactory.ParseExpression(@"s1 ??= """""); + + verifySpeculativeTypeInfo(ifStatement.SpanStart, PublicNullableFlowState.MaybeNull); + verifySpeculativeTypeInfo(ifStatement.Statement.SpanStart, PublicNullableFlowState.NotNull); + + verifySpeculativeTypeInfo(conditionalAccessExpression.SpanStart, PublicNullableFlowState.MaybeNull); + verifySpeculativeTypeInfo(conditionalAccessExpression.WhenNotNull.SpanStart, PublicNullableFlowState.NotNull); + + verifySpeculativeTypeInfo(ternary.WhenTrue.SpanStart, PublicNullableFlowState.MaybeNull); + verifySpeculativeTypeInfo(ternary.WhenFalse.SpanStart, PublicNullableFlowState.NotNull); + + void verifySpeculativeTypeInfo(int position, PublicNullableFlowState expectedFlowState) + { + var specTypeInfo = model.GetSpeculativeTypeInfo(position, newReference, SpeculativeBindingOption.BindAsExpression); + Assert.Equal(expectedFlowState, specTypeInfo.Nullability.FlowState); + specTypeInfo = model.GetSpeculativeTypeInfo(position, newCoalesce, SpeculativeBindingOption.BindAsExpression); + Assert.Equal(PublicNullableFlowState.NotNull, specTypeInfo.Nullability.FlowState); + } + } } }