dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.55k stars 4.54k forks source link

Remove SyntaxTree rooting from interop source generators #83870

Open jkoritzinsky opened 1 year ago

jkoritzinsky commented 1 year ago

In the interop source generators, we root the syntax tree with our Location objects we propagate for diagnostic reporting. We should use the tricks used in #83868 to create Location objects that don't root SyntaxTree objects to reduce the places we root Roslyn object inputs.

ghost commented 1 year ago

Tagging subscribers to this area: @dotnet/interop-contrib See info in area-owners.md if you want to be subscribed.

Issue Details
In the interop source generators, we root the syntax tree with our Location objects we propagate for diagnostic reporting. We should use the tricks used in #83868 to create Location objects that don't root SyntaxTree objects to reduce the places we root Roslyn object inputs.
Author: jkoritzinsky
Assignees: -
Labels: `area-System.Runtime.InteropServices`, `tenet-performance`, `source-generator`
Milestone: -
Sergio0694 commented 1 year ago

Related: could we consider trying to push to get https://github.com/dotnet/roslyn/issues/63776 implemented, especially since you also mentioned you'd be able to contribute the feature yourself assuming it got approved? Using those APIs would completely solve this issue while also allowing both these source generators and the regex ones to be simplified quite a bit, due to no longer having to maintain that very awkward tuple-return-and-branch pattern to return diagnostics and immediately redirect them to an output node before getting just the incremental models to continue down the pipeline.

As a nice side benefit, they'd also be a set of APIs that'd greatly benefit a ton of other generators out there and not just the ones in the runtime (eg. ours in the MVVM Toolkit, as well as those in ComputeSharp, etc.), which is also nice on its own.

What do you think? 🙂