[cDAC] Pack DataGenerator as analyzer asset in Abstractions nupkg#128903
Conversation
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
There was a problem hiding this comment.
Pull request overview
This PR restructures the cDAC source generator so it can be packed as an analyzer asset inside the Microsoft.Diagnostics.DataContractReader.Abstractions NuGet package, enabling downstream consumers to pick up the generator automatically via PackageReference. It also updates the generator’s helper-emission gating logic to account for symbol accessibility, and adjusts internal project references to the generator’s new gen/ location.
Changes:
- Move/house the cDAC generator under
src/native/managed/cdac/gen/so repo-wide generator-project conventions apply, and update cDAC solution/project references accordingly. - Pack the generator into the Abstractions transport package via
ProjectReference+PackAsAnalyzer="true"(no runtime reference). - Update helper emission gating in the generator to check accessibility (not only metadata presence) when deciding whether to emit shared helper types.
Reviewed changes
Copilot reviewed 6 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/native/managed/cdac/tests/DataGenerator/Microsoft.Diagnostics.DataContractReader.DataGeneratorTests.csproj | Update generator ProjectReference path to ..\..\gen\.... |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Microsoft.Diagnostics.DataContractReader.Contracts.csproj | Update generator analyzer ProjectReference path to ..\gen\.... |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Microsoft.Diagnostics.DataContractReader.Abstractions.csproj | Add PackAsAnalyzer="true" reference to embed the generator DLL under analyzers/dotnet/cs/ in the Abstractions package. |
| src/native/managed/cdac/gen/Microsoft.Diagnostics.DataContractReader.DataGenerator.csproj | Generator project under gen/; relies on repo conventions (IsGeneratorProject/extended rules) and keeps Roslyn dependency private. |
| src/native/managed/cdac/gen/CdacGenerator.cs | Switch helper gating to an accessibility-aware check using GetTypesByMetadataName + IsSymbolAccessibleWithin. |
| src/native/managed/cdac/gen/Parser.cs | Attribute/model parsing and member classification logic for [CdacType], [Field], etc. |
| src/native/managed/cdac/gen/Emitter.cs | Generated source emission for IData patterns (ctor/init, optional fields, writable fields, statics). |
| src/native/managed/cdac/gen/Model.cs | Generator internal model records/enums used by the incremental pipeline. |
| src/native/managed/cdac/gen/EquatableArray.cs | Structural array wrapper to keep incremental step inputs equatable. |
| src/native/managed/cdac/gen/LayoutPairSource.cs | Embedded source text for LayoutPair helper emitted into consuming compilations. |
| src/native/managed/cdac/gen/TypeNameResolverSource.cs | Embedded source text for TypeNameResolver helper emitted into consuming compilations. |
| src/native/managed/cdac/gen/IsExternalInit.cs | netstandard2.0 shim for records/init-only support. |
| src/native/managed/cdac/cdac.slnx | Update solution project path for the generator to gen/.... |
Comments suppressed due to low confidence (1)
src/native/managed/cdac/gen/CdacGenerator.cs:41
- The new
IsTypeAccessiblegating is addressing a subtle cross-assembly accessibility case; there isn’t an obvious regression test covering the scenario where a referenced assembly contains an inaccessible internal helper type with the same metadata name (no InternalsVisibleTo), and the generator must still emit its own helper. Adding a targeted test (likely multi-project) would help prevent future regressions in this gating logic.
Moves Microsoft.Diagnostics.DataContractReader.DataGenerator into src/native/managed/cdac/gen/ so the generatorProjects.targets convention auto-detects IsGeneratorProject=true, and wires it into Abstractions.csproj via <ProjectReference PackAsAnalyzer="true" />. The generator dll then ships under analyzers/dotnet/cs/ inside the Abstractions transport nupkg. Downstream consumers that PackageReference Abstractions (e.g. the internal NativeAOT cDAC repo authoring its own [CdacType] partial classes) now get the source generator automatically without needing to ProjectReference the in-repo DataGenerator csproj. Also fixes two pre-existing generator bugs that only manifested for external PackageReference consumers: 1. The LayoutPair/TypeNameResolver emission gate used GetTypeByMetadataName(...) is null, which skipped emission whenever the type was present in metadata even if not accessible from the current compilation (internal type in a referenced assembly with no InternalsVisibleTo grant). Replaced with an IsSymbolAccessibleWithin check so external consumers get their own internal copy. 2. The singular GetTypeByMetadataName returns null on cross-assembly duplicates (when more than one referenced assembly exposes the same internal helper). Switched to plural GetTypesByMetadataName + Any(accessible) so the gate handles that case correctly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
3444bd9 to
90dbd63
Compare
|
/ba-g the timed out leg actually ran in helix fairly quick - 0 tests failed. Assignment was slow |
Note
This PR description was AI-generated with GitHub Copilot CLI.
Summary
Packs the cDAC source generator (
Microsoft.Diagnostics.DataContractReader.DataGenerator) as an analyzer asset inside theMicrosoft.Diagnostics.DataContractReader.Abstractionstransport NuGet package, so downstream consumers pick the generator up automatically viaPackageReference— no in-repoProjectReferenceto the generator csproj required.Changes
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.DataGenerator/→src/native/managed/cdac/gen/. The repo-wideDirectory.Build.propsauto-detects projects undergen/and setsIsGeneratorProject=true, which importseng/generatorProjects.targets(providing theGetAnalyzerPackFilestarget thatPackAsAnalyzer="true"relies on).Abstractions.csprojvia<ProjectReference ... ReferenceOutputAssembly="false" PackAsAnalyzer="true" />. The compiled generator dll ships underanalyzers/dotnet/cs/in the transport nupkg.IsSourceProject=falseandEnforceExtendedAnalyzerRules=trueoverrides that are now redundant under thegen/convention.Contracts.csprojandDataGeneratorTests.csprojProjectReferencepaths to the new location.Generator bug fixes (uncovered by external
PackageReferenceconsumption)LayoutPair/TypeNameResolveremission gate usedGetTypeByMetadataName(...) is null, which suppressed emission whenever the type was present in metadata even if not accessible to the current compilation (internal type in a referenced assembly withoutInternalsVisibleTo). Replaced with anIsSymbolAccessibleWithincheck so external consumers get their own internal copy generated.GetTypeByMetadataNamereturns null on cross-assembly duplicates (more than one referenced assembly exposes the same internal helper). Switched to pluralGetTypesByMetadataName+Any(accessible)so the gate behaves correctly in that case.These bugs do not affect the in-repo
Contractsbuild (it hasInternalsVisibleToto itself and sees a single copy), only externalPackageReferenceconsumers.Validation
DataGeneratorTestscontinue to use the generator viaProjectReference OutputItemType="Analyzer"(unchanged behavior).analyzers/dotnet/cs/Microsoft.Diagnostics.DataContractReader.DataGenerator.dll.