From 461df8857275b57fcc2002529fd6f5a58c90be40 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Sat, 18 Jul 2026 10:40:57 +0200 Subject: [PATCH] Replace automatic-event backing-field references during translation PatternStatementTransform renamed backing-field identifiers to the event after the fact, keyed on the metadata name association alone: references belonging to an event that is not actually automatic were still renamed, binding them to a custom event that is unusable as a value (the same defect class as #3858), and the rename bypassed the resolver checks, so qualifiers were computed for the hidden field instead of the printed event. ExpressionBuilder.ConvertField now performs the substitution, keyed on the AutoEventDecompiler verdict whose memo moves into DecompileRun so that member hiding, the event declaration, and reference translation all decide from one analysis. Checking the verdict's field identity also keeps same-typed sibling events apart (#3575), and the qualifier logic running against the event drops spurious this./type qualifiers from raise sites. mcs 2.x accesses a sibling automatic event's backing field directly inside custom accessors instead of calling the accessor, so the fixture expects the resulting Delegate.Combine form there. Assisted-by: Claude:claude-fable-5:Claude Code Claude-Session: https://claude.ai/code/session_01Btdypgm8utyxqt1Etn2BDi --- .../TestCases/Pretty/AutomaticEvents.cs | 36 +++++++++++++++- .../Pretty/ConstructorInitializers.cs | 2 +- .../CSharp/AutoEventDecompiler.cs | 16 ++++++- .../CSharp/CSharpDecompiler.cs | 22 +--------- .../CSharp/ExpressionBuilder.cs | 43 +++++++++++++++++++ .../Transforms/PatternStatementTransform.cs | 32 -------------- ICSharpCode.Decompiler/DecompileRun.cs | 9 ++++ 7 files changed, 105 insertions(+), 55 deletions(-) diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/AutomaticEvents.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/AutomaticEvents.cs index 37cf49524..7c4adc860 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/AutomaticEvents.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/AutomaticEvents.cs @@ -15,6 +15,33 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty } } } + public class EventNameCollision + { + private event Action ChangedEvent; + public event Action Changed { + add { +#if MCS2 + ChangedEvent = (Action)Delegate.Combine(ChangedEvent, value); +#else + ChangedEvent += value; +#endif + } + remove { +#if MCS2 + ChangedEvent = (Action)Delegate.Remove(ChangedEvent, value); +#else + ChangedEvent -= value; +#endif + } + } + public void RaiseChanged() + { + if (ChangedEvent != null) + { + ChangedEvent(); + } + } + } public class GenericAutomaticEvents where T : EventArgs { public event EventHandler GenericEvent; @@ -30,7 +57,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty public static event EventHandler StaticEventWithHandler; public static void RaiseStaticEvent() { - StaticAutomaticEvents.StaticEvent(); + StaticEvent(); } } public class UnrecognizedCustomEvents @@ -44,5 +71,12 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty Console.WriteLine("remove"); } } + public void RaiseUnused() + { + if (UnusedEvent != null) + { + UnusedEvent(); + } + } } } diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/ConstructorInitializers.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/ConstructorInitializers.cs index 094bd0731..6d2d4041a 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/ConstructorInitializers.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/ConstructorInitializers.cs @@ -242,7 +242,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty public void Print() { - Console.WriteLine(this.A); + Console.WriteLine(A); } } #endif diff --git a/ICSharpCode.Decompiler/CSharp/AutoEventDecompiler.cs b/ICSharpCode.Decompiler/CSharp/AutoEventDecompiler.cs index ad9802304..2170e503c 100644 --- a/ICSharpCode.Decompiler/CSharp/AutoEventDecompiler.cs +++ b/ICSharpCode.Decompiler/CSharp/AutoEventDecompiler.cs @@ -40,7 +40,21 @@ namespace ICSharpCode.Decompiler.CSharp /// static class AutoEventDecompiler { - public static bool IsAutomaticEvent(IDecompilerTypeSystem typeSystem, IEvent ev, CancellationToken cancellationToken, [NotNullWhen(true)] out IField? backingField) + /// + /// Determines whether is an automatic event, memoizing the verdict + /// in so that all consumers decide from the same analysis. + /// + public static bool IsAutomaticEvent(IDecompilerTypeSystem typeSystem, IEvent ev, DecompileRun decompileRun, CancellationToken cancellationToken, [NotNullWhen(true)] out IField? backingField) + { + if (!decompileRun.AutomaticEvents.TryGetValue(ev, out backingField)) + { + backingField = IsAutomaticEvent(typeSystem, ev, cancellationToken, out var field) ? field : null; + decompileRun.AutomaticEvents.Add(ev, backingField); + } + return backingField != null; + } + + static bool IsAutomaticEvent(IDecompilerTypeSystem typeSystem, IEvent ev, CancellationToken cancellationToken, [NotNullWhen(true)] out IField? backingField) { backingField = null; if (ev.IsExplicitInterfaceImplementation || ev.DeclaringTypeDefinition == null) diff --git a/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs b/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs index c29ad1800..947e4d5a9 100644 --- a/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs +++ b/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs @@ -64,10 +64,6 @@ namespace ICSharpCode.Decompiler.CSharp readonly MetadataModule module; readonly MetadataReader metadata; readonly DecompilerSettings settings; - // Memoized AutoEventDecompiler verdicts (null = event is not automatic). The verdict is - // consumed both when the backing field is considered for hiding and when the event - // declaration is built; memoizing it guarantees the two decisions agree. - readonly Dictionary automaticEvents = new Dictionary(); SyntaxTree? syntaxTree; List ilTransforms = GetILTransforms(); @@ -1837,7 +1833,7 @@ namespace ICSharpCode.Decompiler.CSharp return false; if (!module.MetadataFile.PropertyAndEventBackingFieldLookup.IsEventBackingField((FieldDefinitionHandle)field.MetadataToken, out var eventHandle)) return false; - if (IsAutomaticEvent(module.GetDefinition(eventHandle), out _)) + if (AutoEventDecompiler.IsAutomaticEvent(typeSystem, module.GetDefinition(eventHandle), decompileRun, CancellationToken, out _)) return false; // The field may be hidden for an unrelated reason as well; keep it hidden then. var settingsWithoutAutomaticEvents = settings.Clone(); @@ -2530,20 +2526,6 @@ namespace ICSharpCode.Decompiler.CSharp } } - /// - /// Determines whether is an automatic event (see - /// ), memoizing the verdict per event. - /// - bool IsAutomaticEvent(IEvent ev, [NotNullWhen(true)] out IField? backingField) - { - if (!automaticEvents.TryGetValue(ev, out backingField)) - { - backingField = AutoEventDecompiler.IsAutomaticEvent(typeSystem, ev, CancellationToken, out var field) ? field : null; - automaticEvents.Add(ev, backingField); - } - return backingField != null; - } - EntityDeclaration DoDecompile(IEvent ev, DecompileRun decompileRun, ITypeResolveContext decompilationContext) { Debug.Assert(decompilationContext.CurrentMember == ev); @@ -2555,7 +2537,7 @@ namespace ICSharpCode.Decompiler.CSharp var typeSystemAstBuilder = CreateAstBuilder(decompileRun.Settings); IField? backingField = null; bool isAutomaticEvent = adderHasBody && removerHasBody && decompileRun.Settings.AutomaticEvents - && IsAutomaticEvent(ev, out backingField); + && AutoEventDecompiler.IsAutomaticEvent(typeSystem, ev, decompileRun, CancellationToken, out backingField); // A recognized automatic event is built in field-like form directly; its // compiler-generated accessor bodies are never decompiled. typeSystemAstBuilder.UseCustomEvents = !isAutomaticEvent diff --git a/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs b/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs index 661243796..398181075 100644 --- a/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs +++ b/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs @@ -20,6 +20,7 @@ using System; using System.Collections.Generic; using System.Collections.Immutable; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Reflection.Metadata; using System.Threading; @@ -300,6 +301,22 @@ namespace ICSharpCode.Decompiler.CSharp ExpressionWithResolveResult ConvertField(IField field, ILInstruction? targetInstruction = null) { + if (settings.AutomaticEvents && IsBackingFieldOfAutomaticEvent(field, out var ev)) + { + // The field-like event hides its backing field, so the reference is printed as + // the event; inside the declaring type that denotes the backing field. + var eventTarget = TranslateTarget(targetInstruction, + nonVirtualInvocation: true, + memberStatic: ev.IsStatic, + memberDeclaringType: ev.DeclaringType); + bool requireEventTarget = RequiresQualifier(ev, eventTarget); + var eventResolveResult = new MemberResolveResult(eventTarget.ResolveResult, ev); + Expression eventReference = requireEventTarget + ? new MemberReferenceExpression(eventTarget, ev.Name) + : new IdentifierExpression(ev.Name); + return eventReference.WithRR(eventResolveResult); + } + var target = TranslateTarget(targetInstruction, nonVirtualInvocation: true, memberStatic: field.IsStatic, @@ -379,6 +396,32 @@ namespace ICSharpCode.Decompiler.CSharp return expr; } + // References to an automatic event's backing field are printed as the event. Gated on + // the AutoEventDecompiler verdict: a custom event is not usable as a value (#3858), and + // the field-identity check keeps same-typed sibling events apart (#3575). Within the + // event's own accessors the field is printed as-is. + bool IsBackingFieldOfAutomaticEvent(IField field, [NotNullWhen(true)] out IEvent? ev) + { + ev = null; + if (field.MetadataToken.IsNil || field.ParentModule is not MetadataModule module) + return false; + if (!module.MetadataFile.PropertyAndEventBackingFieldLookup.IsEventBackingField((FieldDefinitionHandle)field.MetadataToken, out var eventHandle)) + return false; + ev = module.GetDefinition(eventHandle); + if (decompilationContext.CurrentMember is IMethod { AccessorOwner: IEvent owner } && owner.Equals(ev)) + { + ev = null; + return false; + } + if (!AutoEventDecompiler.IsAutomaticEvent(typeSystem, ev, statementBuilder.decompileRun, cancellationToken, out var backingField) + || !backingField.Equals(field.MemberDefinition)) + { + ev = null; + return false; + } + return true; + } + TranslatedExpression IsType(IsInst inst) { var arg = Translate(inst.Argument); diff --git a/ICSharpCode.Decompiler/CSharp/Transforms/PatternStatementTransform.cs b/ICSharpCode.Decompiler/CSharp/Transforms/PatternStatementTransform.cs index 91f1ec1fd..2f868b6d4 100644 --- a/ICSharpCode.Decompiler/CSharp/Transforms/PatternStatementTransform.cs +++ b/ICSharpCode.Decompiler/CSharp/Transforms/PatternStatementTransform.cs @@ -862,12 +862,6 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms return newIdentifier; } } - if (context.Settings.AutomaticEvents) - { - var newIdentifier = ReplaceEventFieldAnnotation(identifier); - if (newIdentifier != null) - return newIdentifier; - } return base.VisitIdentifier(identifier); } @@ -927,32 +921,6 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms return null; } - Identifier? ReplaceEventFieldAnnotation(Identifier identifier) - { - var parent = identifier.Parent; - if (parent == null) - return null; - var mrr = parent.Annotation(); - if (mrr?.Member is not IField field || field.Accessibility != Accessibility.Private) - return null; - var module = field.ParentModule as MetadataModule; - if (module == null) - return null; - if (module.MetadataFile.PropertyAndEventBackingFieldLookup.IsEventBackingField((FieldDefinitionHandle)field.MetadataToken, out var eventHandle)) - { - var eventDef = module.ResolveEntity(eventHandle) as IEvent; - if (eventDef != null && currentMethod?.AccessorOwner != eventDef) - { - context.Step("Replace event backing field use with event", identifier); - parent.RemoveAnnotations(); - parent.AddAnnotation(new MemberResolveResult(mrr.TargetResult, eventDef)); - identifier.Name = eventDef.Name; - return identifier; - } - } - return null; - } - #region Automatic Events static readonly Expression fieldReferencePattern = new Choice { new IdentifierExpression(Pattern.AnyString), diff --git a/ICSharpCode.Decompiler/DecompileRun.cs b/ICSharpCode.Decompiler/DecompileRun.cs index e9cccf7d0..10cb42fda 100644 --- a/ICSharpCode.Decompiler/DecompileRun.cs +++ b/ICSharpCode.Decompiler/DecompileRun.cs @@ -35,6 +35,15 @@ namespace ICSharpCode.Decompiler public IDocumentationProvider DocumentationProvider { get; set; } public Dictionary RecordDecompilers { get; } = new Dictionary(); +#nullable enable + /// + /// Memoized AutoEventDecompiler verdicts (null = the event is not automatic). Shared so + /// that every consumer of the verdict (member hiding, the event declaration, reference + /// translation) decides from the same analysis. + /// + public Dictionary AutomaticEvents { get; } = new Dictionary(); +#nullable restore + public Dictionary TypeHierarchyIsKnown { get; } = new(); public CSharp.TypeSystem.UsingScope UsingScope { get; }