Wire GetSpeculativeTypeInfo with nullability information

src/Compilers/CSharp/Portable/Binder/Binder_Crefs.cs

@@ -885,7 +885,7 @@ private ImmutableArray<ParameterSymbol> BindCrefParameters(BaseCrefParameterList
         }
 
         /// <remarks>
-        /// Keep in sync with CSharpSemanticModel.GetSpeculativelyBoundExpression.
+        /// Keep in sync with CSharpSemanticModel.GetSpeculativelyBoundExpressionWithoutNullability.
         /// </remarks>
         private TypeSymbol BindCrefParameterOrReturnType(TypeSyntax typeSyntax, MemberCrefSyntax memberCrefSyntax, DiagnosticBag diagnostics)
         {

src/Compilers/CSharp/Portable/Compilation/CSharpSemanticModel.cs

@@ -151,6 +151,18 @@ internal static bool CanGetSemanticInfo(CSharpSyntaxNode node, bool allowNamedAr
         /// <param name="cancellationToken">The cancellation token.</param>
         internal abstract CSharpTypeInfo GetTypeInfoWorker(CSharpSyntaxNode node, CancellationToken cancellationToken = default(CancellationToken));
 
+        /// <summary>
+        /// Binds the provided expression in the given context.
+        /// </summary>
+        /// <param name="position">The position to bind at.</param>
+        /// <param name="expression">The expression to bind</param>
+        /// <param name="bindingOption">How to speculatively bind the given expression. If this is <see cref="SpeculativeBindingOption.BindAsTypeOrNamespace"/>
+        /// then the provided expression should be a <see cref="TypeSyntax"/>.</param>
+        /// <param name="binder">The binder that was used to bind the given syntax.</param>
+        /// <param name="crefSymbols">The symbols used in a cref. If this is not default, then the return is null.</param>
+        /// <returns>The expression that was bound. If <paramref name="crefSymbols"/> is not default, this is null.</returns>
+        internal abstract BoundExpression GetSpeculativelyBoundExpression(int position, ExpressionSyntax expression, SpeculativeBindingOption bindingOption, out Binder binder, out ImmutableArray<Symbol> crefSymbols);
+
         /// <summary>
         /// 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
@@ -261,7 +273,7 @@ private static BoundExpression GetSpeculativelyBoundExpressionHelper(Binder bind
         /// <remarks>
         /// Keep in sync with Binder.BindCrefParameterOrReturnType.
         /// </remarks>
-        private BoundExpression GetSpeculativelyBoundExpression(int position, ExpressionSyntax expression, SpeculativeBindingOption bindingOption, out Binder binder, out ImmutableArray<Symbol> crefSymbols)
+        protected BoundExpression GetSpeculativelyBoundExpressionWithoutNullability(int position, ExpressionSyntax expression, SpeculativeBindingOption bindingOption, out Binder binder, out ImmutableArray<Symbol> crefSymbols)
         {
             if (expression == null)
             {

src/Compilers/CSharp/Portable/Compilation/MemberSemanticModel.cs

@@ -27,6 +27,10 @@ internal abstract partial class MemberSemanticModel : CSharpSemanticModel
         private readonly Dictionary<SyntaxNode, ImmutableArray<BoundNode>> _guardedNodeMap = new Dictionary<SyntaxNode, ImmutableArray<BoundNode>>();
         private Dictionary<SyntaxNode, BoundStatement> _lazyGuardedSynthesizedStatementsMap;
         private NullableWalker.SnapshotManager _lazySnapshotManager;
+        /// <summary>
+        /// Only used when this is a speculative semantic model.
+        /// </summary>
+        private readonly NullableWalker.SnapshotManager _parentSnapshotManagerOpt;
 
         internal readonly Binder RootBinder;
 
@@ -63,7 +67,7 @@ protected MemberSemanticModel(
             this.RootBinder = rootBinder.WithAdditionalFlags(GetSemanticModelBinderFlags());
             _containingSemanticModelOpt = containingSemanticModelOpt;
             _parentSemanticModelOpt = parentSemanticModelOpt;
-            _lazySnapshotManager = snapshotManagerOpt;
+            _parentSnapshotManagerOpt = snapshotManagerOpt;
             _speculatedPosition = speculatedPosition;
 
             _operationFactory = new Lazy<CSharpOperationFactory>(() => new CSharpOperationFactory(this));
@@ -135,13 +139,14 @@ internal override MemberSemanticModel GetMemberModel(SyntaxNode node)
             return IsInTree(node) ? this : null;
         }
 
-        protected NullableWalker.SnapshotManager SnapshotManager
+        /// <remarks>
+        /// This will cause the bound node cache to be populated if nullable semantic analysis is enabled.
+        /// </remarks>
+        protected NullableWalker.SnapshotManager GetSnapshotManager()
         {
-            get
-            {
-                EnsureRootBoundForNullabilityIfNecessary();
-                return _lazySnapshotManager;
-            }
+            EnsureRootBoundForNullabilityIfNecessary();
+            Debug.Assert(_lazySnapshotManager is object || !Compilation.NullableSemanticAnalysisEnabled);
+            return _lazySnapshotManager;
         }
 
         internal sealed override bool TryGetSpeculativeSemanticModelCore(SyntaxTreeSemanticModel parentModel, int position, TypeSyntax type, SpeculativeBindingOption bindingOption, out SemanticModel speculativeModel)
@@ -166,6 +171,27 @@ internal sealed override bool TryGetSpeculativeSemanticModelCore(SyntaxTreeSeman
             return false;
         }
 
+        internal override BoundExpression GetSpeculativelyBoundExpression(int position, ExpressionSyntax expression, SpeculativeBindingOption bindingOption, out Binder binder, out ImmutableArray<Symbol> crefSymbols)
+        {
+            if (expression == null)
+            {
+                throw new ArgumentNullException(nameof(expression));
+            }
+
+            if (!Compilation.NullableSemanticAnalysisEnabled || bindingOption != SpeculativeBindingOption.BindAsExpression)
+            {
+                return GetSpeculativelyBoundExpressionWithoutNullability(position, expression, bindingOption, out binder, out crefSymbols);
+            }
+
+            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, GetSnapshotManager(), takeNewSnapshots: false, out _);
+        }
+
         private Binder GetEnclosingBinderInternalWithinRoot(SyntaxNode node, int position)
         {
             AssertPositionAdjusted(position);
@@ -1431,21 +1457,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;
                 }
@@ -1855,10 +1869,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;
             }
@@ -1890,28 +1904,27 @@ protected void EnsureRootBoundForNullabilityIfNecessary()
             }
             else
             {
-                bindAndRewrite(takeSnapshots: true);
+                bindAndRewrite();
             }
 
             void ensureSpeculativeNodeBound()
             {
                 // Not all speculative models are created with existing snapshots. Attributes,
                 // TypeSyntaxes, and MethodBodies do not depend on existing state in a member,
                 // and so the SnapshotManager can be null in these cases.
-                if (_lazySnapshotManager is null)
+                if (_parentSnapshotManagerOpt is null)
                 {
-                    bindAndRewrite(takeSnapshots: false);
+                    bindAndRewrite();
                     return;
                 }
 
-                var analyzedNullabilitiesBuilder = ImmutableDictionary.CreateBuilder<BoundExpression, (NullabilityInfo, TypeSymbol)>();
-                boundRoot = NullableWalker.AnalyzeAndRewriteSpeculation(_speculatedPosition, boundRoot, binder, _lazySnapshotManager);
-                GuardedAddBoundTreeForStandaloneSyntax(bindableRoot, boundRoot);
+                boundRoot = NullableWalker.AnalyzeAndRewriteSpeculation(_speculatedPosition, boundRoot, binder, _parentSnapshotManagerOpt, takeNewSnapshots: true, out var newSnapshots);
+                GuardedAddBoundTreeForStandaloneSyntax(bindableRoot, boundRoot, newSnapshots);
             }
 
-            void bindAndRewrite(bool takeSnapshots)
+            void bindAndRewrite()
             {
-                boundRoot = RewriteNullableBoundNodesWithSnapshots(boundRoot, binder, diagnostics, takeSnapshots, out var snapshotManager);
+                boundRoot = RewriteNullableBoundNodesWithSnapshots(boundRoot, binder, diagnostics, takeSnapshots: true, out var snapshotManager);
 #if DEBUG
                 // Don't actually cache the results if the nullable analysis is not enabled in debug mode.
                 if (!Compilation.NullableSemanticAnalysisEnabled) return;

src/Compilers/CSharp/Portable/Compilation/MethodBodySemanticModel.cs

@@ -162,7 +162,7 @@ internal override bool TryGetSpeculativeSemanticModelCore(SyntaxTreeSemanticMode
 
             var methodSymbol = (MethodSymbol)this.MemberSymbol;
             binder = new ExecutableCodeBinder(statement, methodSymbol, binder);
-            speculativeModel = CreateSpeculative(parentModel, methodSymbol, statement, binder, SnapshotManager, position);
+            speculativeModel = CreateSpeculative(parentModel, methodSymbol, statement, binder, GetSnapshotManager(), position);
             return true;
         }
 

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<Symbol> 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);

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.SnapshotManager.cs

@@ -17,7 +17,8 @@ internal sealed class SnapshotManager
             /// <summary>
             /// The int key corresponds to <see cref="Snapshot.SharedStateIndex"/>.
             /// </summary>
-            private readonly ImmutableArray<SharedWalkerState> _walkerGlobalStates;
+            private readonly ImmutableArray<SharedWalkerState> _walkerSharedStates;
+
             /// <summary>
             /// The snapshot array should be sorted in ascending order by the position tuple element in order for the binary search algorithm to
             /// function correctly.
@@ -26,9 +27,9 @@ internal sealed class SnapshotManager
 
             private static readonly Func<(int position, Snapshot snapshot), int, int> BinarySearchComparer = (current, target) => current.position.CompareTo(target);
 
-            private SnapshotManager(ImmutableArray<SharedWalkerState> walkerGlobalStates, ImmutableArray<(int position, Snapshot snapshot)> incrementalSnapshots)
+            private SnapshotManager(ImmutableArray<SharedWalkerState> walkerSharedStates, ImmutableArray<(int position, Snapshot snapshot)> incrementalSnapshots)
             {
-                _walkerGlobalStates = walkerGlobalStates;
+                _walkerSharedStates = walkerSharedStates;
                 _incrementalSnapshots = incrementalSnapshots;
 
 #if DEBUG
@@ -43,11 +44,12 @@ private SnapshotManager(ImmutableArray<SharedWalkerState> walkerGlobalStates, Im
 #endif
             }
 
-            internal (NullableWalker, VariableState) RestoreWalkerToAnalyzeNewNode(
+            internal (NullableWalker, VariableState, Symbol) RestoreWalkerToAnalyzeNewNode(
                 int position,
                 BoundNode nodeToAnalyze,
                 Binder binder,
-                ImmutableDictionary<BoundExpression, (NullabilityInfo, TypeSymbol)>.Builder analyzedNullabilityMap)
+                ImmutableDictionary<BoundExpression, (NullabilityInfo, TypeSymbol)>.Builder analyzedNullabilityMap,
+                SnapshotManager.Builder newManagerOpt)
             {
                 var snapshotPosition = _incrementalSnapshots.BinarySearch(position, BinarySearchComparer);
 
@@ -61,11 +63,11 @@ private SnapshotManager(ImmutableArray<SharedWalkerState> walkerGlobalStates, Im
                 }
 
                 (_, Snapshot incrementalSnapshot) = _incrementalSnapshots[snapshotPosition];
-                var globalState = _walkerGlobalStates[incrementalSnapshot.SharedStateIndex];
-                var variableState = new VariableState(globalState.VariableSlot, globalState.VariableBySlot, globalState.VariableTypes, incrementalSnapshot.VariableState.Clone());
-                var method = globalState.Symbol as MethodSymbol;
+                var sharedState = _walkerSharedStates[incrementalSnapshot.SharedStateIndex];
+                var variableState = new VariableState(sharedState.VariableSlot, sharedState.VariableBySlot, sharedState.VariableTypes, incrementalSnapshot.VariableState.Clone());
+                var method = sharedState.Symbol as MethodSymbol;
                 return (new NullableWalker(binder.Compilation,
-                                           globalState.Symbol,
+                                           sharedState.Symbol,
                                            useMethodSignatureParameterTypes: !(method is null),
                                            method,
                                            nodeToAnalyze,
@@ -74,9 +76,10 @@ private SnapshotManager(ImmutableArray<SharedWalkerState> walkerGlobalStates, Im
                                            variableState,
                                            returnTypesOpt: null,
                                            analyzedNullabilityMap,
-                                           snapshotBuilderOpt: null,
+                                           snapshotBuilderOpt: newManagerOpt,
                                            isSpeculative: true),
-                        variableState);
+                        variableState,
+                        sharedState.Symbol);
             }
 
 #if DEBUG
@@ -94,7 +97,7 @@ internal void VerifyNode(BoundNode node)
                 {
                     Debug.Fail($"Did not find a snapshot for {node} `{node.Syntax}.`");
                 }
-                Debug.Assert(_walkerGlobalStates.Length > _incrementalSnapshots[position].snapshot.SharedStateIndex, $"Did not find global state for {node} `{node.Syntax}`.");
+                Debug.Assert(_walkerSharedStates.Length > _incrementalSnapshots[position].snapshot.SharedStateIndex, $"Did not find shared state for {node} `{node.Syntax}`.");
             }
 #endif
 
@@ -108,7 +111,7 @@ internal sealed class Builder
                 private readonly SortedDictionary<int, Snapshot> _incrementalSnapshots = new SortedDictionary<int, Snapshot>();
                 /// <summary>
                 /// Every walker is walking a specific symbol, and can potentially walk each symbol multiple times
-                /// to get to a stable state. Each of these symbols gets a single global state slot, which this
+                /// to get to a stable state. Each of these symbols gets a single shared state slot, which this
                 /// dictionary keeps track of. These slots correspond to indexes into <see cref="_walkerStates"/>.
                 /// </summary>
                 private readonly PooledDictionary<Symbol, int> _symbolToSlot = PooledDictionary<Symbol, int>.GetInstance();
@@ -198,10 +201,10 @@ private readonly struct Snapshot
             internal readonly LocalState VariableState;
             internal readonly int SharedStateIndex;
 
-            internal Snapshot(LocalState variableState, int globalStateIndex)
+            internal Snapshot(LocalState variableState, int sharedStateIndex)
             {
                 VariableState = variableState;
-                SharedStateIndex = globalStateIndex;
+                SharedStateIndex = sharedStateIndex;
             }
         }
     }