Skip to content

Remove redundant primitive-key constraint for old Mono compatibility - #63

Merged
SamboyCoding merged 1 commit into
SamboyCoding:masterfrom
ObjectInSpace:lazy-reflection-lookups
Aug 17, 2026
Merged

Remove redundant primitive-key constraint for old Mono compatibility#63
SamboyCoding merged 1 commit into
SamboyCoding:masterfrom
ObjectInSpace:lazy-reflection-lookups

Conversation

@ObjectInSpace

@ObjectInSpace ObjectInSpace commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Problem

TomlSerializationMethods resolves private generic methods using reflection during type initialization. One of those methods is declared with this constraint:

where TKey : unmanaged, IConvertible

The unmanaged constraint is emitted as System.ValueType modreq(System.Runtime.InteropServices.UnmanagedType). Mono versions predating the fix for mono/mono#6804 cannot resolve that metadata while enumerating the type's methods and throw:

System.TypeLoadException: Could not load type 'Typespec 0x1b000001'.
  at (wrapper managed-to-native) System.MonoType:GetMethodsByName (string,System.Reflection.BindingFlags,bool,System.Type)
  at System.MonoType.GetMethodImpl (System.String name, BindingFlags bindingAttr, ...)
  at System.Type.GetMethod (System.String name, BindingFlags bindingAttr)
  at Tomlet.TomlSerializationMethods..cctor ()

Because the lookup happens in the static constructor, this prevents any use of TomlSerializationMethods. The constraint is also present in the non-MODERN_DOTNET branch compiled for the declared netframework3.5 target.

Change

Remove the redundant unmanaged, IConvertible constraint from PrimitiveKeyedDictionaryDeserializerFor.

The method is private and is only constructed through reflection after GetDeserializer has restricted TKey to integer types, bool, or char. Its body does not rely on either constraint. Removing it therefore preserves the accepted inputs while avoiding the incompatible metadata.

Verification

  • All target frameworks build, including netframework3.5.
  • Test suite passes: 164/164.
  • Verified this constraint-removal revision on Unity 5.0.1f1's embedded Mono, x86, net35 profile. Preferences loaded, the mod initialized fully, and the log contained no TypeLoadException or TypeInitializationException.

Notes

Found while modding a Unity 5.0.1 game, where MelonLoader bundles Tomlet for its config system. I haven't added a regression test because CI does not execute the net35 output on an affected old Mono runtime.

Investigated and drafted with AI assistance. Everything above was verified as described.

@Windows10CE

Windows10CE commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Why would you not simply remove the constraint? It isn't actually doing anything here and the method is already only used internally through reflection

@ObjectInSpace
ObjectInSpace force-pushed the lazy-reflection-lookups branch from 97f02fb to 342fa7d Compare August 16, 2026 06:54
@ObjectInSpace ObjectInSpace changed the title Resolve reflection lookups lazily so a failure can't poison the whole type Remove redundant primitive-key constraint for old Mono compatibility Aug 16, 2026
@ObjectInSpace

Copy link
Copy Markdown
Contributor Author

Good point — the caller already restricts the key type before constructing this private method, and the body doesn't rely on either constraint. I've revised the PR to remove the constraint directly and reverted the lazy lookup changes. All targets build, including netframework3.5, and all 164 tests pass.

@ObjectInSpace

ObjectInSpace commented Aug 16, 2026

Copy link
Copy Markdown
Contributor Author

tested with Unity 5.0.1f1's embedded Mono (x86, net35). mod initialized fully, and the complete log contained no Tomlet, TypeLoadException, or TypeInitializationException failures.

@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 31932571884

Coverage remained the same at 91.24%

Details

  • Coverage remained the same as the base build.
  • Patch coverage: No coverable lines changed in this PR.
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 2146
Covered Lines: 1958
Line Coverage: 91.24%
Relevant Branches: 1144
Covered Branches: 976
Branch Coverage: 85.31%
Branches in Coverage %: No
Coverage Strength: 219.31 hits per line

💛 - Coveralls

@SamboyCoding
SamboyCoding merged commit 723cdfd into SamboyCoding:master Aug 17, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants