From e475e264432981fe56b649cea32b8564a09db6b3 Mon Sep 17 00:00:00 2001 From: Christoph Wille Date: Sat, 15 Aug 2026 11:35:07 +0200 Subject: [PATCH] Make ThisResolveResult an ILVariableResolveResult of the this parameter 'this' and 'base' both read the 'this' parameter of the function being decompiled, but their resolve result did not say so: consumers that key on ILVariableResolveResult (local-reference output, highlighting, hover) could not connect the keyword to the variable, and the qualified/unqualified spellings of the same access carried differently shaped annotations. The resolver has no ILFunction and thus no variable to put into a ThisResolveResult, so it stops synthesizing one: LookInCurrentType looks the name up against the (self-parameterized) current type, which grants the same protected access, and the annotation of an unqualified field access is built from the translated target instead. ResolveThisReference and ResolveBaseReference had no callers left and are removed. Assisted-by: Claude:claude-fable-5:Claude Code --- .../Semantics/ThisResolveResultTests.cs | 141 ++++++++++++++++++ .../CSharp/ExpressionBuilder.cs | 28 ++-- .../CSharp/Resolver/CSharpResolver.cs | 53 ++----- .../IL/Instructions/PatternMatching.cs | 14 +- .../Semantics/ThisResolveResult.cs | 9 +- 5 files changed, 185 insertions(+), 60 deletions(-) create mode 100644 ICSharpCode.Decompiler.Tests/Semantics/ThisResolveResultTests.cs diff --git a/ICSharpCode.Decompiler.Tests/Semantics/ThisResolveResultTests.cs b/ICSharpCode.Decompiler.Tests/Semantics/ThisResolveResultTests.cs new file mode 100644 index 000000000..2dfd39367 --- /dev/null +++ b/ICSharpCode.Decompiler.Tests/Semantics/ThisResolveResultTests.cs @@ -0,0 +1,141 @@ +// Copyright (c) 2026 Christoph Wille +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of this +// software and associated documentation files (the "Software"), to deal in the Software +// without restriction, including without limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons +// to whom the Software is furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR +// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE +// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +using System; +using System.Linq; + +using ICSharpCode.Decompiler.CSharp; +using ICSharpCode.Decompiler.CSharp.Syntax; +using ICSharpCode.Decompiler.IL; +using ICSharpCode.Decompiler.Metadata; +using ICSharpCode.Decompiler.Semantics; +using ICSharpCode.Decompiler.Tests.TypeSystem; +using ICSharpCode.Decompiler.TypeSystem; + +using NUnit.Framework; + +namespace ICSharpCode.Decompiler.Tests.Semantics +{ + // Sample hierarchy decompiled by ThisResolveResultTests: every way of naming the current + // instance (unqualified member, explicit 'this', 'base', a struct's by-ref 'this') has to + // end up annotated with the same 'this' parameter of the method being decompiled. + internal class ThisReferenceBase + { + public virtual int Get() => 0; + } + + internal class ThisReferenceSample : ThisReferenceBase + { + public int value; + + public ThisReferenceSample(int value) + { + // The parameter shadows the field, so this access keeps its qualifier. + this.value = value; + } + + public int Unqualified() => value; + + public override int Get() => base.Get(); + } + + internal struct ThisReferenceStructSample + { + public int value; + + public int Unqualified() => value; + } + + [TestFixture] + public class ThisResolveResultTests + { + // All samples live in this test assembly, so a single decompiler (and the PE file + // shared with the other fixtures) serves every test here. CSharpDecompiler is not + // safe for concurrent decompilations, so this fixture must stay non-parallelizable; + // marking it [Parallelizable] requires a decompiler per test. + static readonly Lazy decompiler = new Lazy( + delegate { + var module = TypeSystemLoaderTests.TestAssembly; + var resolver = new UniversalAssemblyResolver(module.FileName, false, module.Metadata.DetectTargetFrameworkId()); + return new CSharpDecompiler(module, resolver, new DecompilerSettings()); + }); + + static SyntaxTree Decompile(System.Type type) + { + return decompiler.Value.DecompileType(new FullTypeName(type.FullName)); + } + + static ILVariable AssertIsThisParameter(ResolveResult rr, string what) + { + Assert.That(rr, Is.InstanceOf(), what); + var variable = (rr as ILVariableResolveResult)?.Variable; + Assert.That(variable, Is.Not.Null, what); + Assert.That(variable.IsThis(), Is.True, what); + return variable; + } + + static ResolveResult ResolveResultOf(SyntaxTree tree, string methodName, System.Func select) + { + var method = tree.Descendants.OfType().Single(m => m.Name == methodName); + return select(method).GetResolveResult(); + } + + [Test] + public void ExplicitThisIsTheThisParameter() + { + var tree = Decompile(typeof(ThisReferenceSample)); + var ctor = tree.Descendants.OfType().Single(); + var thisRef = ctor.Descendants.OfType().Single(); + var variable = AssertIsThisParameter(thisRef.GetResolveResult(), "this"); + Assert.That(variable.Type.FullName, Is.EqualTo(typeof(ThisReferenceSample).FullName)); + } + + [Test] + public void UnqualifiedFieldAccessTargetsTheThisParameter() + { + var tree = Decompile(typeof(ThisReferenceSample)); + var rr = ResolveResultOf(tree, "Unqualified", m => m.Descendants.OfType().Single(id => id.Identifier == "value")); + var mrr = rr as MemberResolveResult; + Assert.That(mrr, Is.Not.Null, "unqualified field access"); + AssertIsThisParameter(mrr.TargetResult, "target of unqualified field access"); + } + + [Test] + public void BaseReferenceIsTheThisParameterWithTheBaseType() + { + var tree = Decompile(typeof(ThisReferenceSample)); + var rr = ResolveResultOf(tree, "Get", m => m.Descendants.OfType().Single()); + var variable = AssertIsThisParameter(rr, "base"); + Assert.That(rr.Type.FullName, Is.EqualTo(typeof(ThisReferenceBase).FullName)); + Assert.That(variable.Type.FullName, Is.EqualTo(typeof(ThisReferenceSample).FullName)); + Assert.That(((ThisResolveResult)rr).CausesNonVirtualInvocation, Is.True); + } + + [Test] + public void StructUnqualifiedFieldAccessTargetsTheByRefThisParameter() + { + var tree = Decompile(typeof(ThisReferenceStructSample)); + var rr = ResolveResultOf(tree, "Unqualified", m => m.Descendants.OfType().Single(id => id.Identifier == "value")); + var mrr = rr as MemberResolveResult; + Assert.That(mrr, Is.Not.Null, "unqualified field access"); + var variable = AssertIsThisParameter(mrr.TargetResult, "target of unqualified field access"); + Assert.That(variable.Type, Is.InstanceOf()); + Assert.That(mrr.TargetResult.Type.Kind, Is.Not.EqualTo(TypeKind.ByReference), "the reference is spelled as the struct itself"); + } + } +} diff --git a/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs b/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs index 231c00890..62c77cc7b 100644 --- a/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs +++ b/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs @@ -423,8 +423,11 @@ namespace ICSharpCode.Decompiler.CSharp } } - if (mrr == null) + if (mrr == null || !requireTarget) { + // The resolver looked the unqualified name up against the current type, so its + // result does not carry the translated target; annotate the same this/base or + // type target the qualified spelling gets. mrr = new MemberResolveResult(target.ResolveResult, field); } @@ -2831,13 +2834,13 @@ namespace ICSharpCode.Decompiler.CSharp // Additionally check target for null, in order to avoid a crash. if (!memberStatic && target != null) { - if (ShouldUseBaseReference()) + if (ShouldUseBaseReference(out var baseThisVariable)) { var baseReferenceType = resolver.CurrentTypeDefinition.DirectBaseTypes .FirstOrDefault(t => t.Kind != TypeKind.Interface); return new BaseReferenceExpression() .WithILInstruction(target) - .WithRR(new ThisResolveResult(baseReferenceType ?? memberDeclaringType, nonVirtualInvocation)); + .WithRR(new ThisResolveResult(baseThisVariable, baseReferenceType ?? memberDeclaringType, nonVirtualInvocation)); } else { @@ -2881,11 +2884,11 @@ namespace ICSharpCode.Decompiler.CSharp .WithoutILInstruction(); } translatedTarget = EnsureTargetNotNullable(translatedTarget, target); - if (translatedTarget.Expression is ThisReferenceExpression) + if (translatedTarget.Expression is ThisReferenceExpression + && translatedTarget.ResolveResult is ILVariableResolveResult { Variable: var thisVariable }) { // Give an explicit `this` the same resolve result the base-reference branch - // above gives `base`, and that the resolver gives the unqualified spelling of - // the same access. ConvertVariable annotates it as an ordinary local, so + // above gives `base`. ConvertVariable annotates it as an ordinary local, so // without this a consumer asking "does this expression reach instance state" // gets a different answer depending on whether the qualifier happened to be // printed - and the qualifier is printed for reasons (a parameter of the same @@ -2893,7 +2896,7 @@ namespace ICSharpCode.Decompiler.CSharp // question being asked. translatedTarget = new ThisReferenceExpression() .WithILInstruction(target) - .WithRR(new ThisResolveResult(translatedTarget.Type, nonVirtualInvocation)); + .WithRR(new ThisResolveResult(thisVariable, translatedTarget.Type, nonVirtualInvocation)); } return translatedTarget; } @@ -2905,21 +2908,22 @@ namespace ICSharpCode.Decompiler.CSharp .WithRR(new TypeResolveResult(constrainedTo ?? memberDeclaringType)); } - bool ShouldUseBaseReference() + bool ShouldUseBaseReference([NotNullWhen(true)] out ILVariable? thisVariable) { + thisVariable = null; if (!nonVirtualInvocation) return false; - if (!MatchLdThis(target)) + if (!MatchLdThis(target, out thisVariable)) return false; if ((constrainedTo ?? memberDeclaringType).GetDefinition() == resolver.CurrentTypeDefinition) return false; return true; } - bool MatchLdThis(ILInstruction inst) + bool MatchLdThis(ILInstruction inst, [NotNullWhen(true)] out ILVariable? thisVariable) { // ldloc this - if (inst.MatchLdThis()) + if (inst.MatchLdThis(out thisVariable)) return true; if (resolver.CurrentTypeDefinition.Kind == TypeKind.Struct) { @@ -2930,7 +2934,7 @@ namespace ICSharpCode.Decompiler.CSharp return false; if (!type.Equals(type2) || !type.Equals(resolver.CurrentTypeDefinition)) return false; - return arg2.MatchLdThis(); + return arg2.MatchLdThis(out thisVariable); } return false; } diff --git a/ICSharpCode.Decompiler/CSharp/Resolver/CSharpResolver.cs b/ICSharpCode.Decompiler/CSharp/Resolver/CSharpResolver.cs index b54f309d6..881113495 100644 --- a/ICSharpCode.Decompiler/CSharp/Resolver/CSharpResolver.cs +++ b/ICSharpCode.Decompiler/CSharp/Resolver/CSharpResolver.cs @@ -1640,8 +1640,15 @@ namespace ICSharpCode.Decompiler.CSharp.Resolver ResolveResult r; if (lookupMode == NameLookupMode.Expression || lookupMode == NameLookupMode.InvocationTarget) { - var targetResolveResult = (t == this.CurrentTypeDefinition ? ResolveThisReference() : new TypeResolveResult(t)); - r = lookup.Lookup(targetResolveResult, identifier, typeArguments, lookupMode == NameLookupMode.InvocationTarget); + // A ThisResolveResult names the 'this' parameter of an ILFunction, which the + // resolver does not know; the lookup on the current type does not need one, + // as it accepts protected members of the current type on its own. The type + // is self-parameterized so that its members come out specialized, matching + // the members that references from inside the type resolve to. + IType targetType = t == this.CurrentTypeDefinition && t.TypeParameterCount != 0 + ? new ParameterizedType(t, t.TypeParameters) + : t; + r = lookup.Lookup(new TypeResolveResult(targetType), identifier, typeArguments, lookupMode == NameLookupMode.InvocationTarget); } else { @@ -2621,48 +2628,6 @@ namespace ICSharpCode.Decompiler.CSharp.Resolver } #endregion - #region Resolve This/Base Reference - /// - /// Resolves 'this'. - /// - public ResolveResult ResolveThisReference() - { - ITypeDefinition t = CurrentTypeDefinition; - if (t != null) - { - if (t.TypeParameterCount != 0) - { - // Self-parameterize the type - return new ThisResolveResult(new ParameterizedType(t, t.TypeParameters)); - } - else - { - return new ThisResolveResult(t); - } - } - return ErrorResult; - } - - /// - /// Resolves 'base'. - /// - public ResolveResult ResolveBaseReference() - { - ITypeDefinition t = CurrentTypeDefinition; - if (t != null) - { - foreach (IType baseType in t.DirectBaseTypes) - { - if (baseType.Kind != TypeKind.Unknown && baseType.Kind != TypeKind.Interface) - { - return new ThisResolveResult(baseType, causesNonVirtualInvocation: true); - } - } - } - return ErrorResult; - } - #endregion - #region ResolveConditional /// /// Converts the input to bool using the rules for boolean expressions. diff --git a/ICSharpCode.Decompiler/IL/Instructions/PatternMatching.cs b/ICSharpCode.Decompiler/IL/Instructions/PatternMatching.cs index 97bfc5af7..6a0390bad 100644 --- a/ICSharpCode.Decompiler/IL/Instructions/PatternMatching.cs +++ b/ICSharpCode.Decompiler/IL/Instructions/PatternMatching.cs @@ -116,8 +116,18 @@ namespace ICSharpCode.Decompiler.IL public bool MatchLdThis() { - var inst = this as LdLoc; - return inst != null && inst.Variable.Kind == VariableKind.Parameter && inst.Variable.Index < 0; + return MatchLdThis(out _); + } + + public bool MatchLdThis([NotNullWhen(true)] out ILVariable? variable) + { + if (this is LdLoc inst && inst.Variable.IsThis()) + { + variable = inst.Variable; + return true; + } + variable = null; + return false; } public bool MatchStLoc([NotNullWhen(true)] out ILVariable? variable) diff --git a/ICSharpCode.Decompiler/Semantics/ThisResolveResult.cs b/ICSharpCode.Decompiler/Semantics/ThisResolveResult.cs index 4aee03032..2afe6206a 100644 --- a/ICSharpCode.Decompiler/Semantics/ThisResolveResult.cs +++ b/ICSharpCode.Decompiler/Semantics/ThisResolveResult.cs @@ -16,6 +16,8 @@ // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER // DEALINGS IN THE SOFTWARE. +using ICSharpCode.Decompiler.CSharp; +using ICSharpCode.Decompiler.IL; using ICSharpCode.Decompiler.TypeSystem; namespace ICSharpCode.Decompiler.Semantics @@ -23,12 +25,15 @@ namespace ICSharpCode.Decompiler.Semantics /// /// Represents the 'this' reference. /// Also used for the 'base' reference. + /// Both read the 'this' parameter of the current function, so this is also the + /// of that variable. The type is the one the + /// reference is spelled with: 'base' carries the base type, not the variable's type. /// - public class ThisResolveResult : ResolveResult + public class ThisResolveResult : ILVariableResolveResult { bool causesNonVirtualInvocation; - public ThisResolveResult(IType type, bool causesNonVirtualInvocation = false) : base(type) + public ThisResolveResult(ILVariable thisVariable, IType type, bool causesNonVirtualInvocation = false) : base(thisVariable, type) { this.causesNonVirtualInvocation = causesNonVirtualInvocation; }