[typemap] Diagnose unsupported constructor shapes - #12567
Open
simonrozsival wants to merge 65 commits into
Open
Conversation
27 tasks
Contributor
There was a problem hiding this comment.
Copilot review overview
Review tier: Lite
Findings: 1
New issues introduced by this change (2)
| Severity | Finding |
|---|---|
 |
tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Scanner/ConstructorDetectionTests.cs — 💡 suggestion — ScanPeer opens and reads both fixture assemblies from disk on every test invocation.… |
 |
src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/JavaPeerScanner.cs — ❌ error — Base-constructor compatibility for non-explicit constructors is currently checked via… |
What changed in this PR
Adds first-class constructor-shape validation to the trimmable typemap pipeline, surfacing new localized XA4259–XA4262 diagnostics and ensuring generation stops before emitting partial Java/type-map output when constructors are not representable or are ambiguous.
Changes:
- Introduces constructor diagnostics (collision, unsupported parameter shapes, missing compatible base ctor, invalid
SuperArgumentsString) and wires them into the generator/Build.Tasks logger with localized resources. - Expands test coverage with new “invalid constructor” fixture assembly and adds focused unit/integration/build tests to validate diagnostics and “no partial output” behavior.
- Documents new error codes (XA4259–XA4262) and adds them to the docs index/TOC.
| File | Description |
|---|---|
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/TestFixtures/StubAttributes.cs | Extends stub ExportAttribute to support ctor usage and SuperArgumentsString for scanner tests. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Scanner/ConstructorDetectionTests.cs | Adds targeted tests for new constructor diagnostics and representable/explicit cases. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests.csproj | Adds a new fixture project and copies its output alongside existing fixtures. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/InvalidConstructorFixtures/InvalidConstructors.cs | New fixture types that intentionally trigger (or avoid) constructor diagnostics. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/InvalidConstructorFixtures/InvalidConstructorFixtures.csproj | New fixture assembly project (unsafe enabled) used as scanner/generator input. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TrimmableTypeMapGeneratorTests.cs | Verifies generator logs coded errors and returns no partial outputs when ctor diagnostics exist. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTests/UserTypesFixture/UserTypesFixture.csproj | Enables unsafe to support new user fixture types using pointers/function pointers. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTests/UserTypesFixture/UserTypes.cs | Adds user fixture types mirroring ctor-collision/unrepresentable/super-args scenarios. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTests/ScannerRunner.cs | Adds legacy constructor extraction to support ctor parity tests. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTests/ScannerComparisonTests.cs | Excludes ctor-diagnostic fixtures from legacy↔new marshal-method parity comparisons. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTests/ConstructorParityTests.cs | New integration tests that pin legacy behavior around ctor collisions/unrepresentable ctors/super args. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/TrimmableTypeMapBuildTests.cs | Adds a build-level regression ensuring XA4259 fails before any partial typemap Java is written. |
| src/Xamarin.Android.Build.Tasks/Tasks/GenerateTrimmableTypeMap.cs | Implements new logger hooks for XA4259–XA4262 in the MSBuild task. |
| src/Xamarin.Android.Build.Tasks/Properties/Resources.resx | Adds localized strings for XA4259–XA4262. |
| src/Xamarin.Android.Build.Tasks/Properties/Resources.Designer.cs | Updates generated resource accessors for XA4259–XA4262. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/TrimmableTypeMapGenerator.cs | Validates constructor diagnostics early and returns no generated outputs on failure. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/JavaPeerScanner.cs | Computes ctor diagnostics during scanning, including signature collapse, parameter-shape validation, base-ctor compatibility, and SuperArgumentsString validation. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/JavaPeerInfo.cs | Adds ConstructorDiagnostics model plus diagnostic kind/info types. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/ITrimmableTypeMapLogger.cs | Extends logger interface with ctor diagnostic logging hooks. |
| Documentation/docs-mobile/TOC.yml | Adds entries for XA4259–XA4262 docs. |
| Documentation/docs-mobile/messages/xa4259.md | New documentation page for XA4259. |
| Documentation/docs-mobile/messages/xa4260.md | New documentation page for XA4260. |
| Documentation/docs-mobile/messages/xa4261.md | New documentation page for XA4261. |
| Documentation/docs-mobile/messages/xa4262.md | New documentation page for XA4262. |
| Documentation/docs-mobile/messages/index.md | Adds XA4259–XA4262 to the message index listing. |
Files not reviewed (1)
- src/Xamarin.Android.Build.Tasks/Properties/Resources.Designer.cs: Generated file
simonrozsival
force-pushed
the
simonrozsival-constructor-signature-diagnostics
branch
from
August 31, 2026 14:46
e03c49d to
c385503
Compare
simonrozsival
changed the base branch from
main
to
simonrozsival-unsupported-export-signatures
August 31, 2026 14:46
simonrozsival
force-pushed
the
simonrozsival-constructor-signature-diagnostics
branch
2 times, most recently
from
September 1, 2026 12:50
765d233 to
8936f2a
Compare
simonrozsival
force-pushed
the
simonrozsival-constructor-signature-diagnostics
branch
2 times, most recently
from
September 1, 2026 13:15
9d75936 to
7854663
Compare
simonrozsival
force-pushed
the
simonrozsival-constructor-signature-diagnostics
branch
from
September 1, 2026 13:33
7854663 to
8b98f5f
Compare
Match legacy XA4205 and XA4208 validation in the trimmable scanner, before invalid field members can reach generated typemap or Java outputs. Extend semantic, build, and device coverage for valid static and instance ExportField behavior and classify legacy-unsupported field names. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Prove that parameter-count validation takes precedence over void-return validation for an initializer violating both rules, matching measured llvm-ir behavior across trimmable CoreCLR and NativeAOT without partial outputs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Match legacy XA4207 precedence for ExportField methods declared on generic types before parameter-count or void-return validation, and reject them before typemap or Java outputs are written. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Ignore unrelated attributes that share the ExportFieldAttribute simple name by matching the Java.Interop namespace consistently during validation, field collection, and marshal-method registration. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Remove duplicated scanner fixtures and keep focused build/runtime parity coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
simonrozsival
force-pushed
the
simonrozsival-constructor-signature-diagnostics
branch
from
September 1, 2026 13:54
8b98f5f to
3ea14be
Compare
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Reject unresolved managed, generic, and function-pointer export signatures during trimmable scanning with localized XA4263 diagnostics before any typemap, Java, or ACW-map output is written. Preserve legacy XA4206 precedence and supported Java mappings. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Honor ExportParameter return mappings on exported fields, reject incompatible special mappings before generation, and resolve Java peer descriptors by assembly identity through type forwarders. Add semantic, cross-assembly, and three-runtime regressions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
simonrozsival
force-pushed
the
simonrozsival-constructor-signature-diagnostics
branch
from
September 1, 2026 15:16
3ea14be to
663a4ab
Compare
Reject unresolved managed, generic, and function-pointer export signatures during trimmable scanning with localized XA4263 diagnostics before any typemap, Java, or ACW-map output is written. Preserve legacy XA4206 precedence and supported Java mappings. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Honor ExportParameter return mappings on exported fields, reject incompatible special mappings before generation, and resolve Java peer descriptors by assembly identity through type forwarders. Add semantic, cross-assembly, and three-runtime regressions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Require special ExportParameter mappings to target exact scalar Stream or XmlReader types, and match Export, ExportParameter, and ExportField attributes by full Java.Interop identity throughout scanner validation and collection. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Ignore static exported constructors, diagnose unresolved instance constructor parameters without overlapping XA4260 shape ownership, preserve ExportParameter kinds through UCO activation, and resolve enum signatures by assembly identity. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Ignore Export name overrides on metadata constructors, assert UCO dispatch calls the managed constructor rather than a virtual method, and recursively reserve nested structural parameter shapes for XA4260. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Require Stream and XmlReader special export mappings to resolve from their framework assemblies or forwarders, preventing same-full-name user types from receiving framework adapters. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Recognize System.Private.Xml as the canonical XmlReader definition and verify both direct and multi-facade mappings without accepting same-full-name user types. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Exercise the resolved wrong-assembly branch by indexing a user assembly that defines System.Xml.XmlReader instead of relying on a missing assembly reference. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Preserve undefined ExportParameterKind values through scanner validation so unsupported signatures report XA4263 instead of being treated as unspecified. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Use each assembly index's signature provider, pass the type handle to peer detection, and remove the unmatched class brace. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Reuse the constructor fixture scan results across tests instead of reopening and rescanning the same assemblies for each invocation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Resolve conflicts between the refreshed base branch and constructor-shape diagnostics while preserving both validation paths and their test coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Import System.Text for Encoding and include all constructor fixture projects in the solution so Release builds produce the DLLs copied by the test project. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Base automatically changed from
simonrozsival-unsupported-export-signatures
to
main
September 4, 2026 07:20
simonrozsival
added a commit
that referenced
this pull request
Sep 4, 2026
## Summary - reject unsupported `[Export]`/`[ExportField]` signatures during trimmable scanning with localized XA4263 before typemap, Java, or ACW-map output - preserve supported Java peers/interfaces, primitives, strings, ordinary arrays, enums, collections, and valid scalar `[ExportParameter]` mappings - match Export attributes by full `Java.Interop` identity and resolve peer/enum/special framework types by assembly identity through forwarders - validate exported constructors while preserving constructor identity and scalar adapter dispatch ## Final type-identity behavior Special mappings require canonical framework identity, not just a managed full name: - `System.IO.Stream`: `System.Runtime` facade or resolved `System.Private.CoreLib` - `System.Xml.XmlReader`: `System.Xml.ReaderWriter` facade or resolved `System.Private.Xml` - multi-hop facades such as `netstandard → System.Xml.ReaderWriter → System.Private.Xml` are accepted through the existing cycle-safe forwarder resolver - user assemblies defining the same full names are rejected with XA4263 for method parameter/return, ExportField return, and constructor parameter paths Tests use minimal metadata assemblies for direct `System.Private.Xml`, the two-hop facade chain, and a resolved `User.Xml` assembly containing `System.Xml.XmlReader`. A separately compiled Android fixture and end-to-end builds retain the user-collision/no-output controls. ## Constructor diagnostics ownership PR #12567 owns XA4260 for generic, byref, pointer, function-pointer, rectangular-array constructor parameters, including nested SZ-array forms. XA4263 owns unresolved managed types and invalid `[ExportParameter]` kind/type/identity pairs. This PR does not duplicate XA4259/XA4261 analysis. An isolated composition with the latest #12567 reproduces its remaining follow-up: two XA4263-rejected overloads on the same type with default/missing `SuperArgumentsString` also receive secondary XA4259 and XA4261. The coordinator will make that analyzer skip XA4263-rejected constructors after rebasing #12567 above this PR. ## Validation - 802 standalone trimmable tests - 27 integration tests, including semantic classfile comparison, javac, peer/enum collisions, and Stream/XmlReader collisions - 66 export-signature/runtime-identity and no-output build tests - valid llvm-ir CoreCLR, trimmable CoreCLR, and trimmable NativeAOT fixture builds - legacy `GenerateExportedMembers` - latest #12567 composition: stronger two-overload secondary-diagnostic reproduction and 838 full combined standalone tests - `git diff --check`; every added line at most 180 characters; full-name attribute audit clean Stacked on #12596. Tracks #12561.
Match legacy JCW generation by silently skipping ordinary constructors whose managed parameters cannot be represented in Java, while retaining diagnostics for explicit exported constructors. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
SuperArgumentsStringparameter referencesSuperArgumentsString: tokenize literals/comments and defer any expression containing Java lambda (->) or method-reference (::) syntax entirely to javac; retain XA4262 for ordinary bare/call/arithmeticpNreferencesValidation
git diff --check; acceptance commit adds 33 lines (within 180)Part of #12561