SG: Implementation outputs #54798

chsienki · 2021-07-13T21:05:51Z

This implements the idea of 'non semantic' source generation. That is source that can be safely ignored from the IDE perspective (no intellisense impact etc) but required for the on disk build.

I have used the name 'Implementation' for two reasons:

Suggests this is an 'implementation detail' and hidden from the user
We can introduce a mirror 'Reference Source output' in the future to the be the opposite of this output.
- This will allow for scenarios where e.g. generating the body of a method is expensive

Note: this doesn't hook up the IDE side of things yet, but just makes the API available for consumption.

Closes: #53609
Api Review: #54723

- Make output kinds public - Take a parameter during construction - Don't run outputs passed in as disabled to the generator

chsienki · 2021-07-13T23:28:48Z

@dotnet/roslyn-compiler for review please :)

src/Compilers/CSharp/Portable/PublicAPI.Unshipped.txt

src/Compilers/VisualBasic/Portable/PublicAPI.Unshipped.txt

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

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

src/Compilers/CSharp/Portable/SourceGeneration/CSharpGeneratorDriver.cs

chsienki · 2021-07-15T18:36:34Z

Ping @dotnet/roslyn-compiler for a second review please :)

cston · 2021-07-16T16:19:15Z

src/Compilers/Core/Portable/SourceGeneration/IncrementalContexts.cs

-        public void RegisterSourceOutput<TSource>(IncrementalValuesProvider<TSource> source, Action<SourceProductionContext, TSource> action) => source.Node.RegisterOutput(new SourceOutputNode<TSource>(source.Node, action.WrapUserAction()));
+        public void RegisterSourceOutput<TSource>(IncrementalValuesProvider<TSource> source, Action<SourceProductionContext, TSource> action) => source.Node.RegisterOutput(new SourceOutputNode<TSource>(source.Node, action.WrapUserAction(), IncrementalGeneratorOutputKind.Source));
+
+        public void RegisterImplementationSourceOutput<TSource>(IncrementalValueProvider<TSource> source, Action<SourceProductionContext, TSource> action) => source.Node.RegisterOutput(new SourceOutputNode<TSource>(source.Node, action.WrapUserAction(), IncrementalGeneratorOutputKind.Implementation));


source.Node.RegisterOutput(new SourceOutputNode(source.Node, action.WrapUserAction(), IncrementalGeneratorOutputKind.Implementation))

Consider extracting a helper method (or two?) for the implementations of these 4 methods. #Resolved

cston · 2021-07-16T16:26:25Z

src/Compilers/CSharp/Portable/SourceGeneration/CSharpGeneratorDriver.cs

        /// <returns>A new <see cref="CSharpGeneratorDriver"/> instance.</returns>
-        public static CSharpGeneratorDriver Create(IEnumerable<ISourceGenerator> generators, IEnumerable<AdditionalText>? additionalTexts = null, CSharpParseOptions? parseOptions = null, AnalyzerConfigOptionsProvider? optionsProvider = null)
-            => new CSharpGeneratorDriver(parseOptions ?? CSharpParseOptions.Default, generators.ToImmutableArray(), optionsProvider ?? CompilerAnalyzerConfigOptionsProvider.Empty, additionalTexts.AsImmutableOrEmpty());
+        public static CSharpGeneratorDriver Create(IEnumerable<ISourceGenerator> generators, IEnumerable<AdditionalText>? additionalTexts = null, CSharpParseOptions? parseOptions = null, AnalyzerConfigOptionsProvider? optionsProvider = null, IncrementalGeneratorOutputKind disabledOutputs = IncrementalGeneratorOutputKind.None)


IncrementalGeneratorOutputKind disabledOutputs

Will we have additional generator options in the future? If we know now that we will, perhaps it would make sense to pass an options type here rather than a single value. #Closed

Yes, we plan to have more, but IncrementalGeneratorOutputKind is a [Flags] so we can combine them together like IncrementalGeneratorOutputKind.Implementation | IncrementalGeneratorKind.SomeOtherOutput to build up which nodes we don't want.

Adding more IncrementalGeneratorOutputKind values can be accomodated with the current overload but do we expect to add options other than IncrementatlGeneratorOutputKind?

Oh, I see what you're saying. Yeah actually that probably makes sense.

cston · 2021-07-16T16:29:39Z

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

