Runtime: Generic struct including itself breaks the runtime - should be a compiler error?

Created on 28 Jun 2018  路  10Comments  路  Source: dotnet/runtime

Minimal repro:

```c#
struct Foo { X a; }
struct X { }

Even though `X<Y>` is known constant size, the runtime can't load `Foo`:

> Unhandled Exception: System.TypeLoadException: Could not load type 'Foo' from assembly (blah)

---

Not 100% sure whether I should log this in csharplang or coreclr, but it should probably *either*

- work in the runtime (move issue to coreclr)
- tell the user at compile-time (error? similar to CS0523?)

---

real world scenario was actually something more like:

```c#
readonly struct Frame {
    Frame[] subItems;
    // ... some other stuff
}

that I tried to migrate to:

c# readonly struct Frame { Memory<Frame> subItems; // ... some other stuff }

(so I could use oversized arrays leased from the array pool, and recycle them after processing payloads)

area-TypeSystem-coreclr

Most helpful comment

@mgravell

Now, if the question is "is there a T field`, then the compiler can't enforce it because the lib could be an empty reference lib, or could change due to build vs run assemblies

Aren't private struct fields included in reference assemblies nowadays? (Though I guess older reference assemblies that don't include private struct fields are still being used.)

And when the run-time assembly is different than the build-time assembly, I think that's not a problem the compiler should worry about and that it's okay if the runtime throws an exception in that case.

All 10 comments

IIRC I've hit this once or twice. I would love if this could work in the runtime.

Based on the current spec, I think that minimal repro should be valid code (a runtime bug).

However this should not:

struct Foo { X<Foo> a; } // error CS0523
struct X<Y> { Y b; }

A variable of a struct type directly contains the data of the struct, whereas a variable of a class type contains a reference to the data, the latter known as an object. When a struct B contains an instance field of type A and A is a struct type, it is a compile-time error for A to depend on B or a type constructed from B. A struct X directly depends on a struct Y if X contains an instance field of type Y. Given this definition, the complete set of structs upon which a struct depends is the transitive closure of the directly depends on relationship.

Though I suppose it isn't entirely clear how this applies to closed vs open types... This is also a compiler error:

struct Foo<T> { X<Foo<T>> a; } // error CS0523
struct X<Y> { Y b; }

Anyway the spec doesn't mention the type parameter being part of a dependency loop without a field.

@bbarry note: in more realistic examples there is a field - I agree it would bee invalid if the field could involve a type param, but to use the Memory<T> example; that's actually object / int / int. Now, if the question is "is there a T field`, then the compiler can't enforce it because the lib could be an empty reference lib, or could change due to build vs run assemblies - but it is interesting that you think it should work in that scenario - in which case there is nothing for the compiler to enforce and it is a runtime issue.

@mgravell

Now, if the question is "is there a T field`, then the compiler can't enforce it because the lib could be an empty reference lib, or could change due to build vs run assemblies

Aren't private struct fields included in reference assemblies nowadays? (Though I guess older reference assemblies that don't include private struct fields are still being used.)

And when the run-time assembly is different than the build-time assembly, I think that's not a problem the compiler should worry about and that it's okay if the runtime throws an exception in that case.

@svick yeah, there's a couple of other things that break in that scenario, such as "definite assignment" - if you compile against a ref lib it doesn't apply all the usual checks

Regarding Memory (and other Span related types) I think the current language spec should be give 3 different results based on what you are targeting:

  • net461 (or other desktop moniker: using the portable versions) - CS0523
  • netstandard2.0 - compiles, TypeLoadException when loaded desktop framework
  • netcoreapp2.1 - compiles and runs fine (current runtime exception is bug here)

I'd advocate that the language should change such that all 3 compile on the grounds that the struct field dependency is a runtime limitation and thus should not impact the language.

I think the runtime limitation is acceptable and a TypeLoadException is appropriate when a struct contains a direct reference to itself is loaded into memory to be used as an instance, but the type should be able to exist even if you cannot use it.

@bbarry re your last paragraph: that's the current situation. And if it is fixed in the future maybe it doesn't need anything since making it illegal to compile would prevent a valid code setup.

Maybe what this actually needs is an warning or analyzer that - as you say - depends on what you're targeting "woah, this might not work at all", but ... you're right: it isn't a language issue, and as a runtime issue it is "already fixed". This puts us in the position that maybe the broken status quo is the best we can do :/

I think I have been misunderstood:


1:

The current situation:

struct Foo { X<Foo> a; }
struct X<Y> { }

fails at runtime and IMO should not.


2:

A similar situation:

struct Foo { X<Foo> a; }
struct X<Y> { Y b; }

fails at compile time due to the language spec. It also fails at runtime (and IMO should due to the architectural limitations of what value types are within the CLR).


3:

A slightly more complex situation:

//assembly a
struct Foo { X<Foo> a; }

// assembly b version 1.0-ref
struct X<Y> {  }
// assembly b version 1.0-net46
struct X<Y> { Y b; }
// assembly b version 1.0-netcore
struct X<Y> { IntPtr b; }

compiles sometimes and doesn't other times based on a flag not documented to alter this behavior at all.

I think the last case is a spec problem. And fixing it is worth doing even if it is done in a way that case 2 becomes purely a runtime failure, because in that case the language would be more consistent with itself.

Tagging subscribers to this area: @vitek-karas, @agocke
See info in area-owners.md if you want to be subscribed.

Was this page helpful?
0 / 5 - 0 ratings