Runtime: Unused generic type parameter should not cause loader failure

[gafter]

Consider the following code:

```c#
static class Program
{
static void Main()
{
var m = new M();
}
}

struct N { }
struct M { public N E; }
```

This code will compile with C# 6.0 and is legal according to the CLI spec. The type definition is recursive but the layout of the struct is not because the field involved here is an empty struct. The CLR is unable to handle this though and fails at runtime with a TypeLoadException:

'M:Program.Main' failed: Could not load type 'M' from assembly 'ConsoleApplication1, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null'.
System.TypeLoadException: Could not load type 'M' from assembly 'ConsoleApplication1, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null'.
at Program.Main()

See also dotnet/runtime#5479

[MartinJohns]

Stumbled over exactly this issue the last days. 🙁 Really thought it would work, and it would have been a nice complement to the new System.Memory<T> type.

[verelpode]

because the field involved here is an empty struct.

People might reply, _"True but nobody needs an empty struct in practice"_, therefore here is an example that throws TypeLoadException with a non-empty struct. i.e. a more severe example of the failure.
c# class Program { static void Main(string[] args) { Test(); } static SNode Test() { return new SNode(); } struct ArrayElementReference<T> { public T[] Array; public int Index; } struct SNode { ArrayElementReference<SNode> x; int y; } }

[improbable-nickkrempel]

Suggest changing the title from "Unused generic type parameter ..." to "Self-referencing generic type parameter ...", as it's not related to whether the type parameter is used or unused - see @verelpode's example above, and also https://github.com/dotnet/coreclr/issues/20220, which now seems to be a duplicate of this.

[Korporal]

This issue becomes less academic and much more significant when one begins to leverage recently added support for generic pointers, the unmanaged generic constraint and ref.

