Port "Incremental generator nodes must run when no result history" to 17.4

src/Compilers/CSharp/Test/Semantic/SourceGeneration/GeneratorDriverTests.cs

@@ -3196,5 +3196,89 @@ class D {  (int, bool) _field; }";
             diagnostics.Verify();
             compilation.VerifyDiagnostics();
         }
+
+        [Fact]
+        public void IncrementalGenerator_Add_New_Generator_After_Generation()
+        {
+            // 1. run a generator, smuggling out some inputs from context
+            // 2. add a second generator, re-using the inputs from the first step and using a Combine node
+            // 3. run the new graph
+
+            var source = @"
+class C { }
+";
+            Compilation compilation = CreateCompilation(source, options: TestOptions.DebugDllThrowing);
+            compilation.VerifyDiagnostics();
+
+            IncrementalValueProvider<ParseOptions> parseOptionsProvider = default;
+            IncrementalValueProvider<AnalyzerConfigOptionsProvider> configOptionsProvider = default;
+
+            var generator = new IncrementalGeneratorWrapper(new PipelineCallbackGenerator(ctx =>
+            {
+                var source = parseOptionsProvider = ctx.ParseOptionsProvider;
+                var source2 = configOptionsProvider = ctx.AnalyzerConfigOptionsProvider;
+                var combine = source.Combine(source2);
+                ctx.RegisterSourceOutput(combine, (spc, c) => { });
+            }));
+
+            GeneratorDriver driver = CSharpGeneratorDriver.Create(new ISourceGenerator[] { generator });
+            driver = driver.RunGenerators(compilation);
+
+            // parse options and analyzer options are now cached
+            // add a new generator that depends on them
+            bool wasCalled = false;
+            var generator2 = new IncrementalGeneratorWrapper(new PipelineCallbackGenerator2(ctx =>
+            {
+                var source = parseOptionsProvider;
+                var source2 = configOptionsProvider;
+                // this call should always be made, even though the above inputs are cached
+                var transform = source.Select((a, _) => { wasCalled = true; return new object(); });
+                // now combine source2 with the transform. Combine will call single on transform, and we'll crash if it wasn't called
+                var combine = source2.Combine(transform);
+                ctx.RegisterSourceOutput(combine, (spc, c) => { });
+            }));
+
+            driver = driver.AddGenerators(ImmutableArray.Create<ISourceGenerator>(generator2));
+            driver = driver.RunGenerators(compilation);
+            Assert.True(wasCalled);
+        }
+
+        [Fact]
+        public void IncrementalGenerator_Add_New_Generator_After_Generation_SourceOutputNode()
+        {
+            // 1. run a generator, smuggling out some inputs from context
+            // 2. add a second generator, re-using the inputs from the first step
+            // 3. run the new graph
+
+            var source = @"
+class C { }
+";
+            Compilation compilation = CreateCompilation(source, options: TestOptions.DebugDllThrowing);
+            compilation.VerifyDiagnostics();
+
+            IncrementalValueProvider<ParseOptions> parseOptionsProvider = default;
+
+            var generator = new IncrementalGeneratorWrapper(new PipelineCallbackGenerator(ctx =>
+            {
+                var source = parseOptionsProvider = ctx.ParseOptionsProvider;
+                ctx.RegisterSourceOutput(source, (spc, c) => { });
+            }));
+
+            GeneratorDriver driver = CSharpGeneratorDriver.Create(new ISourceGenerator[] { generator });
+            driver = driver.RunGenerators(compilation);
+
+            // parse options are now cached
+            // add a new generator that depends on them
+            bool wasCalled = false;
+            var generator2 = new IncrementalGeneratorWrapper(new PipelineCallbackGenerator2(ctx =>
+            {
+                var source = parseOptionsProvider;
+                ctx.RegisterSourceOutput(source, (spc, c) => { wasCalled = true; });
+            }));
+
+            driver = driver.AddGenerators(ImmutableArray.Create<ISourceGenerator>(generator2));
+            driver = driver.RunGenerators(compilation);
+            Assert.True(wasCalled);
+        }
     }
 }

src/Compilers/CSharp/Test/Semantic/SourceGeneration/StateTableTests.cs

@@ -1036,8 +1036,9 @@ public CallbackNode(Func<DriverStateTable.Builder, NodeStateTable<T>, NodeStateT
                 _callback = callback;
             }
 
-            public NodeStateTable<T> UpdateStateTable(DriverStateTable.Builder graphState, NodeStateTable<T> previousTable, CancellationToken cancellationToken)
+            public NodeStateTable<T> UpdateStateTable(DriverStateTable.Builder graphState, NodeStateTable<T>? previousTable, CancellationToken cancellationToken)
             {
+                previousTable ??= NodeStateTable<T>.Empty;
                 return _callback(graphState, previousTable);
             }
 

