Skip to content

Drop isReferenceType when erasing unknown types - #4070

Merged
siegfriedpammer merged 1 commit into
masterfrom
fix/unknown-type-erasure
Aug 29, 2026
Merged

siegfriedpammer merged 1 commit into
masterfrom
fix/unknown-type-erasure

Conversation

@siegfriedpammer

Copy link
Copy Markdown
Member

Problem

Whether an unresolvable type is a reference type is not a property of the type, but of the metadata that mentioned it: a signature spelling valuetype T yields isReferenceType == false, a bare TypeRef (MemberRef parent, constrained. prefix) yields null.

UnknownType.Equals compares that flag, so the two spellings of one and the same missing type compare unequal. NormalizeTypeVisitor.TypeErasure.EquivalentTypes then reports false and the decompiler emits a cast between a type and itself, e.g. in ILPretty/Issue3729:

MyStruct[] array = (MyStruct[])(object)new MyStruct[1] { new MyStruct(7) };

Fix

NormalizeTypeVisitor erases the flag before comparing. That keeps the relaxation inside the comparisons that ask for erasure, next to the nullability, modopt and tuple erasure it already performs — all use-site spellings of the same kind.

Why not drop the term from UnknownType.Equals

That was the other candidate; it was built and measured, and it is worse. Equals is global, and among other things it keys CSharpConversions.implicitConversionCache (ConcurrentDictionary<(IType, IType), Conversion>). UnknownType.GetHashCode already ignores isReferenceType, so both spellings already share a hash bucket and only Equals keeps them apart; merging them lets whichever conversion is computed first answer for both.

Measured over 15 nuget assemblies decompiled with their dependencies absent:

variant (object) casts
master 21934
Equals change 21824 (−163 in Azure.AI.OpenAI, +398 in Themes.Fluent/Simple)
this PR 21798, never worse on any assembly

Under the Equals variant two sibling types in one method are treated differently — PushParent(val5) next to PushParent((object)val21), both unresolved reference types.

Test

ILPretty/Issue3729's expectation carried the cast above and is updated to the correct output; it is the regression guard for this change.

ICSharpCode.Decompiler.Tests: 3539 total, 3494 passed, 45 skipped, 0 failed.

  • At least one test covering the code changed

Written by Claude Code (claude-opus-5) on behalf of @siegfriedpammer; measurements and test runs reproduced locally.

Comment thread ICSharpCode.Decompiler/TypeSystem/Implementation/UnknownType.cs Outdated
Whether an unresolvable type is a reference type is not a property of the
type but of the metadata that mentioned it: a signature spelling it
`valuetype T` yields false, a bare TypeRef yields null. UnknownType.Equals
compares the flag, so the two spellings of one missing type compared
unequal and EquivalentTypes reported false - the decompiler then emitted a
cast between a type and itself.

Erasing the flag in NormalizeTypeVisitor keeps the relaxation inside the
comparisons that ask for erasure, next to the nullability, modopt and tuple
erasure that are use-site spellings of the same kind. Dropping the term
from UnknownType.Equals instead was measured and rejected: Equals also keys
CSharpConversions' implicit-conversion cache, where merging the two
spellings lets whichever conversion is computed first answer for both,
adding 398 boxing casts across two real-world assemblies.

Assisted-by: Claude:claude-opus-5[1m]:Claude Code
@siegfriedpammer
siegfriedpammer force-pushed the fix/unknown-type-erasure branch from 3a60798 to 4bc51e6 Compare August 29, 2026 16:26
@siegfriedpammer
siegfriedpammer merged commit b279efd into master Aug 29, 2026
19 of 20 checks passed
@siegfriedpammer
siegfriedpammer deleted the fix/unknown-type-erasure branch August 29, 2026 17:40
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.

2 participants