Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[wip] Allow static self-referential struct type fields in interfaces #111984

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

max-charlamb
Copy link
Contributor

@max-charlamb max-charlamb commented Jan 29, 2025

Treat static struct type fields in interfaces as self-referential, similar to static struct type fields inside of structs.

Resolve #104511

@max-charlamb max-charlamb changed the title add interface check [wip] Allow static self-referential struct type fields in interfaces Jan 29, 2025
@JakeYallop
Copy link
Contributor

Will this also resolve

public struct MyStruct
{
    static ImmutableArray<MyStruct> One;
}

which is also mentioned in the issue as causing a type load exception (a separate issue was closed as a duplicate), but has nothing to do with interfaces.

@kg
Copy link
Member

kg commented Jan 31, 2025

Will this also resolve

public struct MyStruct
{
    static ImmutableArray<MyStruct> One;
}

which is also mentioned in the issue as causing a type load exception (a separate issue was closed as a duplicate), but has nothing to do with interfaces.

That's a different-but-similar problem that might be fixable by making some additional changes to this self-referential type check. We'll look into it.

@kg
Copy link
Member

kg commented Jan 31, 2025

For the self-referential ImmutableArray scenario, the minimal change to make it not fail is to change

if (isEnCField || fIsStatic)

so that we don't take that path for the field in question, i.e. by doing !IsValueClass(). Obviously not shippable, but proof of concept that I think we just need to broaden the definition of 'self-referential' here appropriately without breaking anything.

If we simply make all of these cases pass as self-referential though we need to be careful not to break anything - for example there's a bit that goes 'well, the type is self-referential, so we can substitute ourselves for it':

if (IsSelfRef(pByValueClass) ? IsEnum() : pByValueClass->IsEnum())

Which isn't a valid substitution in the scenario where it's self-referential through a generic parameter or implemented interface.

@jkotas jkotas requested a review from davidwrighton January 31, 2025 14:51
@kg
Copy link
Member

kg commented Feb 2, 2025

For the ImmutableArray case this is a narrower solution to identify cases where we might need to load more carefully:
kg@e40d309
image

We can't fix the interface scenario this way because tkTypeDefToAvoidIfPossible doesn't apply to interfaces, only generic arguments. If we expand it to interfaces we can probably reuse the safe load path for the interface scenario too and not need to change anything else.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

System.TypeLoadException When Interface Type Contains Static Field/Property for Derived Struct Type
3 participants