Decide backing-field references by one verdict, not three

Three places decided independently whether a backing field would still be
declared, and they disagreed. ReplaceBackingFieldUsage rewrote a constructor
store into a property assignment whenever the property looked collapsible,
without asking whether PatternStatementTransform would actually remove the
declaration; ConvertField printed the "field" keyword on the FieldKeyword
setting alone, ignoring the GetterOnlyAutomaticProperties veto that
MemberIsHidden applies to the same field.

Two consequences, both silent. A setter-less property under
GetterOnlyAutomaticProperties = false kept its declaration and got "field" in
the getter anyway, so the keyword bound to a second synthesized field and the
declared one went unwritten. A settable property under AutomaticProperties =
false had its initializing store turned into a property assignment that
TransformFieldAndConstructorInitializers could no longer lift, leaving the
constructor in the output with an unconverted base-constructor call.

BackingFieldWillBeRemoved is now the single verdict every branch consults, and
it mirrors the transform's own entry gate. A property that keeps explicit
accessors is no longer addressed by name: the store stays a field reference and
becomes the property initializer, which is what field-backed storage means. The
one exception is a setter-less property's constructor store, which C# allows to
be written as an assignment and which has no other expressible form.

IsBackingFieldOfAutomaticProperty now answers through TryGetBackingField instead
of its own name check, so the two directions of "is this field that property's
storage" cannot diverge on staticness or field type. ReplaceBackingFieldUsage
dispatches on the resolve result rather than the identifier's spelling; after
ConvertField the same field appears both as "field" and under its metadata name
carrying the same annotation, so matching the name was matching the wrong thing.

The keyword's own precondition moves into a CanUseFieldKeyword local function.
Seven clauses with comment blocks wedged between them had to be read as one
expression; as one early return per rule the list reads in order.

