Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Non Semantic Source Generation #54723

Closed
chsienki opened this issue Jul 9, 2021 · 7 comments
Closed

Non Semantic Source Generation #54723

chsienki opened this issue Jul 9, 2021 · 7 comments
Assignees
Labels
api-approved API was approved in API review, it can be implemented Area-Compilers Concept-API This issue involves adding, removing, clarification, or modification of an API. Feature Request
Milestone

Comments

@chsienki
Copy link
Contributor

chsienki commented Jul 9, 2021

Background and Motivation

Today source generators can only provide a single type of Source. This is always run in the IDE and compiler. However there are customers (such as the Orleans team) who produce source that is semantically 'invisible'; that is the added code has a runtime effect, but adds no user visible types in completion or intellisense etc.

This means that in IDE scenarios, we are doing a lot of unnecessary work, when the author could inform us to just skip it.

Proposed API

namespace Microsoft.CodeAnalysis
{
+  [Flags]
-  internal enum IncrementalGeneratorOutputKind
+  public enum IncrementalGeneratorOutputKind
   {
+       None,
        Source,
        PostInit,
+       Implementation
   }

   public readonly struct IncrementalGeneratorInitializationContext
   {
+        public void RegisterImplementationSourceOutput<TSource>(IncrementalValueProvider<TSource> source, Action<SourceProductionContext, TSource> action);
+        public void RegisterImplementationSourceOutput<TSource>(IncrementalValuesProvider<TSource> source, Action<SourceProductionContext, TSource> action);

   }
}

namespace Microsoft.CodeAnalysis.CSharp
{
    public sealed class CSharpGeneratorDriver : GeneratorDriver
    {
-        public static CSharpGeneratorDriver Create(IEnumerable<ISourceGenerator> generators, IEnumerable<AdditionalText>? additionalTexts = null, CSharpParseOptions? parseOptions = null, AnalyzerConfigOptionsProvider? optionsProvider = null)
+        public static CSharpGeneratorDriver Create(IEnumerable<ISourceGenerator> generators, IEnumerable<AdditionalText>? additionalTexts = null, CSharpParseOptions? parseOptions = null, AnalyzerConfigOptionsProvider? optionsProvider = null, IncrementalGeneratorOutputKind disabledOutputs = IncrementalGeneratorOutputKind.None)

    }

}

Usage Examples

// user generator code

context.RegisterSourceOutput(visibleInputs, ...);

context.RegisterImplementationSourceOutput(nonVisibleInputs, ...);

// in batch compiler (regular + implementation)
var driver = CSharpGeneratorDriver.Create(..., IncrementalGeneratorOutputKind.None);

// in IDE  (no implementation, just regular)
var driver = CSharpGeneratorDriver.Create(..., IncrementalGeneratorOutputKind.Implementation);

Alternative Designs

New Enum

The above design re-uses the already existing IncrementalGeneratorOutputKind. However, there may be limited value in being able to disable PostInit and regular source. Should we create a new public enum that only lists certain types of ouputs that can be explicitly toggled?

Enable instead of Disable

Should we make the output types explicitly opt-in? This would require the host to manually opt-in to new output types as they are added which seems unfortunate.

'Dummy' source or decl only

The above proposal only works when the source is entirely 'invisible'. Do we want to think about a mechanism where the generator can provide a reference source of e.g. just declarations for the IDE, and full source for the compiler? We could probably achieve that by adding another RegisterReferenceSourceOutput but it seems orthogonal enough that we could add it later alongside this proposal.

Risks

  • User confusion over when to run Source / ImplementationSource
  • IDE still needs to run non semantic source in order to display under the analyzers tab. (Can we just re-run in those cases?)
@chsienki chsienki added Concept-API This issue involves adding, removing, clarification, or modification of an API. Feature Request labels Jul 9, 2021
@chsienki chsienki added this to the 17.0 milestone Jul 9, 2021
@chsienki chsienki self-assigned this Jul 9, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Jul 9, 2021
@Youssef1313
Copy link
Member

Small thing to consider about naming the enum. Should it be plural (ie, IncrementalGeneratorOutputKinds) since it has FlagsAttribute?

@jaredpar jaredpar removed the untriaged Issues and PRs which have not yet been triaged by a lead label Jul 12, 2021
@chsienki chsienki added api-ready-for-review API is ready for review, it is NOT ready for implementation blocking API needs to reviewed with priority to unblock work labels Jul 15, 2021
@jkoritzinsky
Copy link
Member

How does this interact with diagnostics produced by a source generator?

Would the diagnostics only show up at build time, not at design time?

Additionally, would this be usable by generators that generate code to provide implementations of partial methods without showing errors for missing partial method implementations, or is it just for cases where the generated code is called through clever implementation tricks like "implicit" default constructors or method overloading?

@Sergio0694
Copy link
Contributor

"Additionally, would this be usable by generators that generate code to provide implementations of partial methods without showing errors for missing partial method implementations"

I am also particularly curious about this. In my use case scenario (detailed in #51497), I need to generate some methods for an interface that is added to partial user-defined types. I could just generate those methods as empty to make IntelliSense happy when the generator is running in the IDE, and save a ton of work, and then actually populate them properly during the full compile. I'd be interested to know if such a scenario would be supported by this proposed feature as well 😄

@sharwell
Copy link
Member

Would the diagnostics only show up at build time, not at design time?

I'm guessing:

  • FSA would run the full generation prior to analyzing that compilation (still not during typing)
  • Analyzers node would run full generation prior to opening the generated document (on demand)
  • Manual execution of the Run Code Analysis command would run full generation with diagnostics

@chsienki
Copy link
Contributor Author

chsienki commented Jul 15, 2021

Additionally, would this be usable by generators that generate code to provide implementations of partial methods without showing errors for missing partial method implementations, or is it just for cases where the generated code is called through clever implementation tricks like "implicit" default constructors or method overloading?

I am also particularly curious about this. In my use case scenario (detailed in #51497), I need to generate some methods for an interface that is added to partial user-defined types. I could just generate those methods as empty to make IntelliSense happy when the generator is running in the IDE, and save a ton of work, and then actually populate them properly during the full compile. I'd be interested to know if such a scenario would be supported by this proposed feature as well 😄

Currently this only supports the 'clever tricks' scenario. However the 'ReferenceOnlySource' would allow to do a decl/implementation split.

In terms of diagnostics, yes these would only be produced as part of a 'full build', not during typing in the IDE, as presumably you'd need to do the actual generation in order to create the diagnostics. When thats not the case, you can pass the same node to RegisterSourceOutput and RegisterImplementationSourceOutput and report diagnostics only from the source (if that makes sense?)

@chsienki
Copy link
Contributor Author

Notes

New Enum

Consensus was enum as-is is fine.

Naming

Could we rename somehow? Implementation seems fine, especially when paired with Reference

Reference only

There is a desire to ship the ReferenceSource outputs as well. We should add a new proposal and implementation to do that.

@chsienki chsienki added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation blocking API needs to reviewed with priority to unblock work labels Jul 16, 2021
@chsienki
Copy link
Contributor Author

Closed via #54798

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-approved API was approved in API review, it can be implemented Area-Compilers Concept-API This issue involves adding, removing, clarification, or modification of an API. Feature Request
Projects
None yet
Development

No branches or pull requests

6 participants