Browse Source

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
pull/3889/head
Siegfried Pammer 2 months ago committed by Siegfried Pammer
parent
commit
461df88572
  1. 36
      ICSharpCode.Decompiler.Tests/TestCases/Pretty/AutomaticEvents.cs
  2. 2
      ICSharpCode.Decompiler.Tests/TestCases/Pretty/ConstructorInitializers.cs
  3. 16
      ICSharpCode.Decompiler/CSharp/AutoEventDecompiler.cs
  4. 22
      ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs
  5. 43
      ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs
  6. 32
      ICSharpCode.Decompiler/CSharp/Transforms/PatternStatementTransform.cs
  7. 9
      ICSharpCode.Decompiler/DecompileRun.cs

36
ICSharpCode.Decompiler.Tests/TestCases/Pretty/AutomaticEvents.cs

@ -15,6 +15,33 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty @@ -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<T> where T : EventArgs
{
public event EventHandler<T> GenericEvent;
@ -30,7 +57,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty @@ -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 @@ -44,5 +71,12 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
Console.WriteLine("remove");
}
}
public void RaiseUnused()
{
if (UnusedEvent != null)
{
UnusedEvent();
}
}
}
}

2
ICSharpCode.Decompiler.Tests/TestCases/Pretty/ConstructorInitializers.cs

@ -242,7 +242,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty @@ -242,7 +242,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
public void Print()
{
Console.WriteLine(this.A);
Console.WriteLine(A);
}
}
#endif

16
ICSharpCode.Decompiler/CSharp/AutoEventDecompiler.cs

@ -40,7 +40,21 @@ namespace ICSharpCode.Decompiler.CSharp @@ -40,7 +40,21 @@ namespace ICSharpCode.Decompiler.CSharp
/// </summary>
static class AutoEventDecompiler
{
public static bool IsAutomaticEvent(IDecompilerTypeSystem typeSystem, IEvent ev, CancellationToken cancellationToken, [NotNullWhen(true)] out IField? backingField)
/// <summary>
/// Determines whether <paramref name="ev"/> is an automatic event, memoizing the verdict
/// in <paramref name="decompileRun"/> so that all consumers decide from the same analysis.
/// </summary>
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)

22
ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs

@ -64,10 +64,6 @@ namespace ICSharpCode.Decompiler.CSharp @@ -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<IEvent, IField?> automaticEvents = new Dictionary<IEvent, IField?>();
SyntaxTree? syntaxTree;
List<IILTransform> ilTransforms = GetILTransforms();
@ -1837,7 +1833,7 @@ namespace ICSharpCode.Decompiler.CSharp @@ -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 @@ -2530,20 +2526,6 @@ namespace ICSharpCode.Decompiler.CSharp
}
}
/// <summary>
/// Determines whether <paramref name="ev"/> is an automatic event (see
/// <see cref="AutoEventDecompiler"/>), memoizing the verdict per event.
/// </summary>
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 @@ -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

43
ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs

@ -20,6 +20,7 @@ using System; @@ -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 @@ -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 @@ -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);

32
ICSharpCode.Decompiler/CSharp/Transforms/PatternStatementTransform.cs

@ -862,12 +862,6 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms @@ -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 @@ -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<MemberResolveResult>();
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<MemberResolveResult>();
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),

9
ICSharpCode.Decompiler/DecompileRun.cs

@ -35,6 +35,15 @@ namespace ICSharpCode.Decompiler @@ -35,6 +35,15 @@ namespace ICSharpCode.Decompiler
public IDocumentationProvider DocumentationProvider { get; set; }
public Dictionary<ITypeDefinition, RecordDecompiler> RecordDecompilers { get; } = new Dictionary<ITypeDefinition, RecordDecompiler>();
#nullable enable
/// <summary>
/// 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.
/// </summary>
public Dictionary<IEvent, IField?> AutomaticEvents { get; } = new Dictionary<IEvent, IField?>();
#nullable restore
public Dictionary<ITypeDefinition, bool> TypeHierarchyIsKnown { get; } = new();
public CSharp.TypeSystem.UsingScope UsingScope { get; }

Loading…
Cancel
Save