Assisted-by: Claude:claude-opus-5[1m]:Claude Code
This commit is contained in:
Siegfried Pammer
2026-08-30 11:48:44 +02:00
parent bdf48e5a47
commit e423cfb9ca
8 changed files with 175 additions and 60 deletions
@@ -274,6 +274,8 @@
<None Include="TestCases\Ugly\NoFieldKeyword.Expected.cs" />
<Compile Remove="TestCases\Ugly\NoForEachStatement.Expected.cs" />
<None Include="TestCases\Ugly\NoForEachStatement.Expected.cs" />
<Compile Remove="TestCases\Ugly\NoGetterOnlyAutomaticProperties.Expected.cs" />
<None Include="TestCases\Ugly\NoGetterOnlyAutomaticProperties.Expected.cs" />
<Compile Remove="TestCases\Ugly\NoInitAccessors.Expected.cs" />
<None Include="TestCases\Ugly\NoInitAccessors.Expected.cs" />
<Compile Remove="TestCases\Ugly\NoLocalFunctions.Expected.cs" />
@@ -16,6 +16,17 @@ internal class NoAutomaticProperties
}
}
public int WithInitializer {
[CompilerGenerated]
get {
return field;
}
[CompilerGenerated]
set {
field = value;
}
} = 5;
public int SemiAuto {
get {
return field;
@@ -24,6 +24,8 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Ugly
{
public int Plain { get; set; }
public int WithInitializer { get; set; } = 5;
public int SemiAuto {
get {
return field;
@@ -0,0 +1,29 @@
#if !OPT
using System.Diagnostics;
#endif
using System.Runtime.CompilerServices;
namespace ICSharpCode.Decompiler.Tests.TestCases.Ugly;
internal class NoGetterOnlyAutomaticProperties
{
[CompilerGenerated]
#if !OPT
[DebuggerBrowsable(DebuggerBrowsableState.Never)]
#endif
private readonly int GetOnly__BackingField;
public int GetOnly {
[CompilerGenerated]
get {
return GetOnly__BackingField;
}
}
public int WithSetter { get; set; }
public NoGetterOnlyAutomaticProperties()
{
GetOnly__BackingField = 5;
}
}
@@ -0,0 +1,32 @@
// Copyright (c) 2026 Siegfried Pammer
//
// 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.
namespace ICSharpCode.Decompiler.Tests.TestCases.Ugly
{
internal class NoGetterOnlyAutomaticProperties
{
public int GetOnly { get; }
public int WithSetter { get; set; }
public NoGetterOnlyAutomaticProperties()
{
GetOnly = 5;
}
}
}
@@ -136,6 +136,14 @@ namespace ICSharpCode.Decompiler.Tests
});
}
[Test]
public async Task NoGetterOnlyAutomaticProperties([ValueSource(nameof(roslynLatestOnlyOptions))] CompilerOptions cscOptions)
{
await RunForLibrary(cscOptions: cscOptions, decompilerSettings: new DecompilerSettings {
GetterOnlyAutomaticProperties = false
});
}
[Test]
public async Task NoFieldKeyword([ValueSource(nameof(roslynLatestOnlyOptions))] CompilerOptions cscOptions)
{
@@ -331,25 +331,10 @@ namespace ICSharpCode.Decompiler.CSharp
return eventReference.WithRR(eventResolveResult);
}
if (settings.FieldKeyword
&& decompilationContext.CurrentMember is IProperty accessedProperty
&& accessedProperty.Parameters.Count == 0
// Ask exactly the question PatternStatementTransform asks when it decides whether
// the field declaration can go away. A looser test here prints `field` inside a
// property whose declaration then keeps explicit accessors and its field: on
// recompile the keyword binds to a freshly synthesized backing field while the
// original one stays declared and unwritten - silently different storage.
&& PatternStatementTransform.TryGetBackingField(accessedProperty, out var backingField)
&& field.MemberDefinition.Equals(backingField.MemberDefinition)
// Only THIS instance's field is the `field` keyword. IL can load another
// instance's backing field inside an accessor (weavers, obfuscators, hand-written
// IL); rendering that as `field` would redirect the access, and drop whatever
// side effect producing the target had.
&& (field.IsStatic || TargetIsThis(targetInstruction)))
if (CanUseFieldKeyword())
{
// Inside its own property's get/set/init accessor (including nested lambdas and
// local functions), the backing field is the C# 14 "field" keyword. It must stay
// unqualified: "this.field" would refer to a real member named "field".
// The keyword must stay unqualified: "this.field" would refer to a real member
// named "field".
return new IdentifierExpression("field")
.WithRR(new MemberResolveResult(null, field));
}
@@ -441,6 +426,38 @@ namespace ICSharpCode.Decompiler.CSharp
}
return expr;
// Whether this access may be rendered as the C# 14 "field" keyword: it has to be the
// backing field of the property whose accessor is being decompiled, read off this
// instance, in a property the declaration can actually disappear from. Nested lambdas
// and local functions inside the accessor count as being inside it.
bool CanUseFieldKeyword()
{
if (!settings.FieldKeyword)
return false;
if (decompilationContext.CurrentMember is not IProperty property || property.Parameters.Count != 0)
return false;
// With GetterOnlyAutomaticProperties off, a setter-less property keeps its backing
// field declared (CSharpDecompiler.MemberIsHidden) and PatternStatementTransform
// leaves the property alone, so the keyword would land next to the declaration it
// is supposed to replace.
if (!property.CanSet && !settings.GetterOnlyAutomaticProperties)
return false;
// Exactly the question PatternStatementTransform asks before removing the
// declaration. A looser test prints "field" in a property that then keeps its
// field: on recompile the keyword binds to a freshly synthesized backing field
// while the original stays declared and unwritten - silently different storage.
if (!PatternStatementTransform.TryGetBackingField(property, out var backingField)
|| !field.MemberDefinition.Equals(backingField.MemberDefinition))
{
return false;
}
// Only THIS instance's field is the keyword. IL can load another instance's backing
// field inside an accessor (weavers, obfuscators, hand-written IL); rendering that
// as "field" would redirect the access and drop whatever side effect produced the
// target.
return field.IsStatic || TargetIsThis(targetInstruction);
}
}
// References to an automatic event's backing field are printed as the event. Gated on
@@ -1089,12 +1089,20 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
property = null;
if (!NameCouldBeBackingFieldOfAutomaticProperty(field.Name, out var propertyName))
return false;
if (!field.IsCompilerGenerated())
return false;
property = field.DeclaringTypeDefinition?
var candidate = field.DeclaringTypeDefinition?
.GetProperties(p => p.Name == propertyName, GetMemberOptions.IgnoreInheritedMembers)
.FirstOrDefault();
return property != null;
// Answering through TryGetBackingField keeps the two directions of the same question
// from disagreeing: it is the predicate every transform consults before removing a
// declaration, and it checks more than the name (compiler-generated, staticness and
// field type all have to match the property).
if (candidate == null || !TryGetBackingField(candidate, out var backingField)
|| !field.MemberDefinition.Equals(backingField.MemberDefinition))
{
return false;
}
property = candidate;
return true;
}
/// <summary>
@@ -1119,45 +1127,47 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
Identifier? ReplaceBackingFieldUsage(Identifier identifier)
{
if (NameCouldBeBackingFieldOfAutomaticProperty(identifier.Name, out _))
// The resolve result, not the spelling, identifies the member: after
// ExpressionBuilder.ConvertField the same backing field appears both as the C# 14 "field"
// keyword and under its metadata name, carrying the same annotation either way.
var parent = identifier.Parent;
if (parent == null)
return null;
var mrr = parent.Annotation<MemberResolveResult>();
if (mrr?.Member is not IField field || !IsBackingFieldOfAutomaticProperty(field, out var property)
|| currentMethod?.AccessorOwner == property)
{
var parent = identifier.Parent;
if (parent == null)
return null;
var mrr = parent.Annotation<MemberResolveResult>();
if (mrr?.Member is IField field && IsBackingFieldOfAutomaticProperty(field, out var property)
&& currentMethod?.AccessorOwner != property)
{
if (CanTransformToAutomaticProperty(property, !(field.IsCompilerGenerated() && field.Name == "_" + property.Name)))
{
if (!property.CanSet && !context.Settings.GetterOnlyAutomaticProperties && !context.Settings.FieldKeyword)
return null;
}
else if (context.Settings.FieldKeyword && !property.CanSet && IsConstructorStoreTarget(parent, field)
&& BackingFieldWillBeRemoved(property, field, parent))
{
// A direct store to the backing field of a setter-less field-backed
// property is expressible as a property assignment in a constructor -
// but only where the property declaration actually becomes field-backed.
// If TransformFieldBackedProperty bails, the property keeps explicit
// accessors and no setter, so assigning it would not compile (CS0200).
}
else
{
// Stores that initialize a field-backed property with a setter are left
// as field references and lifted into the property initializer by
// TransformFieldAndConstructorInitializers (a property assignment would
// invoke the setter); everything else is inexpressible with the "field"
// keyword and keeps the field declared.
return null;
}
context.Step("Replace backing field use with property", identifier);
parent.RemoveAnnotations<MemberResolveResult>();
parent.AddAnnotation(new MemberResolveResult(mrr.TargetResult, property));
return Identifier.Create(property.Name);
}
return null;
}
return null;
// With the keyword available TransformAutomaticProperty routes every property through
// TransformFieldBackedProperty, so its verdict on the declaration is the only one that
// counts: while the field stays declared, a field reference remains the correct - and
// only compilable - rendering.
if (context.Settings.FieldKeyword && !BackingFieldWillBeRemoved(property, field, parent))
return null;
if (context.Settings.AutomaticProperties
&& CanTransformToAutomaticProperty(property, !(field.IsCompilerGenerated() && field.Name == "_" + property.Name)))
{
if (!property.CanSet && !context.Settings.GetterOnlyAutomaticProperties)
return null;
}
else if (context.Settings.FieldKeyword && !property.CanSet && IsConstructorStoreTarget(parent, field))
{
// A setter-less field-backed property keeping explicit accessors is still assignable
// by name inside a constructor of its declaring type, and that is the only form the
// store has left once the declaration is gone.
}
else
{
// The property keeps explicit accessors and a setter, so its name would invoke that
// setter. The store stays a field reference and TransformFieldAndConstructorInitializers
// lifts it into the property initializer, which is what field-backed storage means.
return null;
}
context.Step("Replace backing field use with property", identifier);
parent.RemoveAnnotations<MemberResolveResult>();
parent.AddAnnotation(new MemberResolveResult(mrr.TargetResult, property));
return Identifier.Create(property.Name);
}
bool IsConstructorStoreTarget(AstNode node, IField field)
@@ -1170,7 +1180,11 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
/// </summary>
bool BackingFieldWillBeRemoved(IProperty property, IField field, AstNode nodeInTree)
{
return TryGetBackingField(property, out var backingField)
return context.Settings.FieldKeyword
// The same gate VisitPropertyDeclaration applies: a setter-less property is only
// transformed where getter-only auto-properties are allowed.
&& (property.CanSet || context.Settings.GetterOnlyAutomaticProperties)
&& TryGetBackingField(property, out var backingField)
&& field.MemberDefinition.Equals(backingField.MemberDefinition)
&& OutsideReferencesAreExpressible(nodeInTree, backingField);
}