+
+                if (disabledOutput.HasFlag((IncrementalGeneratorOutputKind)kind))
+                {
+                    Assert.DoesNotContain(result.Results[0].GeneratedSources, (s) => s.HintName == Enum.GetName(typeof(IncrementalGeneratorOutputKind), kind) + ".cs");


(s) => s.HintName == Enum.GetName(typeof(IncrementalGeneratorOutputKind), kind) + ".cs"

Consider extracting a local function for use here and below. #Closed

cston · 2021-07-17T04:27:29Z

src/Compilers/Core/Portable/SourceGeneration/GeneratorDriverOptions.cs

+    /// </summary>
+    public readonly struct GeneratorDriverOptions
+    {
+        public static GeneratorDriverOptions Default = new GeneratorDriverOptions(IncrementalGeneratorOutputKind.None);


static

readonly

cston · 2021-07-17T04:30:13Z

src/Compilers/Core/Portable/SourceGeneration/GeneratorDriverOptions.cs

+    /// </summary>
+    public readonly struct GeneratorDriverOptions
+    {
+        public static GeneratorDriverOptions Default = new GeneratorDriverOptions(IncrementalGeneratorOutputKind.None);


Default

Minor point: This field seems unnecessary since the type is a struct so it's inexpensive to create at each call site and because the value is equivalent to default(GeneratorDriverOptions).

cston · 2021-07-17T04:35:55Z

src/Compilers/CSharp/Portable/SourceGeneration/CSharpGeneratorDriver.cs

+        // 3.11 BACKCOMPAT OVERLOAD -- DO NOT TOUCH
+        [EditorBrowsable(EditorBrowsableState.Never)]
+        public static CSharpGeneratorDriver Create(IEnumerable<ISourceGenerator> generators, IEnumerable<AdditionalText>? additionalTexts, CSharpParseOptions? parseOptions, AnalyzerConfigOptionsProvider? optionsProvider)
+            => new CSharpGeneratorDriver(parseOptions ?? CSharpParseOptions.Default, generators.ToImmutableArray(), optionsProvider ?? CompilerAnalyzerConfigOptionsProvider.Empty, additionalTexts.AsImmutableOrEmpty(), driverOptions: GeneratorDriverOptions.Default);


new CSharpGeneratorDriver(parseOptions ?? CSharpParseOptions.Default, generators.ToImmutableArray(), optionsProvider ?? CompilerAnalyzerConfigOptionsProvider.Empty, additionalTexts.AsImmutableOrEmpty(), driverOptions: GeneratorDriverOptions.Default);

Consider calling the new Create() instead to avoid duplicating the handling of optional values: Create(parseOptions, optionsProvider, additionalTexts, driverOptions: null)

cston · 2021-07-19T21:00:34Z

src/Compilers/VisualBasic/Portable/SourceGeneration/VisualBasicGeneratorDriver.vb

-        Public Shared Function Create(generators As ImmutableArray(Of ISourceGenerator), Optional additionalTexts As ImmutableArray(Of AdditionalText) = Nothing, Optional parseOptions As VisualBasicParseOptions = Nothing, Optional analyzerConfigOptionsProvider As AnalyzerConfigOptionsProvider = Nothing) As VisualBasicGeneratorDriver
-            Return New VisualBasicGeneratorDriver(parseOptions, generators, If(analyzerConfigOptionsProvider, CompilerAnalyzerConfigOptionsProvider.Empty), additionalTexts.NullToEmpty())
+        Public Shared Function Create(generators As ImmutableArray(Of ISourceGenerator), Optional additionalTexts As ImmutableArray(Of AdditionalText) = Nothing, Optional parseOptions As VisualBasicParseOptions = Nothing, Optional analyzerConfigOptionsProvider As AnalyzerConfigOptionsProvider = Nothing, Optional driverOptions As GeneratorDriverOptions? = Nothing) As VisualBasicGeneratorDriver
+            Return New VisualBasicGeneratorDriver(parseOptions, generators, If(analyzerConfigOptionsProvider, CompilerAnalyzerConfigOptionsProvider.Empty), additionalTexts.NullToEmpty(), If(driverOptions, DirectCast(Nothing, GeneratorDriverOptions)))


If(driverOptions, DirectCast(Nothing, GeneratorDriverOptions))

Can this be simplified to If(driverOptions, Nothing)?

chsienki added 3 commits July 13, 2021 11:35

Expose output kinds

83f559a

- Make output kinds public - Take a parameter during construction - Don't run outputs passed in as disabled to the generator

Add tests for disabling outputs

2cd0a79

Rename NonSemantic to Implementation

a3b0289

chsienki added Area-Compilers New Feature - Source Generators Source Generators labels Jul 13, 2021

chsienki added this to the 17.0 milestone Jul 13, 2021

chsienki requested a review from a team as a code owner July 13, 2021 21:05

chsienki added 3 commits July 13, 2021 15:12

Implement back compat

fff629c

Fix test after rename

bce926f

Fix public api unshipped

d202d22

jaredpar reviewed Jul 14, 2021

View reviewed changes

Doc comments

f0a7346

Youssef1313 reviewed Jul 14, 2021

View reviewed changes

src/Compilers/CSharp/Portable/SourceGeneration/CSharpGeneratorDriver.cs Outdated Show resolved Hide resolved

chsienki added 2 commits July 14, 2021 09:18

Assert

c94ea23

Assert

9a21b10

jaredpar approved these changes Jul 14, 2021

View reviewed changes

cston reviewed Jul 16, 2021

View reviewed changes

chsienki added 2 commits July 16, 2021 15:58

Extract functions

55a31ea

Introduce GeneratorDriverOptions type

f1905d7

cston reviewed Jul 17, 2021

View reviewed changes

cston approved these changes Jul 17, 2021

View reviewed changes

chsienki added 2 commits July 19, 2021 13:35

PR feedback

49063b5

Fix public api

f138e03

cston reviewed Jul 19, 2021

View reviewed changes

Simplify default handling

e5465f7

cston approved these changes Jul 19, 2021

View reviewed changes

Fix method overloading

2ec5f20

chsienki merged commit 08ab40e into dotnet:main Jul 20, 2021

ghost modified the milestones: 17.0, Next Jul 20, 2021

chsienki mentioned this pull request Jul 22, 2021

Non Semantic Source Generation #54723

Closed

allisonchou modified the milestones: Next, 17.0.P3 Jul 27, 2021

commit-lint bot mentioned this pull request Jul 28, 2021

[pull] main from dotnet:main Blaquez-home/roslyn#2

Open

jkoritzinsky mentioned this pull request Aug 2, 2021

Move to using the new Roslyn IIncrementalGenerator API for better in-VS performance dotnet/runtimelab#1374

Merged

This was referenced Aug 13, 2021

Source Generators: pass for intellisense output #52170

Closed

Incremental Source Generators should support multiple output types #53608

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SG: Implementation outputs #54798

SG: Implementation outputs #54798

chsienki commented Jul 13, 2021 •

edited

Loading

chsienki commented Jul 13, 2021

chsienki commented Jul 15, 2021

cston Jul 16, 2021 •

edited

Loading

cston Jul 16, 2021 •

edited

Loading

chsienki Jul 16, 2021

cston Jul 16, 2021

chsienki Jul 16, 2021

cston Jul 16, 2021 •

edited

Loading

cston Jul 17, 2021

cston Jul 17, 2021

cston Jul 17, 2021

cston Jul 19, 2021

SG: Implementation outputs #54798

SG: Implementation outputs #54798

Conversation

chsienki commented Jul 13, 2021 • edited Loading

chsienki commented Jul 13, 2021

chsienki commented Jul 15, 2021

cston Jul 16, 2021 • edited Loading

Choose a reason for hiding this comment

cston Jul 16, 2021 • edited Loading

Choose a reason for hiding this comment

chsienki Jul 16, 2021

Choose a reason for hiding this comment

cston Jul 16, 2021

Choose a reason for hiding this comment

chsienki Jul 16, 2021

Choose a reason for hiding this comment

cston Jul 16, 2021 • edited Loading

Choose a reason for hiding this comment

cston Jul 17, 2021

Choose a reason for hiding this comment

cston Jul 17, 2021

Choose a reason for hiding this comment

cston Jul 17, 2021

Choose a reason for hiding this comment

cston Jul 19, 2021

Choose a reason for hiding this comment

chsienki commented Jul 13, 2021 •

edited

Loading

cston Jul 16, 2021 •

edited

Loading

cston Jul 16, 2021 •

edited

Loading

cston Jul 16, 2021 •

edited

Loading