Repository navigation
Implement a Roslyn source generator for AssemblyScanAttribute - #3593
Conversation
|
🤖 Draft PR — automatic CI is skipped to save runner minutes.
|
cf8f1d0 to
2a8142a
Compare
xen2
left a comment
There was a problem hiding this comment.
Thanks! A few points:
- Rebase on latest
masterwhich merged #3678. Then you can rebuild AssemblyProcessor which is now deterministic (anybody can verify the build). - Some comments still say the scan index comes from the assembly processor (
AssemblyRegistry.cs,Game.cs,StrideDefaultAssetsPlugin.cs,EditorGameController.cs, ...), and theStrideAssemblyProcessorcomments inStride.Audio.Editor.Wpf.csproj/Stride.UI.Editor.Wpf.csproj. Could you update them? Are those two still needed? - Could you add a few generator tests in
Stride.Core.CompilerServices.Tests(Inherited = false, generic interface, nested type, keyword name)?
Filter duplicate attribute type definitions from generator scan results.
…dn't have attributes or base types listed
Filter inaccessible symbols from assembly-scan generation to prevent generated code from referencing types that are not visible within the assembly. Centralize the accessibility check in a shared compilation helper.
Rename `AssemblyScanGenerator` to `UnifiedGenerator`.
Move generator data and definition records into partial-class files, make the generator sealed, and centralize attribute inheritance detection in a symbol extension.
Prefix reserved and contextual C# keywords with `@` in generated namespace, type, and generic parameter names to keep generated code valid.
Lower the assembly scan results module initializer order from -1000 to -10000 so they run earlier.
Enumerate referenced modules and scan types only in Stride.Core or modules that reference it. Update type enumeration to work from module symbols.
2a8142a to
9610783
Compare
Remove the `StrideAssemblyProcessor` setting from the Audio and UI WPF editor projects.
Thread `CancellationToken` through reference scanning, type data generation, and scan result computation. Check cancellation while processing modules, types, interfaces, targets, and attributes so generator execution stops promptly when cancelled.
xen2
left a comment
There was a problem hiding this comment.
Thanks for the fixes! The AssemblyProcessor binaries match a rebuild of the PR's source, so that part is good (thanks for being the first beta-tester).
You're right about RegisterImplementationSourceOutput, the IDE doesn't run it.
The filter is still nice for builds.
Still open: a few generator tests in Stride.Core.CompilerServices.Tests (Inherited = false, generic interface, nested type, keyword name). They would have caught the issue below.
Let me know if you want me to generate them?
…nerator.cs Co-authored-by: xen2 <virgile.bello@gmail.com>
Rename `GeneratorTestBase` to `UnifiedGeneratorTestBase` and update `DerivedClassTests` inheritance while preserving the existing test context and configuration.
|
@xen2 I added some tests. Let me know if you think anything else should be covered. |
xen2
left a comment
There was a problem hiding this comment.
Thanks for the tests!
Just small things left below.
Move source generator testing to test-only dependencies and remove the unused Net100 reference assemblies package.
Rename `GenericInterfaceTests` to `InterfaceTests` and move the generic interface scanning test. Add coverage verifying assembly scanning discovers `IAppSettingsProvider` implementations from referenced assemblies.
|
Editor CI is failing on all 3 graphics APIs, GameStudio crashes on startup with Looks like GameStudio now gets its own scan index (the old processor only did it for How about making the registration lazy? Module init would just register a factory, something like |
I'll take a look at this tomorrow.
Do you want me to unmerge the generators, or is it still good to have a defined point in module initialization for that? |
…tScanTypes (building it in the module initializer loaded the scanned types' assemblies too early)
# Conflicts: # sources/engine/Stride.Engine/Engine/Game.cs
…it again (AssetQuantumRegistry asks for it from a module initializer the index build triggers)
I took the liberty to do the lazy registration, so that I could merge this right away and you could focus on next steps of assembly processor removal. |
|
I appreciate you getting this across the finish line. I'll rebase the other pull requests promptly! |
No problem, it's me thanking you for all your work on removing the assembly processor. It's a huge task! |
PR Details
This pull request replaces the
AssemblyScanProcessorin the AssemblyProcessor with a new Roslyn source generator inStride.Core.CompilerServices.Related Issue
Part of #1835
Types of changes
Checklist
Note
thatthis needs merged first.I haven't deletedAssemblyScanProcessornor recompiled the AssemblyProcessor. I will do that after Replace ParameterKeyProcessor with Roslyn source generation #3584 is merged.I haven't done any validation beyond checking that Stride compiles. I will do that before I mark this pull request ready for review.