src/Compilers/Core/Portable/SourceGeneration/Nodes/BatchNode.cs

@@ -30,7 +30,7 @@ public BatchNode(IIncrementalGeneratorNode<TInput> sourceNode, IEqualityComparer
 
         private (ImmutableArray<TInput>, ImmutableArray<(IncrementalGeneratorRunStep InputStep, int OutputIndex)>) GetValuesAndInputs(
             NodeStateTable<TInput> sourceTable,
-            NodeStateTable<ImmutableArray<TInput>> previousTable,
+            NodeStateTable<ImmutableArray<TInput>>? previousTable,
             NodeStateTable<ImmutableArray<TInput>>.Builder newTable)
         {
             // Do an initial pass to both get the steps, and determine how many entries we'll have.
@@ -56,6 +56,9 @@ public BatchNode(IIncrementalGeneratorNode<TInput> sourceNode, IEqualityComparer
 
             ImmutableArray<TInput>? tryReusePreviousTableValues(int entryCount)
             {
+                if (previousTable is null)
+                    return null;
+
                 if (previousTable.Count != 1)
                     return null;
 
@@ -103,7 +106,7 @@ ImmutableArray<TInput> computeCurrentTableValues(int entryCount)
             }
         }
 
-        public NodeStateTable<ImmutableArray<TInput>> UpdateStateTable(DriverStateTable.Builder builder, NodeStateTable<ImmutableArray<TInput>> previousTable, CancellationToken cancellationToken)
+        public NodeStateTable<ImmutableArray<TInput>> UpdateStateTable(DriverStateTable.Builder builder, NodeStateTable<ImmutableArray<TInput>>? previousTable, CancellationToken cancellationToken)
         {
             // grab the source inputs
             var sourceTable = builder.GetLatestStateTableForNode(_sourceNode);
@@ -125,7 +128,7 @@ public NodeStateTable<ImmutableArray<TInput>> UpdateStateTable(DriverStateTable.
 
             var (sourceValues, sourceInputs) = GetValuesAndInputs(sourceTable, previousTable, newTable);
 
-            if (previousTable.IsEmpty)
+            if (previousTable is null || previousTable.IsEmpty)
             {
                 newTable.AddEntry(sourceValues, EntryState.Added, stopwatch.Elapsed, sourceInputs, EntryState.Added);
             }

src/Compilers/Core/Portable/SourceGeneration/Nodes/CombineNode.cs

@@ -26,13 +26,13 @@ public CombineNode(IIncrementalGeneratorNode<TInput1> input1, IIncrementalGenera
             _name = name;
         }
 
-        public NodeStateTable<(TInput1, TInput2)> UpdateStateTable(DriverStateTable.Builder graphState, NodeStateTable<(TInput1, TInput2)> previousTable, CancellationToken cancellationToken)
+        public NodeStateTable<(TInput1, TInput2)> UpdateStateTable(DriverStateTable.Builder graphState, NodeStateTable<(TInput1, TInput2)>? previousTable, CancellationToken cancellationToken)
         {
             // get both input tables
             var input1Table = graphState.GetLatestStateTableForNode(_input1);
             var input2Table = graphState.GetLatestStateTableForNode(_input2);
 
-            if (input1Table.IsCached && input2Table.IsCached)
+            if (input1Table.IsCached && input2Table.IsCached && previousTable is not null)
             {
                 if (graphState.DriverState.TrackIncrementalSteps)
                 {

src/Compilers/Core/Portable/SourceGeneration/Nodes/DriverStateTable.cs

@@ -54,7 +54,7 @@ public NodeStateTable<T> GetLatestStateTableForNode<T>(IIncrementalGeneratorNode
                 }
 
                 // get the previous table, if there was one for this node
-                NodeStateTable<T> previousTable = _previousTable._tables.GetStateTableOrEmpty<T>(source);
+                NodeStateTable<T>? previousTable = _previousTable._tables.GetStateTable<T>(source);
 
                 // request the node update its state based on the current driver table and store the new result
                 var newTable = source.UpdateStateTable(this, previousTable, _cancellationToken);
@@ -63,8 +63,9 @@ public NodeStateTable<T> GetLatestStateTableForNode<T>(IIncrementalGeneratorNode
             }
 
             public NodeStateTable<T>.Builder CreateTableBuilder<T>(
-                NodeStateTable<T> previousTable, string? stepName, IEqualityComparer<T>? equalityComparer, int? tableCapacity = null)
+                NodeStateTable<T>? previousTable, string? stepName, IEqualityComparer<T>? equalityComparer, int? tableCapacity = null)
             {
+                previousTable ??= NodeStateTable<T>.Empty;
                 return previousTable.ToBuilder(stepName, DriverState.TrackIncrementalSteps, equalityComparer, tableCapacity);
             }
 

src/Compilers/Core/Portable/SourceGeneration/Nodes/IIncrementalGeneratorNode.cs

@@ -3,7 +3,6 @@
 // See the LICENSE file in the project root for more information.
 
 using System.Collections.Generic;
-using System.Collections.Immutable;
 using System.Threading;
 
 namespace Microsoft.CodeAnalysis
@@ -14,7 +13,7 @@ namespace Microsoft.CodeAnalysis
     /// <typeparam name="T">The type of value this step operates on</typeparam>
     internal interface IIncrementalGeneratorNode<T>
     {
-        NodeStateTable<T> UpdateStateTable(DriverStateTable.Builder graphState, NodeStateTable<T> previousTable, CancellationToken cancellationToken);
+        NodeStateTable<T> UpdateStateTable(DriverStateTable.Builder graphState, NodeStateTable<T>? previousTable, CancellationToken cancellationToken);
 
         IIncrementalGeneratorNode<T> WithComparer(IEqualityComparer<T> comparer);
 

src/Compilers/Core/Portable/SourceGeneration/Nodes/InputNode.cs

@@ -38,7 +38,7 @@ private InputNode(Func<DriverStateTable.Builder, ImmutableArray<T>> getInput, Ac
             _name = name;
         }
 
-        public NodeStateTable<T> UpdateStateTable(DriverStateTable.Builder graphState, NodeStateTable<T> previousTable, CancellationToken cancellationToken)
+        public NodeStateTable<T> UpdateStateTable(DriverStateTable.Builder graphState, NodeStateTable<T>? previousTable, CancellationToken cancellationToken)
         {
             var stopwatch = SharedStopwatch.StartNew();
             var inputItems = _getInput(graphState);
@@ -57,32 +57,35 @@ public NodeStateTable<T> UpdateStateTable(DriverStateTable.Builder graphState, N
             // We always have no inputs steps into an InputNode, but we track the difference between "no inputs" (empty collection) and "no step information" (default value)
             var noInputStepsStepInfo = builder.TrackIncrementalSteps ? ImmutableArray<(IncrementalGeneratorRunStep, int)>.Empty : default;
 
-            // for each item in the previous table, check if its still in the new items
-            int itemIndex = 0;
-            foreach (var (oldItem, _, _, _) in previousTable)
+            if (previousTable is not null)
             {
-                if (itemsSet.Remove(oldItem))
+                // for each item in the previous table, check if its still in the new items
+                int itemIndex = 0;
+                foreach (var (oldItem, _, _, _) in previousTable)
                 {
-                    // we're iterating the table, so know that it has entries
-                    var usedCache = builder.TryUseCachedEntries(elapsedTime, noInputStepsStepInfo);
-                    Debug.Assert(usedCache);
+                    if (itemsSet.Remove(oldItem))
+                    {
+                        // we're iterating the table, so know that it has entries
+                        var usedCache = builder.TryUseCachedEntries(elapsedTime, noInputStepsStepInfo);
+                        Debug.Assert(usedCache);
+                    }
+                    else if (inputItems.Length == previousTable.Count)
+                    {
+                        // When the number of items matches the previous iteration, we use a heuristic to mark the input as modified
+                        // This allows us to correctly 'replace' items even when they aren't actually the same. In the case that the
+                        // item really isn't modified, but a new item, we still function correctly as we mostly treat them the same,
+                        // but will perform an extra comparison that is omitted in the pure 'added' case.
+                        var modified = builder.TryModifyEntry(inputItems[itemIndex], _comparer, elapsedTime, noInputStepsStepInfo, EntryState.Modified);
+                        Debug.Assert(modified);
+                        itemsSet.Remove(inputItems[itemIndex]);
+                    }
+                    else
+                    {
+                        var removed = builder.TryRemoveEntries(elapsedTime, noInputStepsStepInfo);
+                        Debug.Assert(removed);
+                    }
+                    itemIndex++;
                 }
-                else if (inputItems.Length == previousTable.Count)
-                {
-                    // When the number of items matches the previous iteration, we use a heuristic to mark the input as modified
-                    // This allows us to correctly 'replace' items even when they aren't actually the same. In the case that the
-                    // item really isn't modified, but a new item, we still function correctly as we mostly treat them the same,
-                    // but will perform an extra comparison that is omitted in the pure 'added' case.
-                    var modified = builder.TryModifyEntry(inputItems[itemIndex], _comparer, elapsedTime, noInputStepsStepInfo, EntryState.Modified);
-                    Debug.Assert(modified);
-                    itemsSet.Remove(inputItems[itemIndex]);
-                }
-                else
-                {
-                    var removed = builder.TryRemoveEntries(elapsedTime, noInputStepsStepInfo);
-                    Debug.Assert(removed);
-                }
-                itemIndex++;
             }
 
             // any remaining new items are added

src/Compilers/Core/Portable/SourceGeneration/Nodes/SourceOutputNode.cs

@@ -35,11 +35,11 @@ public SourceOutputNode(IIncrementalGeneratorNode<TInput> source, Action<SourceP
 
         public IncrementalGeneratorOutputKind Kind => _outputKind;
 
-        public NodeStateTable<TOutput> UpdateStateTable(DriverStateTable.Builder graphState, NodeStateTable<TOutput> previousTable, CancellationToken cancellationToken)
+        public NodeStateTable<TOutput> UpdateStateTable(DriverStateTable.Builder graphState, NodeStateTable<TOutput>? previousTable, CancellationToken cancellationToken)
         {
             string stepName = Kind == IncrementalGeneratorOutputKind.Source ? WellKnownGeneratorOutputs.SourceOutput : WellKnownGeneratorOutputs.ImplementationSourceOutput;
             var sourceTable = graphState.GetLatestStateTableForNode(_source);
-            if (sourceTable.IsCached)
+            if (sourceTable.IsCached && previousTable is not null)
             {
                 if (graphState.DriverState.TrackIncrementalSteps)
                 {

src/Compilers/Core/Portable/SourceGeneration/Nodes/StateTableStore.cs

@@ -25,12 +25,17 @@ private StateTableStore(ImmutableSegmentedDictionary<object, IStateTable> tables
         public bool TryGetValue(object key, [NotNullWhen(true)] out IStateTable? table) => _tables.TryGetValue(key, out table);
 
         public NodeStateTable<T> GetStateTableOrEmpty<T>(object input)
+        {
+            return GetStateTable<T>(input) ?? NodeStateTable<T>.Empty;
+        }
+
+        public NodeStateTable<T>? GetStateTable<T>(object input)
         {
             if (TryGetValue(input, out var output))
             {
                 return (NodeStateTable<T>)output;
             }
-            return NodeStateTable<T>.Empty;
+            return null;
         }
 
         public sealed class Builder

src/Compilers/Core/Portable/SourceGeneration/Nodes/SyntaxInputNode.cs

@@ -28,7 +28,7 @@ internal SyntaxInputNode(ISyntaxSelectionStrategy<T> inputNode, Action<SyntaxInp
             _name = name;
         }
 
-        public NodeStateTable<T> UpdateStateTable(DriverStateTable.Builder graphState, NodeStateTable<T> previousTable, CancellationToken cancellationToken)
+        public NodeStateTable<T> UpdateStateTable(DriverStateTable.Builder graphState, NodeStateTable<T>? previousTable, CancellationToken cancellationToken)
         {
             return (NodeStateTable<T>)graphState.SyntaxStore.GetSyntaxInputTable(this, graphState.GetLatestStateTableForNode(SharedInputNodes.SyntaxTrees));
         }

src/Compilers/Core/Portable/SourceGeneration/Nodes/SyntaxReceiverStrategy.cs

@@ -31,7 +31,6 @@ public SyntaxReceiverStrategy(
 
         private sealed class Builder : ISyntaxInputBuilder
         {
-            private readonly SyntaxReceiverStrategy<T> _owner;
             private readonly object _key;
             private readonly NodeStateTable<ISyntaxContextReceiver?>.Builder _nodeStateTable;
             private readonly ISyntaxContextReceiver? _receiver;
@@ -40,7 +39,6 @@ private sealed class Builder : ISyntaxInputBuilder
 
             public Builder(SyntaxReceiverStrategy<T> owner, object key, StateTableStore driverStateTable, bool trackIncrementalSteps)
             {
-                _owner = owner;
                 _key = key;
                 _nodeStateTable = driverStateTable.GetStateTableOrEmpty<ISyntaxContextReceiver?>(_key).ToBuilder(stepName: null, trackIncrementalSteps);
                 try

src/Compilers/Core/Portable/SourceGeneration/Nodes/TransformNode.cs

@@ -39,11 +39,11 @@ public IIncrementalGeneratorNode<TOutput> WithComparer(IEqualityComparer<TOutput
         public IIncrementalGeneratorNode<TOutput> WithTrackingName(string name)
             => new TransformNode<TInput, TOutput>(_sourceNode, _func, _comparer, name);
 
-        public NodeStateTable<TOutput> UpdateStateTable(DriverStateTable.Builder builder, NodeStateTable<TOutput> previousTable, CancellationToken cancellationToken)
+        public NodeStateTable<TOutput> UpdateStateTable(DriverStateTable.Builder builder, NodeStateTable<TOutput>? previousTable, CancellationToken cancellationToken)
         {
             // grab the source inputs
             var sourceTable = builder.GetLatestStateTableForNode(_sourceNode);
-            if (sourceTable.IsCached)
+            if (sourceTable.IsCached && previousTable is not null)
             {
                 if (builder.DriverState.TrackIncrementalSteps)
                 {