[generator] Remove IntDefinitionAttribute and the dead javadoc-to-mdoc tool - #12683
Draft
simonrozsival wants to merge 1 commit into
Draft
[generator] Remove IntDefinitionAttribute and the dead javadoc-to-mdoc tool#12683simonrozsival wants to merge 1 commit into
simonrozsival wants to merge 1 commit into
Conversation
This is an experimental, intentionally breaking change proposed as a
draft PR for feedback, not a change intended to merge as-is.
IntDefinitionAttribute
-----------------------
`Android.Runtime.IntDefinitionAttribute` was applied by the generator to
every enum member (11,243 applications across Mono.Android API-37) to
record the originating Java field name/managed member name for the sole
purpose of feeding a legacy javadoc-to-mdoc enum-mapping importer. That
importer is not part of any build in this repo (see below), so the
attribute has no remaining in-tree consumer:
* BindingStudio does not read it; it consumes api.xml/map.csv/methodmap.csv.
* The modern docs pipeline (`_RunMdoc`/`UpdateExternalDocumentation` in
Mono.Android.targets) shells out to the external `mdoc.exe`/`mdoc.dll`
tool and never touches this attribute.
* Trimmed apps already strip every instance of it via
ILLink.LinkAttributes.xml's `RemoveAttributeInstances`, so this mainly
benefits reference assemblies, NuGet package size, and non-trimmed
(library/Debug) consumption.
Changes:
* Delete `Android.Runtime.IntDefinitionAttribute` and its PublicAPI
entries (marked `*REMOVED*` in every active API level's
PublicAPI.Unshipped.txt, following repo convention).
* Stop the generator (EnumGenerator.cs) from emitting `[IntDefinition]`;
remove the now-dead `IntDefinitionAttr` SourceWriter, its support
fixture, and update generator test baselines (489/489 generator-Tests
pass).
* Remove the one hand-authored usage (VibrationEffectEffectType),
preserving the enum values themselves.
* Remove the now-moot ILLink.LinkAttributes.xml entry for the type.
Measured impact (API-37, this machine, before/after, generator
rebuilt for each side):
* Debug: runtime -1,399,296 bytes, ref -1,399,296 bytes
* Release: runtime -1,398,784 bytes, ref -1,398,784 bytes
* CustomAttribute table: exactly 11,243 IntDefinitionAttribute rows
removed in every configuration (plus ~7-11 additional CA rows from
attributes on the deleted type/members themselves).
* #Blob heap: ~1,263,300 bytes smaller in every configuration, closely
matching the prior estimate of 1,263,277 bytes.
* TypeDef -1, MethodDef -5 (the attribute class + its 5 members).
* `dotnet build` ApiCompat against the committed
tests/api-compatibility reference assembly (10,095 attribute usages
in the recorded contract) now reports exactly the expected break:
`TypesMustExist: Type 'Android.Runtime.IntDefinitionAttribute' does
not exist in the implementation but it does exist in the contract`,
Total Issues: 1. This is intentionally NOT suppressed via the
acceptable-breakages file so CI surfaces it for review.
* GeneratedEnumAttribute is untouched (separate file, separate
SourceWriter/CecilApiImporter path) and generated enums are
otherwise unaffected.
Old/pre-compiled bindings that reference the removed attribute type
will continue to compile, load, and run normally for all code that does
not reflect over it; standard CLR semantics mean only reflection calls
that enumerate custom attributes on members decorated with
IntDefinitionAttribute (e.g. `GetCustomAttributes()`) will fail once the
type is gone, since attribute type resolution happens lazily at the
point of reflection. This is the accepted breaking-change surface for
this draft.
javadoc-to-mdoc / JavadocImporter
-----------------------------------
`src/Xamarin.Android.Tools.JavadocImporter/` (javadoc-to-mdoc.cs and
support files) and `tools/javadoc2mdoc/` (its console driver) are not
referenced by any .slnx/.sln in this repo -- grepping
Xamarin.Android.slnx, Xamarin.Android.Build.Tasks.slnx, and
Xamarin.Android-Tests.slnx for "javadoc" returns zero matches, so
neither project is part of the current build graph. The MSBuild flag
that would route to it, `_UseLegacyJavadocImport`, is never set to
`true` anywhere in-repo, so its only consumer
(`Xamarin.Android.Tasks.ImportJavaDoc`, invoked from
Xamarin.Android.Bindings.Documentation.targets'
`BuildDocumentation` target) was already unreachable dead code before
this change. No CI workflow, Makefile target, or doc references the
tool either.
Changes:
* Delete `src/Xamarin.Android.Tools.JavadocImporter/` and
`tools/javadoc2mdoc/` entirely (16 files; no .csproj/.slnx changes
needed since neither was referenced anywhere).
* Remove the `JavadocToMdocToolExe` property
(Xamarin.Android.Bindings.Core.targets) and the dead `ImportJavaDoc`
MSBuild task invocation + UsingTask declaration
(Xamarin.Android.Bindings.Documentation.targets), plus the now-unused
`ImportJavaDoc.cs` task class in Xamarin.Android.Build.Tasks.
* Left `JavaDoc`/`MDoc` tasks, `_ExtractJavaDocJars`,
`_GenerateJavaDocFromSourceJars`, and the `JavaDocJar`/`JavaSourceJar`
item groups untouched: these wrap the JDK's own `javadoc` tool and
the external `mdoc.exe`, are unrelated to the deleted
javadoc-to-mdoc.cs tool, and remain part of the (still-gated,
unrelated) legacy binding-project doc-extraction infrastructure --
out of scope for this change.
* Left the already-`[Obsolete("Do not use.")]`
`Xamarin.AndroidTools.MonoDroidSdk.JavaDocToMDocExe` string constant
untouched (different assembly, already marked dead by a prior
maintainer, no project dependency).
* Confirmed via full-repo search that the only remaining references to
IntDefinitionAttribute/JavadocImporter/javadoc-to-mdoc are in
`bin/` build output (gitignored) and the `external/android-api-docs`
submodule's already-published historical docs (out of scope, a
separate repo/history).
Validated with:
* `dotnet test external/Java.Interop/tests/generator-Tests/generator-Tests.csproj`
-> 489/489 passed.
* `dotnet build src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.csproj`
(which pulls in Mono.Android) -> 0 errors with
`-p:DisableApiCompatibilityCheck=True`; without it, exactly the
expected single ApiCompat TypesMustExist finding.
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.
This is an intentionally breaking, experimental change. It is not a drop-in fix — it removes a public API type and deletes a whole (dead) tool. Opening as a draft to get feedback on the approach, the compatibility analysis, and whether this is something the team wants to pursue, before investing in updating the committed API-compat reference/acceptable-breakages files.
Background
Android.Runtime.IntDefinitionAttributeis applied by the generator to every enum member it binds (11,243 applications on Mono.Android API-37 alone). Each application carries two strings: the managed member name (ConstantMember) and the originating Java field (JniField). Investigation showed this attribute exists solely to feed a legacy javadoc→mdoc enum-mapping importer (javadoc-to-mdoc.cs/Xamarin.Android.Tools.JavadocImporter), and that importer turns out to be dead code that isn't part of any build in this repo.Evidence the attribute has no remaining in-tree consumer
IntDefinitionAttributeat all — it consumesapi.xml/map.csv/methodmap.csv(per the requester's own research)._RunMdoc/UpdateExternalDocumentationinsrc/Mono.Android/Mono.Android.targets) shells out to the externalmdoc.exe/mdoc.dllNuGet tool viamdoc update/mdoc fx-bootstrap. It has nothing to do with this attribute or withjavadoc-to-mdoc.cs.IntDefinitionAttributeviaILLink.LinkAttributes.xml'sRemoveAttributeInstances. So this change mainly benefits reference assemblies, NuGet package size, and non-trimmed (Debug/library) consumption — trimmed/linked apps see no runtime benefit since the attribute was already gone there.Evidence javadoc-to-mdoc is dead code
grep -i javadoc Xamarin.Android.slnx Xamarin.Android.Build.Tasks.slnx Xamarin.Android-Tests.slnx→ zero matches. Neithersrc/Xamarin.Android.Tools.JavadocImporter/Xamarin.Android.Tools.JavadocImporter.csprojnortools/javadoc2mdoc/javadoc2mdoc.csprojis referenced by any solution in this repo — they are not part of the build graph at all, not merely unused at runtime._UseLegacyJavadocImport, is never set totrueanywhere in-repo. Its only consumer, theXamarin.Android.Tasks.ImportJavaDoctask (invoked fromXamarin.Android.Bindings.Documentation.targets'sBuildDocumentationtarget), was therefore already unreachable dead code before this PR — and it targets a tool executable (javadoc-to-mdoc.exe) that no project in this repo has built for some time anyway.What this PR removes
IntDefinitionAttribute:src/Mono.Android/Android.Runtime/IntDefinitionAttribute.cs(the type itself — not gated, not deprecated, fully removed).external/Java.Interop/.../EnumGenerator.cs) from emitting[IntDefinition]; removes the now-deadIntDefinitionAttrSourceWriter helper and its test support fixture; updates generator expected-output test baselines.Android.OS.VibrationEffectEffectType(the enum values are preserved — only the attribute applications are removed).ILLink.LinkAttributes.xmlentry for the type.Android.Runtime.IntDefinitionAttribute*entries from every active API level'sPublicAPI.Shipped.txt, and adds the corresponding*REMOVED*markers to eachPublicAPI.Unshipped.txt(API-35, 36, 36.1, 37, 37.1, 37.2), matching this repo's convention for intentional public API removal.javadoc-to-mdoc/JavadocImporter:src/Xamarin.Android.Tools.JavadocImporter/(14 files: the importer source, HTML/DTD entity resources, etc.) andtools/javadoc2mdoc/(its console driver) entirely. No.csproj/.slnxedits were needed for this because — per the evidence above — neither project was referenced anywhere.JavadocToMdocToolExeMSBuild property (Xamarin.Android.Bindings.Core.targets) and the deadImportJavaDoctask invocation + itsUsingTaskdeclaration (Xamarin.Android.Bindings.Documentation.targets), plus the now-unusedImportJavaDoc.cstask class fromXamarin.Android.Build.Tasks.JavaDoc/MDocMSBuild tasks,_ExtractJavaDocJars/_GenerateJavaDocFromSourceJarstargets, andJavaDocJar/JavaSourceJaritem groups. These wrap the JDK's ownjavadoctool and the externalmdoc.exe— they are unrelated to the deletedjavadoc-to-mdoc.cstool and remain part of a separate (still_UseLegacyJavadocImport-gated, still unrelated) legacy binding-project doc-extraction path that is out of scope here. Also left untouched: the already-[Obsolete("Do not use.")]Xamarin.AndroidTools.MonoDroidSdk.JavaDocToMDocExestring constant (different assembly, already marked dead by a prior maintainer).IntDefinitionAttribute/JavadocImporter/javadoc-to-mdocare inbin/build output (gitignored) and theexternal/android-api-docssubmodule's already-published historical docs for older API levels (a separate repo/history, out of scope).Measured size impact (API-37, before/after, generator rebuilt for each side)
Metadata-level breakdown (via
System.Reflection.Metadata, all four Debug/Release runtime/ref DLLs):IntDefinitionAttributerows removed in every configuration (this matches the prior estimate exactly), plus a further 7-11 CA rows from attributes applied to the deleted type/members themselves.ApiCompat
Running a normal
dotnet build(i.e. withoutDisableApiCompatibilityCheck) against the committedtests/api-compatibility/reference/net11.0/Mono.Android.dllcontract (which itself contains 10,095IntDefinitionAttributeusages) now reports:This is the expected, intended breaking change — ApiCompat's
TypesMustExistrule reports the missing type once rather than once per usage site. This PR deliberately does not updatetests/api-compatibility/acceptable-breakages-vReference-net11.0.txtto suppress this, so CI will flag it for reviewers rather than silently pass. If this design is accepted, that file (and the reference dump) would need to be regenerated as a follow-up.Compatibility risk for old/pre-compiled bindings
Any previously-compiled assembly (NuGet binding package, app, etc.) that references
Android.Runtime.IntDefinitionAttributewill:GetCustomAttributes()/GetCustomAttributesData()on a member decorated with it.IntDefinitionAttribute(standard CLR behavior for a missing metadata-referenced type). No in-tree code does this after this PR (see evidence above), so the risk is scoped to external tooling/reflection that specifically walks enum-member attributes looking forIntDefinitionAttribute— which, per the dead-tool evidence above, should not exist for any currently-active in-repo or documented workflow.Verified unaffected
GeneratedEnumAttribute— untouched; it lives in a separate file with its own SourceWriter andCecilApiImportercode path, structurally unrelated toIntDefinitionAttribute.GeneratedEnumAttributeapplications are unchanged; only the extra[IntDefinition]application is gone.generator-Tests: 489/489 passed after the removal (fixture/expected-output files updated to match the new no-attribute output).dotnet build src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.csproj(which pulls in Mono.Android as a project reference) builds with 0 errors using-p:DisableApiCompatibilityCheck=True; without that flag, exactly the single expected ApiCompat finding above.What feedback/CI signal this draft is seeking
IntDefinitionAttributeandjavadoc-to-mdoc/JavadocImporterare truly dead and safe to remove outright (vs. a more conservative gate-off-by-default design that was also prototyped and can be resurrected if preferred).TypesMustExistfinding) is acceptable to formally suppress via the acceptable-breakages file, or whether this should instead ship as a genuinely breaking change in a major version with release-notes coverage.Co-authored-by: Copilot App 223556219+Copilot@users.noreply.github.com