Add helper for source generators to filter to nodes with attribute matching an attribute name #61293

CyrusNajmabadi · 2022-05-12T23:55:09Z

…tching an attribute name

CyrusNajmabadi · 2022-05-12T23:56:50Z

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

+        // Create an aggregated view of all global aliases across all files.  This should only update when an individual
+        // file changes its global aliases.
+        var allUpGlobalAliasesProvider = individualFileGlobalAliasesProvider
+            .Collect().Select((arrays, _) => GlobalAliases.Create(arrays.SelectMany(a => a.AliasAndSymbolNames).ToImmutableArray()));


note: it's unclear to me how Collect works. My hope is that it doesn't rerun later parts if it sees that all its inputs have retained value-equality.

Collect is implemented using the BatchNode<T> which does have caching semantics. At the same time we should be able to verify this with testing correct?

yeah, i believe this is workign properly (and all the tests validate each stage of hte pipeline. wouldn't mind additional confirmation though. I was askign this prior to getting all the tests and learning a lot that way.

CyrusNajmabadi · 2022-05-12T23:57:37Z

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

+
+        // Combine the two providers so that we reanalyze every file if the global aliases change, or we reanalyze a
+        // particular file when it's compilation unit changes.
+        var compilationUnitAndGlobalAliasesProvider = compilationUnitProvider.Combine(allUpGlobalAliasesProvider);


similarly, if a change happens and the aggregrated global aliases list is unchanged, i'm hoping this provider will not rerun its transform for all compilation units.

In this case would it change when e.g DesignTime vs normal compilation unit type changes (when no code has changed between the DesignTime and the normal compilation unit?

If no code changes, then it won't rerun.

…ributeLookup

jaredpar · 2022-06-08T23:52:09Z

Took some time this afternoon to see how this new API would work in existing attribute based generators. It's a really smooth transition to the new API:

CyrusNajmabadi · 2022-06-09T01:23:08Z

@stephentoub Do you have other generators that look for attributes to determine what to run on?

stephentoub · 2022-06-09T01:34:50Z

All of our generators. In addition to the two Jared mentions, also the LibraryImport generator, the LoggingMessage generator, and the EventSource generator.

CyrusNajmabadi · 2022-06-09T01:51:48Z

Thanks!

AraHaan · 2022-06-10T03:30:59Z

I also have an attribute based generator too, as for migration to the new apis it would need to be tested with it which is located here.

chsienki · 2022-06-13T20:53:30Z

...e/SourceGeneration/IncrementalGeneratorInitializationContext_ForAttributeWithMetadataName.cs

+    internal IncrementalValuesProvider<T> ForAttributeWithMetadataName<T>(
+        string fullyQualifiedMetadataName,
+        Func<SyntaxNode, CancellationToken, bool> predicate,
+        Func<GeneratorAttributeSyntaxContext, CancellationToken, T> transform)


We should move this into SyntaxValueProvider. You can pass through any of the inputs you need at the .ctor.

chsienki · 2022-06-13T20:55:59Z

...e/SourceGeneration/IncrementalGeneratorInitializationContext_ForAttributeWithMetadataName.cs

+        // Group all the nodes by syntax tree, so we can process a whole syntax tree at a time.  This will let us make
+        // the required semantic model for it once, instead of potentially many times (in the rare, but possible case of
+        // a single file with a ton of matching nodes in it).
+        var groupedNodes = collectedNodes.SelectMany(


It's possible to re-write this code so that it isn't based on the public API and instead operates as an ISyntaxSelectionStrategy<T> that gets executed when the node is asked to update. That will allow it to share semantic model and syntax tree walks with the other syntax inputs. That should improve perf and reduce the memory usage, but it's something we can do afterwards without changing the public API

chsienki · 2022-06-13T21:07:01Z

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


        public IncrementalValueProvider<Compilation> CompilationProvider => new IncrementalValueProvider<Compilation>(SharedInputNodes.Compilation.WithRegisterOutput(RegisterOutput).WithTrackingName(WellKnownGeneratorInputs.Compilation));

+        internal IncrementalValueProvider<CompilationOptions> CompilationOptionsProvider => new(SharedInputNodes.CompilationOptions.WithRegisterOutput(RegisterOutput).WithTrackingName(WellKnownGeneratorInputs.CompilationOptions));


This isn't used anywhere other than inside ForAttributeWithSimpleName, so we can just use it directly in that method instead.

chsienki · 2022-06-13T21:11:53Z

...ble/SourceGeneration/IncrementalGeneratorInitializationContext_ForAttributeWithSimpleName.cs

+        var syntaxHelper = this.SyntaxHelper;
+
+        // Create a provider that provides (and updates) the global aliases for any particular file when it is edited.
+        var individualFileGlobalAliasesProvider = this.SyntaxProvider.CreateSyntaxProvider(


Basically what @jaredpar said, its possible to make this more efficient if we re-write it as an ISyntaxSelectionStrategy<T> but it won't affect the public API and we should not block on it. Lets do it in a follow up PR at some point in the future.

chsienki · 2022-06-13T21:12:24Z

...ble/SourceGeneration/IncrementalGeneratorInitializationContext_ForAttributeWithSimpleName.cs

+        // Create a provider that provides (and updates) the global aliases for any particular file when it is edited.
+        var individualFileGlobalAliasesProvider = this.SyntaxProvider.CreateSyntaxProvider(
+            static (n, _) => n is ICompilationUnitSyntax,
+            static (context, _) => getGlobalAliasesInCompilationUnit(context.SyntaxHelper, context.Node)).WithTrackingName("individualFileGlobalAliases_ForAttribute");


We need the tracking names for our internal testing, but they 'leak' into public as they'll now show up in users tracking results if they use this API. I think thats fine, but I think we should get consensus that we're a) ok with that and b) agree that this is not a public API contract and the nodes can be removed / renamed without it being considered a breaking change.

but I think we should get consensus that we're a) ok with that and b) agree that this is not a public API contract and the nodes can be removed / renamed without it being considered a breaking change.

Totally agreed. We coudl bring that to API meeting and then doc this explicitly.

CyrusNajmabadi · 2022-06-14T17:35:17Z

We should move this into SyntaxValueProvider. You can pass through any of the inputs you need at the .ctor.

Done.

CyrusNajmabadi · 2022-06-14T17:36:09Z

This isn't used anywhere other than inside ForAttributeWithSimpleName, so we can just use it directly in that method instead.

I tried ot move this. HOwever, it depends on RegisterOutput, which i couldn't reference from another type. So i wans't sure how best to move this. What do you recommend?

CyrusNajmabadi · 2022-06-14T20:16:55Z

@chsienki anything blocking on this? or are you ok for this goign in and doing any improvements as followup work to come?

chsienki · 2022-06-14T20:55:03Z

This isn't used anywhere other than inside ForAttributeWithSimpleName, so we can just use it directly in that method instead.

I tried ot move this. HOwever, it depends on RegisterOutput, which i couldn't reference from another type. So i wans't sure how best to move this. What do you recommend?

Ok, no worries, this is internal so fine for now.

chsienki

LGTM. Thanks!

CyrusNajmabadi added 2 commits May 12, 2022 16:40

Add helper for source generators to filter to nodes with attribute ma…

af00fad

…tching an attribute name

Simplify

400f3b6

CyrusNajmabadi requested a review from chsienki May 12, 2022 23:55

dotnet-issue-labeler bot added the Area-Compilers label May 12, 2022

Doc

c396037

CyrusNajmabadi commented May 12, 2022

View reviewed changes

CyrusNajmabadi added 23 commits May 12, 2022 17:00

Add docs

efa3873

Add tracking names

54fc5d0

Simplify

6fe27f3

Operate on attribute lists

e5149f5

Validate argument

e007656

Add tests

51ec0a0

Add tests

ab8cb20

Add tests

31741bd

Add tests

f03429c

Add tests

886b754

Temp

27e76f5

Add tests

2da2eb1

Crash in driver

72b0373

rename

9524986

Add semantic check

b1bec44

Update tracking name

64c2545

Merge remote-tracking branch 'upstream/update-runstep-table' into att…

83cc7f3

…ributeLookup

Add test

da3b9f5

REname tests

c6dfaa1

add tests

cca9aa7

add tests

9020e0c

REname tests

00ebabf

Add tests

cd9fa46

CyrusNajmabadi added 3 commits June 7, 2022 16:21

Add theories

c591e66

Theories

3101731

Merge remote-tracking branch 'upstream/main' into attributeLookup

c6ba1bc

Use reference equality for compilation options comparisons

0cc8e7a

jaredpar approved these changes Jun 9, 2022

View reviewed changes

chsienki reviewed Jun 13, 2022

View reviewed changes

CyrusNajmabadi added 4 commits June 14, 2022 10:23

Merge remote-tracking branch 'upstream/main' into attributeLookup

5f57e08

Move api

6e27446

Fix tests

e566f05

Fix tests

5ec4786

CyrusNajmabadi requested a review from chsienki June 14, 2022 17:36

runfoapp bot mentioned this pull request Jun 14, 2022

NuGet.Protocol.Core.Types.FatalProtocolException exception in helix work items #58197

Open

lint

303d0a1

CyrusNajmabadi mentioned this pull request Jun 14, 2022

Decide on rules for WithTrackingName #61888

Open

chsienki approved these changes Jun 14, 2022

View reviewed changes

CyrusNajmabadi merged commit 44c573d into dotnet:main Jun 14, 2022

CyrusNajmabadi deleted the attributeLookup branch June 14, 2022 21:01

ghost added this to the Next milestone Jun 14, 2022

RikkiGibson modified the milestones: Next, 17.3 P3 Jun 28, 2022

Youssef1313 mentioned this pull request Sep 21, 2022

Move DependencyPropertyGenerator to be incremental unoplatform/uno#9914

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add helper for source generators to filter to nodes with attribute matching an attribute name #61293

Add helper for source generators to filter to nodes with attribute matching an attribute name #61293

CyrusNajmabadi commented May 12, 2022 •

edited

Loading

CyrusNajmabadi May 12, 2022

jaredpar Jun 3, 2022

CyrusNajmabadi Jun 3, 2022

CyrusNajmabadi May 12, 2022

AraHaan Jun 10, 2022 •

edited

Loading

CyrusNajmabadi Jun 10, 2022

jaredpar commented Jun 8, 2022 •

edited

Loading

CyrusNajmabadi commented Jun 9, 2022

stephentoub commented Jun 9, 2022

CyrusNajmabadi commented Jun 9, 2022

AraHaan commented Jun 10, 2022 •

edited

Loading

chsienki Jun 13, 2022

chsienki Jun 13, 2022

chsienki Jun 13, 2022

chsienki Jun 13, 2022

chsienki Jun 13, 2022

CyrusNajmabadi Jun 13, 2022

CyrusNajmabadi commented Jun 14, 2022

CyrusNajmabadi commented Jun 14, 2022

CyrusNajmabadi commented Jun 14, 2022

chsienki commented Jun 14, 2022

chsienki left a comment


		public IncrementalValueProvider<Compilation> CompilationProvider => new IncrementalValueProvider<Compilation>(SharedInputNodes.Compilation.WithRegisterOutput(RegisterOutput).WithTrackingName(WellKnownGeneratorInputs.Compilation));

		internal IncrementalValueProvider<CompilationOptions> CompilationOptionsProvider => new(SharedInputNodes.CompilationOptions.WithRegisterOutput(RegisterOutput).WithTrackingName(WellKnownGeneratorInputs.CompilationOptions));

Add helper for source generators to filter to nodes with attribute matching an attribute name #61293

Add helper for source generators to filter to nodes with attribute matching an attribute name #61293

Conversation

CyrusNajmabadi commented May 12, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AraHaan Jun 10, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaredpar commented Jun 8, 2022 • edited Loading

CyrusNajmabadi commented Jun 9, 2022

stephentoub commented Jun 9, 2022

CyrusNajmabadi commented Jun 9, 2022

AraHaan commented Jun 10, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CyrusNajmabadi commented Jun 14, 2022

CyrusNajmabadi commented Jun 14, 2022

CyrusNajmabadi commented Jun 14, 2022

chsienki commented Jun 14, 2022

chsienki left a comment

Choose a reason for hiding this comment

CyrusNajmabadi commented May 12, 2022 •

edited

Loading

AraHaan Jun 10, 2022 •

edited

Loading

jaredpar commented Jun 8, 2022 •

edited

Loading

AraHaan commented Jun 10, 2022 •

edited

Loading