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

Workarounds for issue #104511 #112115

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Workarounds for issue #104511 #112115

wants to merge 3 commits into from

Conversation

kg
Copy link
Member

@kg kg commented Feb 4, 2025

Draft to check for failures on CI
Alternative to #111984

For two scenarios touched on in issue #104511, we can solve them through different paths in the same InitializeFieldDescs method:

Static field on a struct, of a generic struct type that indirectly references the type hosting the field, i.e.

struct X { static ImmutableArray<X> F; }

We have existing infrastructure for doing cycle-avoidant type loads in cases like this - tkTypeDefToAvoidIfPossible - so we just need to sense that we've encountered a scenario where it might be necessary. We can do this by checking whether the field is:

  1. static
  2. of struct type
  3. of a generic struct type
    We already know 1 & 2, so I add a bit of signature decoding to determine 3. If all three conditions are met we use tkTypeDefToAvoidIfPossible to avoid loading ourselves when loading the field type. This feels like a decent compromise to fix this scenario and shouldn't impact the vast majority of fields in existing applications.

Static field on an interface, of a struct type that implements said interface, i.e.

interface I { static Y F; }
struct Y : I { }

tkTypeDefToAvoidIfPossible can't save us here and flowing it through into the place where interfaces are loaded would require some significant code changes, so I decided to see if I could narrowly target a workaround based on our existing handling of self-referential static fields.
This PR's approach is to look up the type of each static struct field on an interface, and if it's a typedef (not a typeref, which means it's in the current module and can form a cycle), we scan the interfaceimpl table for that typedef to see if we're in it. If we are, treat the field type as if it's a reference to us. This taps into the existing logic for handling self-referential static fields and seems to work, though I had to tweak some code related to enums.

The cost of doing this could get prohibitive for large numbers of static struct type fields on an interface, though I'm having trouble imagining a scenario that would require that, and there are some fairly obvious ways to optimize it.

kg added 2 commits February 1, 2025 17:29
…n an interface in order to determine whether it implements the interface and prevent a cycle during type loading
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.

2 participants