From 2a3a17224d06faa72452757e33d251ec326e0e8f Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Tue, 29 Sep 2026 08:51:36 +0100 Subject: [PATCH 1/5] perf(source-gen): stop InfrastructureGenerator pinning an old Compilation InfrastructureGenerator used CompilationProvider.WithComparer(...) to skip the reference walk on syntax-only edits. When a comparer reports "equal", Roslyn's InputNode keeps the previous entry in its state table, so the driver held the first Compilation (syntax trees, bound state) alive until references changed, which in an IDE can be the whole session. Drop the comparer and memoize the reference walk in a ConditionalWeakTable keyed on the backing array of Compilation.ExternalReferences. Syntax-only edits keep that array, so the memo hits and AssemblyInfoModel equality keeps the generated source cached. Adding, removing, rebuilding or editing a reference produces a new array, so extraction still reruns (keeps #6912). The cache holds only strings, never a Compilation or MetadataReference. Co-Authored-By: Claude --- ...pilationTriggerOnEveryKeystrokeComparer.cs | 78 ------------------- .../CodeGenerators/InfrastructureGenerator.cs | 65 ++++++++++++++-- ...InfrastructureGeneratorIncrementalTests.cs | 54 ++++++++++++- 3 files changed, 112 insertions(+), 85 deletions(-) delete mode 100644 src/TUnit.Core.SourceGenerator/CodeGenerators/Equality/PreventCompilationTriggerOnEveryKeystrokeComparer.cs diff --git a/src/TUnit.Core.SourceGenerator/CodeGenerators/Equality/PreventCompilationTriggerOnEveryKeystrokeComparer.cs b/src/TUnit.Core.SourceGenerator/CodeGenerators/Equality/PreventCompilationTriggerOnEveryKeystrokeComparer.cs deleted file mode 100644 index bf9148cd6b3..00000000000 --- a/src/TUnit.Core.SourceGenerator/CodeGenerators/Equality/PreventCompilationTriggerOnEveryKeystrokeComparer.cs +++ /dev/null @@ -1,78 +0,0 @@ -using System.Collections.Immutable; -using Microsoft.CodeAnalysis; - -namespace TUnit.Core.SourceGenerator.CodeGenerators.Equality; - -/// -/// Treats two compilations as equal when they would produce the same reference-derived output: -/// same language, assembly name and metadata references. Syntax-only edits (ordinary keystrokes) -/// keep the same instances, so they compare equal and the -/// downstream reference walk is skipped. Adding, removing, rebuilding or editing a reference does not. -/// -public class PreventCompilationTriggerOnEveryKeystrokeComparer : IEqualityComparer -{ - public bool Equals(Compilation? x, Compilation? y) - { - if (ReferenceEquals(x, y)) - { - return true; - } - - if (x is null) - { - return false; - } - - if (y is null) - { - return false; - } - - if (x.GetType() != y.GetType()) - { - return false; - } - - return x.Language == y.Language - && x.AssemblyName == y.AssemblyName - && ReferencesEqual(x.ExternalReferences, y.ExternalReferences) - && ReferencesEqual(x.DirectiveReferences, y.DirectiveReferences); - } - - public int GetHashCode(Compilation obj) - { - unchecked - { - return (obj.Language.GetHashCode() * 397) ^ (obj.AssemblyName != null ? obj.AssemblyName.GetHashCode() : 0); - } - } - - private static bool ReferencesEqual(ImmutableArray x, ImmutableArray y) - { - // Syntax-only edits usually keep the same backing array, so this avoids the element walk. - if (x == y) - { - return true; - } - - if (x.Length != y.Length) - { - return false; - } - - for (var i = 0; i < x.Length; i++) - { - // Hosts reuse reference instances while the referenced file or project is unchanged. - // A new instance means the reference was added, rebuilt or (for an IDE project - // reference) its source was edited, which can change the types and dependencies the - // reference walk selects. Rerun extraction then; AssemblyInfoModel equality keeps the - // generated source cached when the extracted model is unchanged. - if (!ReferenceEquals(x[i], y[i])) - { - return false; - } - } - - return true; - } -} diff --git a/src/TUnit.Core.SourceGenerator/CodeGenerators/InfrastructureGenerator.cs b/src/TUnit.Core.SourceGenerator/CodeGenerators/InfrastructureGenerator.cs index a9746741288..72834d3a78b 100644 --- a/src/TUnit.Core.SourceGenerator/CodeGenerators/InfrastructureGenerator.cs +++ b/src/TUnit.Core.SourceGenerator/CodeGenerators/InfrastructureGenerator.cs @@ -1,5 +1,6 @@ +using System.Collections.Immutable; +using System.Runtime.CompilerServices; using Microsoft.CodeAnalysis; -using TUnit.Core.SourceGenerator.CodeGenerators.Equality; using TUnit.Core.SourceGenerator.Models; using TUnit.Core.SourceGenerator.Models.Extracted; @@ -57,11 +58,13 @@ public void Initialize(IncrementalGeneratorInitializationContext context) return !string.Equals(value, "false", StringComparison.OrdinalIgnoreCase); }); - // Extract assembly names as primitives in the transform step - // This enables proper incremental caching + // Extract assembly names as primitives in the transform step. AssemblyInfoModel equality + // keeps the generated source cached across keystrokes. No custom Compilation comparer here: + // when a comparer reports "equal", the input node keeps the OLD compilation in its table, + // pinning it (trees, bound state) until references change. The reference walk is memoized + // on the reference list instead, so syntax-only edits still skip it. var assemblyInfoProvider = context.CompilationProvider - .WithComparer(new PreventCompilationTriggerOnEveryKeystrokeComparer()) - .Select((compilation, _) => ExtractAssemblyInfo(compilation)) + .Select((compilation, _) => GetAssemblyInfo(compilation)) .WithTrackingName(ExtractAssemblyInfoStep) .Combine(enabledProvider); @@ -77,6 +80,58 @@ public void Initialize(IncrementalGeneratorInitializationContext context) }); } + /// + /// Returns the memoized model while the compilation keeps the same reference list. Syntax-only + /// edits reuse the backing array of ; adding, removing, + /// rebuilding or (for an IDE project reference) editing a reference produces a new array, so + /// extraction reruns. The table is keyed weakly on that array and its values hold only strings, + /// so no Compilation or MetadataReference is kept alive by the cache. + /// + private static AssemblyInfoModel GetAssemblyInfo(Compilation compilation) + { + // Directive references (#r in scripts) are not part of the key; skip memoization for them. + if (!compilation.DirectiveReferences.IsEmpty) + { + return ExtractAssemblyInfo(compilation); + } + + var references = compilation.ExternalReferences; + var key = Unsafe.As, MetadataReference[]?>(ref references); + if (key is null) + { + return ExtractAssemblyInfo(compilation); + } + + var holder = AssemblyInfoCache.GetValue(key, static _ => new AssemblyInfoCacheEntry()); + var cached = holder.Value; + if (cached is not null + && cached.Language == compilation.Language + && cached.AssemblyName == compilation.AssemblyName) + { + return cached.Model; + } + + var model = ExtractAssemblyInfo(compilation); + holder.Value = new CachedAssemblyInfo(compilation.Language, compilation.AssemblyName, model); + return model; + } + + private static readonly ConditionalWeakTable AssemblyInfoCache = new(); + + private sealed class AssemblyInfoCacheEntry + { + public volatile CachedAssemblyInfo? Value; + } + + private sealed class CachedAssemblyInfo(string language, string? assemblyName, AssemblyInfoModel model) + { + public string Language { get; } = language; + + public string? AssemblyName { get; } = assemblyName; + + public AssemblyInfoModel Model { get; } = model; + } + /// /// Extracts all needed data as primitives in the transform step. /// This enables proper incremental caching - the model contains only strings. diff --git a/tests/TUnit.SourceGenerator.IncrementalTests/InfrastructureGeneratorIncrementalTests.cs b/tests/TUnit.SourceGenerator.IncrementalTests/InfrastructureGeneratorIncrementalTests.cs index 2bd2c4f274a..13db0d59f71 100644 --- a/tests/TUnit.SourceGenerator.IncrementalTests/InfrastructureGeneratorIncrementalTests.cs +++ b/tests/TUnit.SourceGenerator.IncrementalTests/InfrastructureGeneratorIncrementalTests.cs @@ -1,3 +1,4 @@ +using System.Runtime.CompilerServices; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using TUnit.Core.SourceGenerator.CodeGenerators; @@ -26,10 +27,15 @@ public void EditSource_ShouldNotRegenerate() var driver1 = TestHelper.GenerateTracked(compilation1); AssertRunReason(driver1, IncrementalStepRunReason.New); + var model1 = GetExtractedModel(driver1); + // Syntax-only edits rerun the cheap Select, which returns the memoized model (same instance, + // so the reference walk was skipped) and keeps the generated source cached. var compilation2 = compilation1.AddSyntaxTrees(CSharpSyntaxTree.ParseText("struct MyValue {}")); var driver2 = driver1.RunGenerators(compilation2); - AssertRunReason(driver2, IncrementalStepRunReason.Cached); + AssertRunReason(driver2, IncrementalStepRunReason.Unchanged); + Xunit.Assert.Same(model1, GetExtractedModel(driver2)); + TestHelper.AssertSourceOutputsCached(driver2.GetRunResult().Results[0]); var compilation3 = TestHelper.ReplaceMethodDeclaration(compilation1, "Test1", """ @@ -40,9 +46,53 @@ public void Test1() } """); var driver3 = driver2.RunGenerators(compilation3); - AssertRunReason(driver3, IncrementalStepRunReason.Cached); + AssertRunReason(driver3, IncrementalStepRunReason.Unchanged); + Xunit.Assert.Same(model1, GetExtractedModel(driver3)); + TestHelper.AssertSourceOutputsCached(driver3.GetRunResult().Results[0]); + } + + [Fact] + public void EditSource_DoesNotRetainPreviousCompilation() + { + var (driver, firstCompilation, latestCompilation) = RunSourceEdits(10); + + for (var i = 0; i < 3; i++) + { + GC.Collect(); + GC.WaitForPendingFinalizers(); + } + + // A comparer that reports the new compilation as equal makes the input node keep the old + // one, pinning its syntax trees and bound state for as long as the driver lives. + Xunit.Assert.False(firstCompilation.IsAlive, "The generator driver kept the first compilation alive after syntax-only edits."); + GC.KeepAlive(driver); + GC.KeepAlive(latestCompilation); } + [MethodImpl(MethodImplOptions.NoInlining)] + private static (GeneratorDriver Driver, WeakReference FirstCompilation, Compilation LatestCompilation) RunSourceEdits(int edits) + { + Compilation compilation = Fixture.CreateLibrary(CSharpSyntaxTree.ParseText(DefaultSource, CSharpParseOptions.Default)); + var firstCompilation = new WeakReference(compilation); + + var driver = CSharpGeneratorDriver + .Create([new InfrastructureGenerator().AsSourceGenerator()]) + .RunGenerators(compilation); + + for (var i = 0; i < edits; i++) + { + compilation = compilation.ReplaceSyntaxTree( + compilation.SyntaxTrees.First(), + CSharpSyntaxTree.ParseText(DefaultSource + $"\npublic class Edit{i} {{ }}", CSharpParseOptions.Default)); + driver = driver.RunGenerators(compilation); + } + + return (driver, firstCompilation, compilation); + } + + private static object? GetExtractedModel(GeneratorDriver driver) => + driver.GetRunResult().Results[0].TrackedSteps[InfrastructureGenerator.ExtractAssemblyInfoStep][0].Outputs[0].Value; + [Fact] public void AddReference_ShouldRegenerate() { From 242761574f7d07ba60ee8ec39178336a92522735 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Tue, 29 Sep 2026 09:35:18 +0100 Subject: [PATCH 2/5] fix(source-gen): keep InfrastructureGenerator memo correct when source shadows a selected type GetTypeByMetadataName prefers a source type over a referenced one, so the selected typeof() targets can depend on source. The reference-keyed memo is now only stored when no candidate was shadowed, and on reuse each selected type is checked against the current source assembly; if any is shadowed, extraction reruns. Returned models now always match a fresh extraction, including across independent drivers sharing a reference array. --- .../CodeGenerators/InfrastructureGenerator.cs | 77 ++++++++++++++++--- ...InfrastructureGeneratorIncrementalTests.cs | 64 +++++++++++++++ 2 files changed, 132 insertions(+), 9 deletions(-) diff --git a/src/TUnit.Core.SourceGenerator/CodeGenerators/InfrastructureGenerator.cs b/src/TUnit.Core.SourceGenerator/CodeGenerators/InfrastructureGenerator.cs index 72834d3a78b..35209d25c4f 100644 --- a/src/TUnit.Core.SourceGenerator/CodeGenerators/InfrastructureGenerator.cs +++ b/src/TUnit.Core.SourceGenerator/CodeGenerators/InfrastructureGenerator.cs @@ -86,36 +86,67 @@ public void Initialize(IncrementalGeneratorInitializationContext context) /// rebuilding or (for an IDE project reference) editing a reference produces a new array, so /// extraction reruns. The table is keyed weakly on that array and its values hold only strings, /// so no Compilation or MetadataReference is kept alive by the cache. + /// + /// The selection is reference-derived except for one source dependency: + /// prefers a source type over a referenced type with + /// the same metadata name, so source can shadow a candidate. The memo is therefore only stored + /// when no candidate was shadowed (the selection then equals a purely reference-based one), and on + /// reuse each selected type is checked against the current source; if any is now shadowed, + /// extraction reruns. The table is static and can be hit by independent drivers that share a + /// reference array, so this keeps every returned model identical to a fresh extraction. + /// /// private static AssemblyInfoModel GetAssemblyInfo(Compilation compilation) { // Directive references (#r in scripts) are not part of the key; skip memoization for them. if (!compilation.DirectiveReferences.IsEmpty) { - return ExtractAssemblyInfo(compilation); + return ExtractAssemblyInfo(compilation, out _); } + // Deliberate layout assumption: ImmutableArray is a struct wrapping a single T[] field + // (ImmutableCollectionsMarshal.AsArray does the same, but is not available to netstandard2.0 + // analyzers). The incremental tests for added/replaced references guard this. var references = compilation.ExternalReferences; var key = Unsafe.As, MetadataReference[]?>(ref references); if (key is null) { - return ExtractAssemblyInfo(compilation); + return ExtractAssemblyInfo(compilation, out _); } var holder = AssemblyInfoCache.GetValue(key, static _ => new AssemblyInfoCacheEntry()); var cached = holder.Value; if (cached is not null && cached.Language == compilation.Language - && cached.AssemblyName == compilation.AssemblyName) + && cached.AssemblyName == compilation.AssemblyName + && !IsAnyShadowedBySource(compilation, cached.SelectedMetadataNames)) { return cached.Model; } - var model = ExtractAssemblyInfo(compilation); - holder.Value = new CachedAssemblyInfo(compilation.Language, compilation.AssemblyName, model); + var model = ExtractAssemblyInfo(compilation, out var selectedMetadataNames); + if (selectedMetadataNames is not null) + { + holder.Value = new CachedAssemblyInfo(compilation.Language, compilation.AssemblyName, model, selectedMetadataNames); + } + return model; } + private static bool IsAnyShadowedBySource(Compilation compilation, string[] selectedMetadataNames) + { + var sourceAssembly = compilation.Assembly; + foreach (var metadataName in selectedMetadataNames) + { + if (sourceAssembly.GetTypeByMetadataName(metadataName) is not null) + { + return true; + } + } + + return false; + } + private static readonly ConditionalWeakTable AssemblyInfoCache = new(); private sealed class AssemblyInfoCacheEntry @@ -123,22 +154,31 @@ private sealed class AssemblyInfoCacheEntry public volatile CachedAssemblyInfo? Value; } - private sealed class CachedAssemblyInfo(string language, string? assemblyName, AssemblyInfoModel model) + private sealed class CachedAssemblyInfo(string language, string? assemblyName, AssemblyInfoModel model, string[] selectedMetadataNames) { public string Language { get; } = language; public string? AssemblyName { get; } = assemblyName; public AssemblyInfoModel Model { get; } = model; + + public string[] SelectedMetadataNames { get; } = selectedMetadataNames; } /// /// Extracts all needed data as primitives in the transform step. /// This enables proper incremental caching - the model contains only strings. /// - private static AssemblyInfoModel ExtractAssemblyInfo(Compilation compilation) + /// The compilation to inspect. + /// + /// Metadata names of the selected types, or when a source type shadowed a + /// candidate, in which case the result depends on source and must not be memoized. + /// + private static AssemblyInfoModel ExtractAssemblyInfo(Compilation compilation, out string[]? selectedMetadataNames) { var assembliesToLoad = new List(); + var metadataNames = new List(); + var shadowedBySource = false; // Find TUnit.Core assembly - only assemblies referencing this can contain tests var tunitCoreAssembly = FindTUnitCoreAssembly(compilation); @@ -177,14 +217,17 @@ private static AssemblyInfoModel ExtractAssemblyInfo(Compilation compilation) { if (ShouldLoadAssembly(assembly, compilation)) { - var publicType = GetFirstUniquePublicType(assembly, compilation); + var publicType = GetFirstUniquePublicType(assembly, compilation, out var metadataName, ref shadowedBySource); if (publicType != null) { assembliesToLoad.Add(publicType); + metadataNames.Add(metadataName!); } } } + selectedMetadataNames = shadowedBySource ? null : [.. metadataNames]; + return new AssemblyInfoModel { AssemblyName = compilation.Assembly.Name, @@ -331,7 +374,11 @@ private static bool IsLoadableAtRuntime(IAssemblySymbol assembly, Compilation co /// Gets the first public type from an assembly that can be uniquely resolved by the compilation. /// This avoids CS0433 errors when multiple assemblies define types with the same fully-qualified name. /// - private static string? GetFirstUniquePublicType(IAssemblySymbol assembly, Compilation compilation) + private static string? GetFirstUniquePublicType( + IAssemblySymbol assembly, + Compilation compilation, + out string? selectedMetadataName, + ref bool shadowedBySource) { foreach (var type in GetPublicTypesRecursive(assembly.GlobalNamespace)) { @@ -348,10 +395,12 @@ private static bool IsLoadableAtRuntime(IAssemblySymbol assembly, Compilation co // GetTypeByMetadataName returns null when the type name is ambiguous if (SymbolEqualityComparer.Default.Equals(resolvedType, type)) { + selectedMetadataName = metadataName; return type.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat); } // null or different type = ambiguous, try next type + shadowedBySource |= IsFromSource(resolvedType, compilation); } // Fallback: try generic types if no non-generic unique type was found @@ -382,13 +431,23 @@ private static bool IsLoadableAtRuntime(IAssemblySymbol assembly, Compilation co typeName = typeName.Substring(0, genericStart) + openGenericSuffix; } + selectedMetadataName = metadataName; return typeName; } + + shadowedBySource |= IsFromSource(resolvedType, compilation); } + selectedMetadataName = null; return null; // No unique type found, skip this assembly } + // GetTypeByMetadataName returns the compilation's own (source) type when one exists, hiding + // any referenced type with the same metadata name. + private static bool IsFromSource(INamedTypeSymbol? resolvedType, Compilation compilation) => + resolvedType is not null + && SymbolEqualityComparer.Default.Equals(resolvedType.ContainingAssembly, compilation.Assembly); + /// /// Gets the full metadata name for a type (e.g., "Namespace.OuterClass+NestedClass"). /// This is the format expected by Compilation.GetTypeByMetadataName(). diff --git a/tests/TUnit.SourceGenerator.IncrementalTests/InfrastructureGeneratorIncrementalTests.cs b/tests/TUnit.SourceGenerator.IncrementalTests/InfrastructureGeneratorIncrementalTests.cs index 13db0d59f71..0bf0f004015 100644 --- a/tests/TUnit.SourceGenerator.IncrementalTests/InfrastructureGeneratorIncrementalTests.cs +++ b/tests/TUnit.SourceGenerator.IncrementalTests/InfrastructureGeneratorIncrementalTests.cs @@ -146,6 +146,70 @@ public void EditProjectReference_ShouldRegenerate() AssertRunReason(driver3, IncrementalStepRunReason.Unchanged); } + private const string ShadowingSource = "namespace OtherTestLibrary { public class FirstHooks { } }"; + + [Fact] + public void FreshDriver_SameReferences_SourceShadowsSelectedType_SelectsNextType() + { + var baseCompilation = CreateWithTwoTypeLibrary(); + + // Populate the shared memo from an unrelated driver. + var unshadowed = GenerateInfrastructure(CSharpGeneratorDriver.Create(new InfrastructureGenerator()).RunGenerators(baseCompilation)); + Xunit.Assert.Contains("typeof(global::OtherTestLibrary.FirstHooks)", unshadowed); + + // Same reference array, but a source type now takes FirstHooks' name, so typeof() would bind + // to the source type and never initialize OtherTestLibrary. + var shadowedCompilation = baseCompilation.AddSyntaxTrees(CSharpSyntaxTree.ParseText(ShadowingSource)); + var shadowed = GenerateInfrastructure(CSharpGeneratorDriver.Create(new InfrastructureGenerator()).RunGenerators(shadowedCompilation)); + Xunit.Assert.Contains("typeof(global::OtherTestLibrary.SecondHooks)", shadowed); + Xunit.Assert.DoesNotContain("typeof(global::OtherTestLibrary.FirstHooks)", shadowed); + } + + [Fact] + public void FreshDriver_SameReferences_SourceSensitiveSelectionIsNotReused() + { + var baseCompilation = CreateWithTwoTypeLibrary(); + var shadowedCompilation = baseCompilation.AddSyntaxTrees(CSharpSyntaxTree.ParseText(ShadowingSource)); + + // The shadowed run must not memoize its (source-dependent) choice for later compilations. + var shadowed = GenerateInfrastructure(CSharpGeneratorDriver.Create(new InfrastructureGenerator()).RunGenerators(shadowedCompilation)); + Xunit.Assert.Contains("typeof(global::OtherTestLibrary.SecondHooks)", shadowed); + + var unshadowed = GenerateInfrastructure(CSharpGeneratorDriver.Create(new InfrastructureGenerator()).RunGenerators(baseCompilation)); + Xunit.Assert.Contains("typeof(global::OtherTestLibrary.FirstHooks)", unshadowed); + } + + [Fact] + public void EditSource_ShadowingSelectedType_ShouldRegenerate() + { + var compilation1 = CreateWithTwoTypeLibrary(); + var driver1 = TestHelper.GenerateTracked(compilation1); + Xunit.Assert.Contains("typeof(global::OtherTestLibrary.FirstHooks)", GenerateInfrastructure(driver1)); + + var compilation2 = compilation1.AddSyntaxTrees(CSharpSyntaxTree.ParseText(ShadowingSource)); + var driver2 = driver1.RunGenerators(compilation2); + AssertRunReason(driver2, IncrementalStepRunReason.Modified); + Xunit.Assert.Contains("typeof(global::OtherTestLibrary.SecondHooks)", GenerateInfrastructure(driver2)); + + var driver3 = driver2.RunGenerators(compilation1); + AssertRunReason(driver3, IncrementalStepRunReason.Modified); + Xunit.Assert.Contains("typeof(global::OtherTestLibrary.FirstHooks)", GenerateInfrastructure(driver3)); + } + + private static CSharpCompilation CreateWithTwoTypeLibrary() + { + var consumer = Fixture.CreateLibrary(CSharpSyntaxTree.ParseText(DefaultSource, CSharpParseOptions.Default)); + var library = CSharpCompilation.Create( + "OtherTestLibrary", + [CSharpSyntaxTree.ParseText("namespace OtherTestLibrary { public class FirstHooks { [TUnit.Core.Before(TUnit.Core.HookType.Assembly)] public static void Setup() { } } public class SecondHooks { } }")], + consumer.References, + new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)); + return consumer.AddReferences(library.ToMetadataReference()); + } + + private static string GenerateInfrastructure(GeneratorDriver driver) => + driver.GetRunResult().GeneratedTrees.Single(t => t.FilePath.EndsWith("TUnitInfrastructure.g.cs")).ToString(); + private static CSharpCompilation CreateHookLibrary(Compilation consumer, string typeName) => CSharpCompilation.Create( "OtherTestLibrary", From f5d8c5521c956e57f3fb624b7c1050c10f79996d Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Tue, 29 Sep 2026 11:05:44 +0100 Subject: [PATCH 3/5] fix(source-gen): key InfrastructureGenerator memo on reference-binding options Two compilations sharing one reference array can still bind references differently when AssemblyIdentityComparer, MetadataReferenceResolver or MetadataImportOptions differ, so the memo now only reuses a model extracted under the same options. Also verify the ImmutableArray single-field layout once at startup and disable the memo (never correctness, only speed) if it does not hold, and document that ExtractAssemblyInfo must stay reference-derived. --- .../CodeGenerators/InfrastructureGenerator.cs | 81 ++++++++++++++++--- ...InfrastructureGeneratorIncrementalTests.cs | 25 ++++++ 2 files changed, 93 insertions(+), 13 deletions(-) diff --git a/src/TUnit.Core.SourceGenerator/CodeGenerators/InfrastructureGenerator.cs b/src/TUnit.Core.SourceGenerator/CodeGenerators/InfrastructureGenerator.cs index 35209d25c4f..815ca4facea 100644 --- a/src/TUnit.Core.SourceGenerator/CodeGenerators/InfrastructureGenerator.cs +++ b/src/TUnit.Core.SourceGenerator/CodeGenerators/InfrastructureGenerator.cs @@ -84,8 +84,9 @@ public void Initialize(IncrementalGeneratorInitializationContext context) /// Returns the memoized model while the compilation keeps the same reference list. Syntax-only /// edits reuse the backing array of ; adding, removing, /// rebuilding or (for an IDE project reference) editing a reference produces a new array, so - /// extraction reruns. The table is keyed weakly on that array and its values hold only strings, - /// so no Compilation or MetadataReference is kept alive by the cache. + /// extraction reruns. The table is keyed weakly on that array and its values hold only strings and + /// the reference-binding option objects, so no Compilation or MetadataReference is kept alive by + /// the cache. /// /// The selection is reference-derived except for one source dependency: /// prefers a source type over a referenced type with @@ -104,11 +105,7 @@ private static AssemblyInfoModel GetAssemblyInfo(Compilation compilation) return ExtractAssemblyInfo(compilation, out _); } - // Deliberate layout assumption: ImmutableArray is a struct wrapping a single T[] field - // (ImmutableCollectionsMarshal.AsArray does the same, but is not available to netstandard2.0 - // analyzers). The incremental tests for added/replaced references guard this. - var references = compilation.ExternalReferences; - var key = Unsafe.As, MetadataReference[]?>(ref references); + var key = GetReferencesKey(compilation.ExternalReferences); if (key is null) { return ExtractAssemblyInfo(compilation, out _); @@ -117,8 +114,7 @@ private static AssemblyInfoModel GetAssemblyInfo(Compilation compilation) var holder = AssemblyInfoCache.GetValue(key, static _ => new AssemblyInfoCacheEntry()); var cached = holder.Value; if (cached is not null - && cached.Language == compilation.Language - && cached.AssemblyName == compilation.AssemblyName + && cached.Matches(compilation) && !IsAnyShadowedBySource(compilation, cached.SelectedMetadataNames)) { return cached.Model; @@ -127,12 +123,47 @@ private static AssemblyInfoModel GetAssemblyInfo(Compilation compilation) var model = ExtractAssemblyInfo(compilation, out var selectedMetadataNames); if (selectedMetadataNames is not null) { - holder.Value = new CachedAssemblyInfo(compilation.Language, compilation.AssemblyName, model, selectedMetadataNames); + holder.Value = new CachedAssemblyInfo(compilation, model, selectedMetadataNames); } return model; } + /// + /// Returns the array backing , or when the + /// layout check failed (memoization is then disabled, which costs speed but never correctness). + /// + private static MetadataReference[]? GetReferencesKey(ImmutableArray references) + { + // Deliberate layout assumption: ImmutableArray is a struct wrapping a single T[] field + // (ImmutableCollectionsMarshal.AsArray does the same, but is not available to netstandard2.0 + // analyzers). IsImmutableArrayLayoutAsExpected verifies it once at startup, and + // EditSource_ShouldNotRegenerate fails if the memo stops being used. + if (!IsImmutableArrayLayoutAsExpected) + { + return null; + } + + return Unsafe.As, MetadataReference[]?>(ref references); + } + + private static readonly bool IsImmutableArrayLayoutAsExpected = CheckImmutableArrayLayout(); + + private static bool CheckImmutableArrayLayout() + { + if (Unsafe.SizeOf>() != Unsafe.SizeOf()) + { + return false; + } + + var probe = ImmutableArray.Create(new object(), new object()); + var array = Unsafe.As, object[]?>(ref probe); + return array is not null + && array.Length == probe.Length + && ReferenceEquals(array[0], probe[0]) + && ReferenceEquals(array[1], probe[1]); + } + private static bool IsAnyShadowedBySource(Compilation compilation, string[] selectedMetadataNames) { var sourceAssembly = compilation.Assembly; @@ -154,20 +185,44 @@ private sealed class AssemblyInfoCacheEntry public volatile CachedAssemblyInfo? Value; } - private sealed class CachedAssemblyInfo(string language, string? assemblyName, AssemblyInfoModel model, string[] selectedMetadataNames) + private sealed class CachedAssemblyInfo(Compilation compilation, AssemblyInfoModel model, string[] selectedMetadataNames) { - public string Language { get; } = language; + private readonly string _language = compilation.Language; + private readonly string? _assemblyName = compilation.AssemblyName; - public string? AssemblyName { get; } = assemblyName; + // Options that change how references bind to assembly symbols (unification, implicitly + // resolved missing assemblies, imported metadata), and so can change the selection even + // when the reference array is the same. Only the comparer and resolver objects are held, + // never the Compilation. + private readonly AssemblyIdentityComparer _assemblyIdentityComparer = compilation.Options.AssemblyIdentityComparer; + private readonly MetadataReferenceResolver? _metadataReferenceResolver = compilation.Options.MetadataReferenceResolver; + private readonly MetadataImportOptions _metadataImportOptions = compilation.Options.MetadataImportOptions; public AssemblyInfoModel Model { get; } = model; public string[] SelectedMetadataNames { get; } = selectedMetadataNames; + + public bool Matches(Compilation compilation) + { + var options = compilation.Options; + return _language == compilation.Language + && _assemblyName == compilation.AssemblyName + && ReferenceEquals(_assemblyIdentityComparer, options.AssemblyIdentityComparer) + && Equals(_metadataReferenceResolver, options.MetadataReferenceResolver) + && _metadataImportOptions == options.MetadataImportOptions; + } } /// /// Extracts all needed data as primitives in the transform step. /// This enables proper incremental caching - the model contains only strings. + /// + /// The result is memoized by across compilations and drivers, so it + /// must depend only on the references, their binding options, the language and the assembly name. + /// The one source dependency (a source type shadowing a candidate) is reported through + /// and re-checked on reuse. Any new input read here + /// must be added to . + /// /// /// The compilation to inspect. /// diff --git a/tests/TUnit.SourceGenerator.IncrementalTests/InfrastructureGeneratorIncrementalTests.cs b/tests/TUnit.SourceGenerator.IncrementalTests/InfrastructureGeneratorIncrementalTests.cs index 0bf0f004015..901c4e6ec08 100644 --- a/tests/TUnit.SourceGenerator.IncrementalTests/InfrastructureGeneratorIncrementalTests.cs +++ b/tests/TUnit.SourceGenerator.IncrementalTests/InfrastructureGeneratorIncrementalTests.cs @@ -196,6 +196,31 @@ public void EditSource_ShadowingSelectedType_ShouldRegenerate() Xunit.Assert.Contains("typeof(global::OtherTestLibrary.FirstHooks)", GenerateInfrastructure(driver3)); } + [Fact] + public void FreshDriver_SameReferences_DifferentBindingOptions_ReExtracts() + { + var baseCompilation = Fixture.CreateLibrary(CSharpSyntaxTree.ParseText(DefaultSource, CSharpParseOptions.Default)); + var baseModel = GetExtractedModel(TestHelper.GenerateTracked(baseCompilation)); + + // Same reference array, so the memo is hit, but options that change how references bind to + // assembly symbols must not reuse a model extracted under different options. + var withComparer = baseCompilation.WithOptions( + baseCompilation.Options.WithAssemblyIdentityComparer(DesktopAssemblyIdentityComparer.Default)); + Xunit.Assert.Equal(baseCompilation.ExternalReferences, withComparer.ExternalReferences); + var comparerModel = GetExtractedModel(TestHelper.GenerateTracked(withComparer)); + Xunit.Assert.NotSame(baseModel, comparerModel); + Xunit.Assert.Equal(baseModel, comparerModel); + + var withImportOptions = baseCompilation.WithOptions( + baseCompilation.Options.WithMetadataImportOptions(MetadataImportOptions.All)); + var importModel = GetExtractedModel(TestHelper.GenerateTracked(withImportOptions)); + Xunit.Assert.NotSame(baseModel, importModel); + + // The original options still reuse the memo once it holds their extraction again. + var again = GetExtractedModel(TestHelper.GenerateTracked(baseCompilation)); + Xunit.Assert.Same(again, GetExtractedModel(TestHelper.GenerateTracked(baseCompilation))); + } + private static CSharpCompilation CreateWithTwoTypeLibrary() { var consumer = Fixture.CreateLibrary(CSharpSyntaxTree.ParseText(DefaultSource, CSharpParseOptions.Default)); From e93802c67d36fe1d124f6dd8b9a267c633c419ae Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Tue, 29 Sep 2026 11:15:55 +0100 Subject: [PATCH 4/5] fix(source-gen): never memoize script compilations in InfrastructureGenerator The scripting host binds submissions with the internal CompilationOptions.ReferencesSupersedeLowerVersions option, which the memo cannot compare. Skip the memo for any script compilation, and document why the key is the ImmutableArray backing array and why the holder race is benign. --- .../CodeGenerators/InfrastructureGenerator.cs | 20 +++++++++++++++++-- ...InfrastructureGeneratorIncrementalTests.cs | 20 +++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/src/TUnit.Core.SourceGenerator/CodeGenerators/InfrastructureGenerator.cs b/src/TUnit.Core.SourceGenerator/CodeGenerators/InfrastructureGenerator.cs index 815ca4facea..191e3b68e47 100644 --- a/src/TUnit.Core.SourceGenerator/CodeGenerators/InfrastructureGenerator.cs +++ b/src/TUnit.Core.SourceGenerator/CodeGenerators/InfrastructureGenerator.cs @@ -99,8 +99,11 @@ public void Initialize(IncrementalGeneratorInitializationContext context) /// private static AssemblyInfoModel GetAssemblyInfo(Compilation compilation) { - // Directive references (#r in scripts) are not part of the key; skip memoization for them. - if (!compilation.DirectiveReferences.IsEmpty) + // Script submissions are never memoized. Their #r directive references are not part of the + // key, and the scripting host binds them with CompilationOptions.ReferencesSupersedeLowerVersions, + // which is internal and so cannot be compared in Matches. Scripting is the only public API + // that sets it, so excluding scripts keeps the memo equal to a fresh extraction. + if (compilation.ScriptCompilationInfo is not null || !compilation.DirectiveReferences.IsEmpty) { return ExtractAssemblyInfo(compilation, out _); } @@ -112,6 +115,9 @@ private static AssemblyInfoModel GetAssemblyInfo(Compilation compilation) } var holder = AssemblyInfoCache.GetValue(key, static _ => new AssemblyInfoCacheEntry()); + + // Racy by design: two threads can both miss, both extract and both store. Every stored model + // equals a fresh extraction, so the last write wins and nothing is lost but duplicate work. var cached = holder.Value; if (cached is not null && cached.Matches(compilation) @@ -130,6 +136,16 @@ private static AssemblyInfoModel GetAssemblyInfo(Compilation compilation) } /// + /// Why the key is the backing array rather than something public: + /// + /// is an , a struct, + /// so it cannot be a key. Boxing it would create a new + /// object each call and never hit. Its backing array is the stable identity Roslyn reuses across + /// source-only edits. + /// A value-type projection in the pipeline (or MetadataReferencesProvider) cannot replace + /// this: extraction needs the to bind referenced assembly symbols, and a node + /// that carries the Compilation into the transform is exactly what pinned it before. + /// /// Returns the array backing , or when the /// layout check failed (memoization is then disabled, which costs speed but never correctness). /// diff --git a/tests/TUnit.SourceGenerator.IncrementalTests/InfrastructureGeneratorIncrementalTests.cs b/tests/TUnit.SourceGenerator.IncrementalTests/InfrastructureGeneratorIncrementalTests.cs index 901c4e6ec08..d3b7b175ecc 100644 --- a/tests/TUnit.SourceGenerator.IncrementalTests/InfrastructureGeneratorIncrementalTests.cs +++ b/tests/TUnit.SourceGenerator.IncrementalTests/InfrastructureGeneratorIncrementalTests.cs @@ -221,6 +221,26 @@ public void FreshDriver_SameReferences_DifferentBindingOptions_ReExtracts() Xunit.Assert.Same(again, GetExtractedModel(TestHelper.GenerateTracked(baseCompilation))); } + [Fact] + public void ScriptCompilation_SameReferences_IsNeverMemoized() + { + var baseCompilation = Fixture.CreateLibrary(CSharpSyntaxTree.ParseText(DefaultSource, CSharpParseOptions.Default)); + var baseModel = GetExtractedModel(TestHelper.GenerateTracked(baseCompilation)); + + // The scripting host binds submissions with the internal ReferencesSupersedeLowerVersions option, + // which Matches cannot compare, so script compilations must always extract fresh. + var script = CSharpCompilation.CreateScriptCompilation( + baseCompilation.AssemblyName!, + CSharpSyntaxTree.ParseText("var x = 1;", CSharpParseOptions.Default.WithKind(SourceCodeKind.Script)), + baseCompilation.ExternalReferences); + Xunit.Assert.Equal(baseCompilation.ExternalReferences, script.ExternalReferences); + + var first = GetExtractedModel(TestHelper.GenerateTracked(script)); + var second = GetExtractedModel(TestHelper.GenerateTracked(script)); + Xunit.Assert.NotSame(baseModel, first); + Xunit.Assert.NotSame(first, second); + } + private static CSharpCompilation CreateWithTwoTypeLibrary() { var consumer = Fixture.CreateLibrary(CSharpSyntaxTree.ParseText(DefaultSource, CSharpParseOptions.Default)); From 0fe438f6d3496cfc2ffe4c720931af06123dc91d Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Tue, 29 Sep 2026 11:34:13 +0100 Subject: [PATCH 5/5] refactor(source-gen): memoize InfrastructureGenerator reference walk per driver Replace the static ConditionalWeakTable keyed via Unsafe.As on the ImmutableArray backing array with a single-slot memo created in Initialize. References are compared with ImmutableArray ==, options by instance, plus assembly name; scripts bypass the memo and source shadowing is still re-checked on each hit. Every miss replaces or clears the slot, so it only ever holds the latest compilation's references and options, never a Compilation or syntax tree. The walk costs ~6.5 ms (p90 ~9 ms) per syntax edit on TUnit.TestProject (211 references), so the memo is kept. Memo tests now exercise the memo directly, since the driver keeps the previous output instance whenever a rerun produces an equal model. --- .../CodeGenerators/InfrastructureGenerator.cs | 182 ++++++------------ .../Properties/AssemblyInfo.cs | 3 + ...InfrastructureGeneratorIncrementalTests.cs | 125 ++++++++---- 3 files changed, 145 insertions(+), 165 deletions(-) create mode 100644 src/TUnit.Core.SourceGenerator/Properties/AssemblyInfo.cs diff --git a/src/TUnit.Core.SourceGenerator/CodeGenerators/InfrastructureGenerator.cs b/src/TUnit.Core.SourceGenerator/CodeGenerators/InfrastructureGenerator.cs index 191e3b68e47..ff310d78f3f 100644 --- a/src/TUnit.Core.SourceGenerator/CodeGenerators/InfrastructureGenerator.cs +++ b/src/TUnit.Core.SourceGenerator/CodeGenerators/InfrastructureGenerator.cs @@ -1,5 +1,4 @@ using System.Collections.Immutable; -using System.Runtime.CompilerServices; using Microsoft.CodeAnalysis; using TUnit.Core.SourceGenerator.Models; using TUnit.Core.SourceGenerator.Models.Extracted; @@ -62,9 +61,10 @@ public void Initialize(IncrementalGeneratorInitializationContext context) // keeps the generated source cached across keystrokes. No custom Compilation comparer here: // when a comparer reports "equal", the input node keeps the OLD compilation in its table, // pinning it (trees, bound state) until references change. The reference walk is memoized - // on the reference list instead, so syntax-only edits still skip it. + // per driver instead, so syntax-only edits still skip it. + var memo = new AssemblyInfoMemo(); var assemblyInfoProvider = context.CompilationProvider - .Select((compilation, _) => GetAssemblyInfo(compilation)) + .Select((compilation, _) => memo.GetAssemblyInfo(compilation)) .WithTrackingName(ExtractAssemblyInfoStep) .Combine(enabledProvider); @@ -81,103 +81,74 @@ public void Initialize(IncrementalGeneratorInitializationContext context) } /// - /// Returns the memoized model while the compilation keeps the same reference list. Syntax-only - /// edits reuse the backing array of ; adding, removing, - /// rebuilding or (for an IDE project reference) editing a reference produces a new array, so - /// extraction reruns. The table is keyed weakly on that array and its values hold only strings and - /// the reference-binding option objects, so no Compilation or MetadataReference is kept alive by - /// the cache. + /// Single-slot memo of the reference walk, created per and so owned by one + /// generator driver. The walk costs several milliseconds per run on a real test project (about + /// 6.5 ms for TUnit.TestProject's 211 references), and in the IDE it would otherwise run on every + /// keystroke. /// - /// The selection is reference-derived except for one source dependency: - /// prefers a source type over a referenced type with - /// the same metadata name, so source can shadow a candidate. The memo is therefore only stored - /// when no candidate was shadowed (the selection then equals a purely reference-based one), and on - /// reuse each selected type is checked against the current source; if any is now shadowed, - /// extraction reruns. The table is static and can be hit by independent drivers that share a - /// reference array, so this keeps every returned model identical to a fresh extraction. + /// The key is every compilation input the walk reads except source: + /// (compared with + /// ==, which compares the backing array by reference; syntax-only edits reuse it, while any + /// reference change produces a new one), the instance and the + /// assembly name. Scripts are never memoized. The one source dependency, a source type shadowing a + /// selected type, is re-checked on every hit. + /// + /// + /// The slot only ever describes the latest compilation this driver saw: every miss replaces or + /// clears it. It holds that compilation's reference list and options (which the live compilation + /// holds anyway), never a or syntax tree, and it dies with the driver. /// /// - private static AssemblyInfoModel GetAssemblyInfo(Compilation compilation) + internal sealed class AssemblyInfoMemo { - // Script submissions are never memoized. Their #r directive references are not part of the - // key, and the scripting host binds them with CompilationOptions.ReferencesSupersedeLowerVersions, - // which is internal and so cannot be compared in Matches. Scripting is the only public API - // that sets it, so excluding scripts keeps the memo equal to a fresh extraction. - if (compilation.ScriptCompilationInfo is not null || !compilation.DirectiveReferences.IsEmpty) - { - return ExtractAssemblyInfo(compilation, out _); - } + // Racy by design: concurrent runs may both miss, both extract and both store. Entries are + // immutable and each equals a fresh extraction for its key, so the only cost is duplicate work. + private volatile Entry? _last; - var key = GetReferencesKey(compilation.ExternalReferences); - if (key is null) + public AssemblyInfoModel GetAssemblyInfo(Compilation compilation) { - return ExtractAssemblyInfo(compilation, out _); - } + // Script submissions read inputs outside the key (#r directive references, the previous + // submission), so they always extract fresh. + if (compilation.ScriptCompilationInfo is not null || !compilation.DirectiveReferences.IsEmpty) + { + _last = null; + return ExtractAssemblyInfo(compilation, out _); + } - var holder = AssemblyInfoCache.GetValue(key, static _ => new AssemblyInfoCacheEntry()); + var last = _last; + if (last is not null + && last.References == compilation.ExternalReferences + && ReferenceEquals(last.Options, compilation.Options) + && last.AssemblyName == compilation.AssemblyName + && !IsAnyShadowedBySource(compilation, last.SelectedMetadataNames)) + { + return last.Model; + } - // Racy by design: two threads can both miss, both extract and both store. Every stored model - // equals a fresh extraction, so the last write wins and nothing is lost but duplicate work. - var cached = holder.Value; - if (cached is not null - && cached.Matches(compilation) - && !IsAnyShadowedBySource(compilation, cached.SelectedMetadataNames)) - { - return cached.Model; + var model = ExtractAssemblyInfo(compilation, out var selectedMetadataNames); + _last = selectedMetadataNames is null + ? null + : new Entry(compilation.ExternalReferences, compilation.Options, compilation.AssemblyName, model, selectedMetadataNames); + return model; } - var model = ExtractAssemblyInfo(compilation, out var selectedMetadataNames); - if (selectedMetadataNames is not null) + private sealed class Entry( + ImmutableArray references, + CompilationOptions options, + string? assemblyName, + AssemblyInfoModel model, + string[] selectedMetadataNames) { - holder.Value = new CachedAssemblyInfo(compilation, model, selectedMetadataNames); - } + public ImmutableArray References { get; } = references; - return model; - } - - /// - /// Why the key is the backing array rather than something public: - /// - /// is an , a struct, - /// so it cannot be a key. Boxing it would create a new - /// object each call and never hit. Its backing array is the stable identity Roslyn reuses across - /// source-only edits. - /// A value-type projection in the pipeline (or MetadataReferencesProvider) cannot replace - /// this: extraction needs the to bind referenced assembly symbols, and a node - /// that carries the Compilation into the transform is exactly what pinned it before. - /// - /// Returns the array backing , or when the - /// layout check failed (memoization is then disabled, which costs speed but never correctness). - /// - private static MetadataReference[]? GetReferencesKey(ImmutableArray references) - { - // Deliberate layout assumption: ImmutableArray is a struct wrapping a single T[] field - // (ImmutableCollectionsMarshal.AsArray does the same, but is not available to netstandard2.0 - // analyzers). IsImmutableArrayLayoutAsExpected verifies it once at startup, and - // EditSource_ShouldNotRegenerate fails if the memo stops being used. - if (!IsImmutableArrayLayoutAsExpected) - { - return null; - } + public CompilationOptions Options { get; } = options; - return Unsafe.As, MetadataReference[]?>(ref references); - } + public string? AssemblyName { get; } = assemblyName; - private static readonly bool IsImmutableArrayLayoutAsExpected = CheckImmutableArrayLayout(); + public AssemblyInfoModel Model { get; } = model; - private static bool CheckImmutableArrayLayout() - { - if (Unsafe.SizeOf>() != Unsafe.SizeOf()) - { - return false; + public string[] SelectedMetadataNames { get; } = selectedMetadataNames; } - - var probe = ImmutableArray.Create(new object(), new object()); - var array = Unsafe.As, object[]?>(ref probe); - return array is not null - && array.Length == probe.Length - && ReferenceEquals(array[0], probe[0]) - && ReferenceEquals(array[1], probe[1]); } private static bool IsAnyShadowedBySource(Compilation compilation, string[] selectedMetadataNames) @@ -194,50 +165,13 @@ private static bool IsAnyShadowedBySource(Compilation compilation, string[] sele return false; } - private static readonly ConditionalWeakTable AssemblyInfoCache = new(); - - private sealed class AssemblyInfoCacheEntry - { - public volatile CachedAssemblyInfo? Value; - } - - private sealed class CachedAssemblyInfo(Compilation compilation, AssemblyInfoModel model, string[] selectedMetadataNames) - { - private readonly string _language = compilation.Language; - private readonly string? _assemblyName = compilation.AssemblyName; - - // Options that change how references bind to assembly symbols (unification, implicitly - // resolved missing assemblies, imported metadata), and so can change the selection even - // when the reference array is the same. Only the comparer and resolver objects are held, - // never the Compilation. - private readonly AssemblyIdentityComparer _assemblyIdentityComparer = compilation.Options.AssemblyIdentityComparer; - private readonly MetadataReferenceResolver? _metadataReferenceResolver = compilation.Options.MetadataReferenceResolver; - private readonly MetadataImportOptions _metadataImportOptions = compilation.Options.MetadataImportOptions; - - public AssemblyInfoModel Model { get; } = model; - - public string[] SelectedMetadataNames { get; } = selectedMetadataNames; - - public bool Matches(Compilation compilation) - { - var options = compilation.Options; - return _language == compilation.Language - && _assemblyName == compilation.AssemblyName - && ReferenceEquals(_assemblyIdentityComparer, options.AssemblyIdentityComparer) - && Equals(_metadataReferenceResolver, options.MetadataReferenceResolver) - && _metadataImportOptions == options.MetadataImportOptions; - } - } - /// /// Extracts all needed data as primitives in the transform step. /// This enables proper incremental caching - the model contains only strings. /// - /// The result is memoized by across compilations and drivers, so it - /// must depend only on the references, their binding options, the language and the assembly name. - /// The one source dependency (a source type shadowing a candidate) is reported through - /// and re-checked on reuse. Any new input read here - /// must be added to . + /// The result is memoized by across syntax-only edits, so any + /// dependency on source must be reported through (as the + /// shadowing check does) so that reuse can re-check it. /// /// /// The compilation to inspect. diff --git a/src/TUnit.Core.SourceGenerator/Properties/AssemblyInfo.cs b/src/TUnit.Core.SourceGenerator/Properties/AssemblyInfo.cs new file mode 100644 index 00000000000..ddb03cd92b2 --- /dev/null +++ b/src/TUnit.Core.SourceGenerator/Properties/AssemblyInfo.cs @@ -0,0 +1,3 @@ +using System.Runtime.CompilerServices; + +[assembly: InternalsVisibleTo("TUnit.SourceGenerator.IncrementalTests")] diff --git a/tests/TUnit.SourceGenerator.IncrementalTests/InfrastructureGeneratorIncrementalTests.cs b/tests/TUnit.SourceGenerator.IncrementalTests/InfrastructureGeneratorIncrementalTests.cs index d3b7b175ecc..cc67303266c 100644 --- a/tests/TUnit.SourceGenerator.IncrementalTests/InfrastructureGeneratorIncrementalTests.cs +++ b/tests/TUnit.SourceGenerator.IncrementalTests/InfrastructureGeneratorIncrementalTests.cs @@ -27,14 +27,12 @@ public void EditSource_ShouldNotRegenerate() var driver1 = TestHelper.GenerateTracked(compilation1); AssertRunReason(driver1, IncrementalStepRunReason.New); - var model1 = GetExtractedModel(driver1); - // Syntax-only edits rerun the cheap Select, which returns the memoized model (same instance, - // so the reference walk was skipped) and keeps the generated source cached. + // Syntax-only edits rerun the cheap Select, whose model is unchanged, so the generated source + // stays cached. Memo_SyntaxOnlyEdits_ReuseModel checks that the reference walk is skipped. var compilation2 = compilation1.AddSyntaxTrees(CSharpSyntaxTree.ParseText("struct MyValue {}")); var driver2 = driver1.RunGenerators(compilation2); AssertRunReason(driver2, IncrementalStepRunReason.Unchanged); - Xunit.Assert.Same(model1, GetExtractedModel(driver2)); TestHelper.AssertSourceOutputsCached(driver2.GetRunResult().Results[0]); var compilation3 = TestHelper.ReplaceMethodDeclaration(compilation1, "Test1", @@ -47,7 +45,6 @@ public void Test1() """); var driver3 = driver2.RunGenerators(compilation3); AssertRunReason(driver3, IncrementalStepRunReason.Unchanged); - Xunit.Assert.Same(model1, GetExtractedModel(driver3)); TestHelper.AssertSourceOutputsCached(driver3.GetRunResult().Results[0]); } @@ -149,34 +146,31 @@ public void EditProjectReference_ShouldRegenerate() private const string ShadowingSource = "namespace OtherTestLibrary { public class FirstHooks { } }"; [Fact] - public void FreshDriver_SameReferences_SourceShadowsSelectedType_SelectsNextType() + public void FreshDriver_SameReferences_DoesNotShareMemo() { - var baseCompilation = CreateWithTwoTypeLibrary(); - - // Populate the shared memo from an unrelated driver. - var unshadowed = GenerateInfrastructure(CSharpGeneratorDriver.Create(new InfrastructureGenerator()).RunGenerators(baseCompilation)); - Xunit.Assert.Contains("typeof(global::OtherTestLibrary.FirstHooks)", unshadowed); + var compilation = Fixture.CreateLibrary(CSharpSyntaxTree.ParseText(DefaultSource, CSharpParseOptions.Default)); - // Same reference array, but a source type now takes FirstHooks' name, so typeof() would bind - // to the source type and never initialize OtherTestLibrary. - var shadowedCompilation = baseCompilation.AddSyntaxTrees(CSharpSyntaxTree.ParseText(ShadowingSource)); - var shadowed = GenerateInfrastructure(CSharpGeneratorDriver.Create(new InfrastructureGenerator()).RunGenerators(shadowedCompilation)); - Xunit.Assert.Contains("typeof(global::OtherTestLibrary.SecondHooks)", shadowed); - Xunit.Assert.DoesNotContain("typeof(global::OtherTestLibrary.FirstHooks)", shadowed); + // The memo belongs to one driver, so an unrelated driver over the same reference list + // extracts fresh instead of reusing another driver's result. + var first = GetExtractedModel(TestHelper.GenerateTracked(compilation)); + var second = GetExtractedModel(TestHelper.GenerateTracked(compilation)); + Xunit.Assert.NotSame(first, second); + Xunit.Assert.Equal(first, second); } [Fact] - public void FreshDriver_SameReferences_SourceSensitiveSelectionIsNotReused() + public void EditSource_SourceSensitiveSelectionIsNotReused() { var baseCompilation = CreateWithTwoTypeLibrary(); var shadowedCompilation = baseCompilation.AddSyntaxTrees(CSharpSyntaxTree.ParseText(ShadowingSource)); // The shadowed run must not memoize its (source-dependent) choice for later compilations. - var shadowed = GenerateInfrastructure(CSharpGeneratorDriver.Create(new InfrastructureGenerator()).RunGenerators(shadowedCompilation)); - Xunit.Assert.Contains("typeof(global::OtherTestLibrary.SecondHooks)", shadowed); + var driver1 = TestHelper.GenerateTracked(shadowedCompilation); + Xunit.Assert.Contains("typeof(global::OtherTestLibrary.SecondHooks)", GenerateInfrastructure(driver1)); - var unshadowed = GenerateInfrastructure(CSharpGeneratorDriver.Create(new InfrastructureGenerator()).RunGenerators(baseCompilation)); - Xunit.Assert.Contains("typeof(global::OtherTestLibrary.FirstHooks)", unshadowed); + var driver2 = driver1.RunGenerators(baseCompilation); + AssertRunReason(driver2, IncrementalStepRunReason.Modified); + Xunit.Assert.Contains("typeof(global::OtherTestLibrary.FirstHooks)", GenerateInfrastructure(driver2)); } [Fact] @@ -196,47 +190,96 @@ public void EditSource_ShadowingSelectedType_ShouldRegenerate() Xunit.Assert.Contains("typeof(global::OtherTestLibrary.FirstHooks)", GenerateInfrastructure(driver3)); } + // The driver keeps the previous output instance whenever a rerun produces an equal model, so + // whether the walk was skipped is only observable on the memo itself: it returns the same + // instance on a hit and a new one after re-extracting. + [Fact] - public void FreshDriver_SameReferences_DifferentBindingOptions_ReExtracts() + public void Memo_SyntaxOnlyEdits_ReuseModel() { + var memo = new InfrastructureGenerator.AssemblyInfoMemo(); + var compilation1 = Fixture.CreateLibrary(CSharpSyntaxTree.ParseText(DefaultSource, CSharpParseOptions.Default)); + var model1 = memo.GetAssemblyInfo(compilation1); + + var compilation2 = compilation1.AddSyntaxTrees(CSharpSyntaxTree.ParseText("struct MyValue {}")); + Xunit.Assert.Same(model1, memo.GetAssemblyInfo(compilation2)); + + var compilation3 = TestHelper.ReplaceMethodDeclaration(compilation1, "Test1", + """ + [Test] + public void Test1() + { + var x = 1; + } + """); + Xunit.Assert.Same(model1, memo.GetAssemblyInfo(compilation3)); + } + + [Fact] + public void Memo_ReferenceChange_ReExtracts() + { + var memo = new InfrastructureGenerator.AssemblyInfoMemo(); + var compilation1 = Fixture.CreateLibrary(CSharpSyntaxTree.ParseText(DefaultSource, CSharpParseOptions.Default)); + var model1 = memo.GetAssemblyInfo(compilation1); + + var compilation2 = compilation1.AddReferences(CreateHookLibrary(compilation1, "SharedHooks").ToMetadataReference()); + var model2 = memo.GetAssemblyInfo(compilation2); + Xunit.Assert.NotSame(model1, model2); + Xunit.Assert.NotEqual(model1, model2); + } + + [Fact] + public void Memo_OptionsOrAssemblyNameChange_ReExtracts() + { + var memo = new InfrastructureGenerator.AssemblyInfoMemo(); var baseCompilation = Fixture.CreateLibrary(CSharpSyntaxTree.ParseText(DefaultSource, CSharpParseOptions.Default)); - var baseModel = GetExtractedModel(TestHelper.GenerateTracked(baseCompilation)); + var baseModel = memo.GetAssemblyInfo(baseCompilation); - // Same reference array, so the memo is hit, but options that change how references bind to - // assembly symbols must not reuse a model extracted under different options. + // Same reference list, but options can change how references bind to assembly symbols, + // so a new options instance must not reuse the model extracted under the old one. var withComparer = baseCompilation.WithOptions( baseCompilation.Options.WithAssemblyIdentityComparer(DesktopAssemblyIdentityComparer.Default)); - Xunit.Assert.Equal(baseCompilation.ExternalReferences, withComparer.ExternalReferences); - var comparerModel = GetExtractedModel(TestHelper.GenerateTracked(withComparer)); + Xunit.Assert.True(baseCompilation.ExternalReferences == withComparer.ExternalReferences); + var comparerModel = memo.GetAssemblyInfo(withComparer); Xunit.Assert.NotSame(baseModel, comparerModel); Xunit.Assert.Equal(baseModel, comparerModel); var withImportOptions = baseCompilation.WithOptions( baseCompilation.Options.WithMetadataImportOptions(MetadataImportOptions.All)); - var importModel = GetExtractedModel(TestHelper.GenerateTracked(withImportOptions)); - Xunit.Assert.NotSame(baseModel, importModel); - - // The original options still reuse the memo once it holds their extraction again. - var again = GetExtractedModel(TestHelper.GenerateTracked(baseCompilation)); - Xunit.Assert.Same(again, GetExtractedModel(TestHelper.GenerateTracked(baseCompilation))); + var importModel = memo.GetAssemblyInfo(withImportOptions); + Xunit.Assert.NotSame(comparerModel, importModel); + + // Syntax-only edits under the new options reuse the memo again. + Xunit.Assert.Same(importModel, memo.GetAssemblyInfo( + withImportOptions.AddSyntaxTrees(CSharpSyntaxTree.ParseText("struct MyValue {}")))); + + var renamed = withImportOptions.WithAssemblyName("Renamed"); + Xunit.Assert.True(withImportOptions.ExternalReferences == renamed.ExternalReferences); + var renamedModel = memo.GetAssemblyInfo(renamed); + Xunit.Assert.NotSame(importModel, renamedModel); + Xunit.Assert.Equal("Renamed", renamedModel.AssemblyName); } [Fact] - public void ScriptCompilation_SameReferences_IsNeverMemoized() + public void Memo_ScriptCompilation_IsNeverMemoized() { + var memo = new InfrastructureGenerator.AssemblyInfoMemo(); var baseCompilation = Fixture.CreateLibrary(CSharpSyntaxTree.ParseText(DefaultSource, CSharpParseOptions.Default)); - var baseModel = GetExtractedModel(TestHelper.GenerateTracked(baseCompilation)); + var baseModel = memo.GetAssemblyInfo(baseCompilation); - // The scripting host binds submissions with the internal ReferencesSupersedeLowerVersions option, - // which Matches cannot compare, so script compilations must always extract fresh. + // Script submissions read inputs outside the memo key (#r directives, the previous + // submission and the internal ReferencesSupersedeLowerVersions option), so they always + // extract fresh. var script = CSharpCompilation.CreateScriptCompilation( baseCompilation.AssemblyName!, CSharpSyntaxTree.ParseText("var x = 1;", CSharpParseOptions.Default.WithKind(SourceCodeKind.Script)), baseCompilation.ExternalReferences); - Xunit.Assert.Equal(baseCompilation.ExternalReferences, script.ExternalReferences); + Xunit.Assert.True(baseCompilation.ExternalReferences == script.ExternalReferences); - var first = GetExtractedModel(TestHelper.GenerateTracked(script)); - var second = GetExtractedModel(TestHelper.GenerateTracked(script)); + var first = memo.GetAssemblyInfo(script); + var second = memo.GetAssemblyInfo(script.ReplaceSyntaxTree( + script.SyntaxTrees.Single(), + CSharpSyntaxTree.ParseText("var y = 2;", CSharpParseOptions.Default.WithKind(SourceCodeKind.Script)))); Xunit.Assert.NotSame(baseModel, first); Xunit.Assert.NotSame(first, second); }