Calling PrintMembers in a record containing a public static field/property of its own type causes a stack overflow

[syndek]

Version Used:
5.0.100-rc.1.20452.10

Steps to Reproduce:
Given any record which declares a public static field/property of its own type, for example:

record TestRecord
{
    public static readonly TestRecord StaticTestRecord = new TestRecord();
}

Attempting to in any way, directly or indirectly, call PrintMembers (for example, through something like ToString) causes a StackOverflowException to occur. This is potentially to be expected, as the static field/property of the type is a member of the record and will therefore be included in the call to PrintMembers, but it's also very frustrating behaviour. Patterns such as the null object pattern aren't easily achievable with records as a result, as any attempt to declare a static 'null' instance of the record within itself results in this behaviour. Debugging is also affected, as the debugger window displays instances of types using their ToString representations.

It is understandable that this would happen with instance members of the same type, but I don't feel it's necessary to include static members in calls to PrintMembers as a result.

Expected Behavior:
Ignore the field/property.

Actual Behavior:
System.StackOverflowException: 'Exception_WasThrown' when executed.

[svick]

I believe this is already fixed in master. The spec change is dotnet/csharplang#3919, and the compiler change #47868.

[Youssef1313]

@svick The issue title is misleading. This is another problem that's unrelated to the property/field being static, so #47868 doesn't fix it.

var r = new TestRecord();
System.Console.WriteLine(r.ToString());

record TestRecord
{
    public readonly TestRecord NonStaticTestRecord = new TestRecord();
}

The previous code will still cause Stackoverflow.

By a first look, one could say we should exclude fields/properties of the same record type. But that's not a complete fix too. Consider the following:

var r = new A();
System.Console.WriteLine(r.ToString());

record A
{
    public readonly B RecordB = new B();
}

record B
{
    public readonly A RecordA = new A();
}

This also generates stackoverflow.

Consider deep nesting too:

var r = new A();
System.Console.WriteLine(r.ToString());

record A
{
    public readonly B RecordB = new B();
}

record B
{
    public readonly C RecordC = new C();
}

record C
{
    public readonly D RecordD = new D();
}

record D
{
    public readonly A RecordA = new A();
}

[Youssef1313]

Should records be completely excluded from PrintMembers? or find a way to detect if it will cause a circular/infinite recursive calls?

[svick]

@Youssef1313 The initial post explicitly called out that this is not about instance members:

It is understandable that this would happen with instance members of the same type, […]

Also, LDM already considered this issue in the past, and decided that doing nothing is good enough:

We will not attempt any special handling here. We could consider adding a DebuggerDisplay with more special, reflective handling at a later point in time if this proves to be a pain point, but we don't think it's worth the cost in regular ToString calls.

[Youssef1313]

@svick Oh that is right. In that case this should be already solved.

[syndek]

Ultimately, the solution to actually 'fix' this problem is a way to tell the compiler to ignore certain members when generating the PrintMembers method.

What about an attribute? Something along the lines of:

[AttributeUsage(AttributeTargets.Field | AttributeTargets.Property)]
public class PrintMembersIgnoreAttribute : Attribute { }

Which could then be applied to a field/property in a record type like so:

record TestRecord
{
    [PrintMembersIgnore]
    public static readonly TestRecord StaticTestRecord = new TestRecord();
}

The member would then be ignored when the compiler generates the PrintMembers method, thus preventing this problem from occurring.

[Youssef1313]

@syndek Currently, the developer doesn't have a control on which fields/properties are excluded.

However, all static fields and properties are always excluded. So the issue is already fixed in master (you're just not seeing the fix because it was just merged to master 2 days ago).

[syndek]

Rereading the comments from earlier, I now see that this is indeed fixed, which is excellent. Still a shame that this problem will continue to exist with instance members, but that is technically outside of the scope of this issue, so I'll consider this sorted.

Thanks all.

[Youssef1313]

@syndek I opened a language proposal dotnet/csharplang#3925 for this.

[syndek]

Looks good! Thanks for the help here.

[orthoxerox]

Can confirm it's fixed in RC2