Here is a concrete and realistic example (which comes from actual code I've developed) https://github.com/dotnet/roslyn/issues/35324.

[Korporal]

Still no progress on this bug. I can only assume that a few users are raising this, because a truly fundamental failure to process completely legal generated code strikes me as something that would get a little more attention.

[GSPP]

@Korporal the 3.0 release is getting closer and the team is becoming more selective in what issues to take on. That might be the reason. (I'm not a team member.)

[karelz]

Also, this issue was reported by 3 people so far. So while your realistic example may be bad experience (I didn't check it yet), the reality is not too many people hit it yet, which means lower priority. Hardly something as must-have for 3.0 (just my opinion).

[Korporal]

It's been around for three and a half year though!

[gafter]

the reality is not too many people hit it yet

We've had to work around this issue multiple times in Roslyn. Did you need separate reports each time we hit this to realize that it is a persistent problem?

[Korporal]

@karelz I understand this seems to be relativly small impact but it is very fundamental in nature, a very foundational capability. In my case I'm working on a C# rewrite of a custom memory allocator that sits on top of an application's unmanaged memory space. Recent advances to C# in areas like type constraints and the ref keyword have made this possible. However this bug is a solid roadblock because we have a generic struct type SmartPointer<T> which supports lists and trees that cannot be implemented because code compiles but fails at runtime.

The struct type SmartPointer<T> when being used in a list, appears as a member field in a struct type T that can "point" to another instance of a T, this is where the recursive reference comes up. For reasons I can't go into here, we cannot use a conventional unsafe pointer.

(Just to be clear these struct instances are not in managed memory but allocated from unmanaged area of a process's addess space using the custom allocator).

Is there any possibility the bug could be given to a competent intern to see if they can do anything with it? I dont care about it being in 3.0, but currently its going nowhere and this could continue for years more if Microsoft continue to treat it as low importance.

Leaving it unfixed for year after year could also make it harder to fix because the CLR may get refactored or restructured in ways that make the fix more difficult. This feature really is a basic expectation.

Id love to look at it myself even but will not pretend I have the internals knowledge or experience to actually deliver.

[grandseiken]

FWIW, this is an issue we hit repeatedly, particularly in the context of _generated_ C# code, where it is often not easy or feasible to work around (changing output of the code generator wholesale to avoid this problem would be API breaking or have unacceptable knock-on effects, detecting and avoiding on a case-by-case basis would lead to inconsistent or incompatible APIs in some cases).

[Korporal]

@karelz
I see there's no movement on this, is anyone in a position to say that this will or won't be addressed and if so when approx it might be fixed?

[karelz]

No idea - maybe @davidwrighton or @jkotas can comment?

I still believe it is not serious enough to prioritize it. If you have a fix, I think we would be open to a contribution (if it is not overly complex, it is high-quality and not breaking).
General rule is Future = maybe one day.

[davidwrighton]

This has been a known issue in the CLR typesystem since generics were added many years ago. The general problem is that the field type of fields is computed at type load time during initial creation of the type data structures, and that would have to change as part of fixing this issue. In general, it is believed that changes to logic in this system are somewhat high in risk, and without a clear justification for risky work, we can't justify the work. There is a related similar issue with static fields, where some ECMA permitted static fields are not loadable by the runtime.

To the point that @Korporal was making that leaving this issue around will make it harder to fix in the future, unfortunately that isn't really the case. The reasons for this being difficult in the CLR have been part of the type loader design since the late 90's when the runtime was first implemented before generics were even designed in the first place, and so I don't believe we are in significant risk of making it a more difficult fix.

[Korporal]

@davidwrighton

Thanks for elaborating.

I'm certainly pleased that the perceived risk isn't increasing over time, I'm also extremely interested to read "The reasons for this being difficult in the CLR have been part of the type loader design since the late 90's when the runtime was first implemented before generics were even designed in the first place".

So the fundamental flaw is not a result of generic support but something much more fundamental?

Can anyone tell me what I must basically do to be able to begin exploring this and perhaps attempting to address it on a fork of this repo?

Thanks

[improbablejan]

@Korporal

If it's at all helpful (and it might not be since I saw the codebase exactly once), when we ran into this I did try to understand where the issue comes from and how difficult it would be to fix: https://github.com/dotnet/coreclr/issues/20220#issuecomment-430148594

[Korporal]

@improbablejan - That is helpful any information like that is very helpful. Is it possible to point me to the source files specifically? I know it's not a simple matter of editing a few files and moving on but seeing the code will help me.

From what you wrote in that earlier comment (20220) it may be that a fundamental change is needed to enable this, and that may be why it's deemed very risky.

Thanks

[jkotas]

Is it possible to point me to the source files specifically?

This is the specific line that is the root cause of problem: https://github.com/dotnet/coreclr/blob/0a80d79fb64a5878a5163345ac182917f12c95c5/src/vm/methodtablebuilder.cpp#L4035

it may be that a fundamental change is needed to enable this

Yep.

[kekekeks]

Apparently the "full-fledged" CLR is the only runtime where it doesn't work. Mono and experimental CoreRT have no issues with running this code.

[mangod9]

Closing since there doesnt seem to be any particular reasons to be fix in the near term.

[MartinJohns]

@mangod9 Is it really a good approach to simply close issues regarding bugs, just because you don't plan to fix them in the near time? Especially considering that even the Roslyn team had to workaround this bug several times. Personally I'm still waiting and hoping for this to be fixed, and having an open issue is a nice way to at least track it.

I understand that it can be discouraging to have eternally open bug issues... But simply closing them without any resolution just seems weird and the wrong way to handle things.

edit: One of my most frustrating situations in recent times is to look up a bug... Only to find an issue closed due to no activity, while the bug still remains to the present day. Depending on the project this happens fairly frequently. It saddens me to see .NET go the same way.

[mangod9]

I understand your concern @MartinJohns. From the discussion above its unlikely it will be fixed in 6 (or 7) given the risk. Perhaps CoreRT is a possible way around? But happy to keep it open given the interest in this issue :)

[Korporal]

This is just not going away is it! I've raised this several times over the past year or two. We've created code that absolutely relies on this mechanism and that work is just stuck, not going anywhere. The system in question works very closely with mapped memory on Windows and this one bug causes huge headaches for what could be short, sweet, elegant logic.

I wish I had the time and skills to dig into this but I simply don't.

[HurricanKai]

Just encountered this with ReadOnlyMemory<Self>. Very odd that Self[] is allowed but (ReadOnly)Memory<Self> breaks.

[andre-ss6]

Just encountered this issue trying to specifically avoid recursion by boxing the reference 😅

    public struct Box<T> where T : struct {
        private object _boxedValue;

        public T Value { get => (T)_boxedValue; set => _boxedValue = value; }
    }

    public struct BoundingVolume
    {
        // other stuff...

        public Box<BoundingVolume> Father { get; set; }
    }

(Btw, I know there are other ways to do this... Just wanted to point out that I too was a bit puzzled when I encountered the issue)