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

File-scoped IsExternalInit type is used in modreq for init accessors #67079

Closed
jkoritzinsky opened this issue Feb 27, 2023 · 4 comments · Fixed by #67183
Closed

File-scoped IsExternalInit type is used in modreq for init accessors #67079

jkoritzinsky opened this issue Feb 27, 2023 · 4 comments · Fixed by #67183
Assignees
Labels
Milestone

Comments

@jkoritzinsky
Copy link
Member

Version Used: 4.4 (version in .NET SDK 7.0.103)

Steps to Reproduce:

Sharplab link (.NET): https://sharplab.io/#v2:EYLgxg9gTgpgtADwGwBYA0AXEBLANgHwAEAmARgFgAoKgOwEMBbGAZwAc6wYACQ0gOgBKAVxoZsTPgGEIDVnhhQAygoBu2TsyoBvKlz1cAZvJ7EuASWYBRBBgX1cZmtgxctAXyofqlQgGYTXABiEBDauvp+PKQADFwAQnRQrlwA5jAYANxc2E6ZXG5cALxcAEQAEjC4uBBcAOrQuAAmAIQlGZ5UVCS+YZT6JsThejp9/foqiYYhRVw0MADuQSEAFACU7aNjUQCcywYhfAlQ60P5nkA==

Sharplab link (.NET Framework): https://sharplab.io/#v2:EYLgHgbALANALiAlgGwD4AEBMBGAsAKAIDsBDAWwFMBnABxIGMKACdbAOgCUBXIuRStgGEA9mRooKAJwDKUgG6JGVAgG8CTDUwBmElpiYBJKgFEwcKaWQGiiOExUBfAk8L50AZj1MAYsOGr1TQ8WbAAGJgAhEkl7JgBzCjgAbiZEG2SmByYAXiYAIgAJCmRkYSYAdWFJZAATAEI8pOcCAix3APxNPUxAjTVOrs05aO0/HKYiCgB3Hz8ACgBKJoHBkIBOOa0/NijJJd7M5yA=

When available, a file-scoped IsExternalInit type is used in modreqs for init accessors, even on platforms where an IsExternalInit type is available on the platform. Then, the resulting IL for the setter doesn't have the correctly named modreq, so it ends up being treated as a regular set accessor, not an init accessor.

Expected Behavior:

The file-scoped IsExternalInit type is not considered a valid definition for the "predefined type" System.Runtime.CompilerServices.IsExternalInit.

Actual Behavior:

A file-scoped IsExternalInit type is not only considered but is preferred over non-file-scoped definitions of IsExternalInit for init accessors. Alternatively, the modreq is emitted in some fashion such that it is considered a valid init-accessor.

@RikkiGibson
Copy link
Contributor

RikkiGibson commented Mar 5, 2023

It's not clear to me when well-known types from the core library should be preferred over declarations of those same types in the current compilation.

For example, when a type is declared both in the current compilation and in a reference, we warn and bind to the type in the current compilation. Is the issue here that we are binding to the wrong type, or is it that we are missing a warning?

@jkoritzinsky
Copy link
Member Author

The issue here is that we bind to a file-scoped type that has the well-known name in source, but after the file-scoped name mangling is applied has a different name in metadata. As a result, defining an init accessor using a file-scoped IsExternalInit type ends up creating a set accessor with a modreq to the file-scoped type (with the mangled name) in the resulting metadata instead of an init accessor.

@jkoritzinsky
Copy link
Member Author

Basically, file-scoped types will never have the "correct" names in metadata even if they have them in source, so binding to them to enable C# language features causes the experience in source to not match the resulting metadata.

@RikkiGibson
Copy link
Contributor

Yeah, perhaps this means we should check that MetadataName matches what we expect for well-known type lookups, or if that doesn't work for some reason, we can special case on IsFileLocal. Thanks!

@RikkiGibson RikkiGibson added Bug and removed untriaged Issues and PRs which have not yet been triaged by a lead labels Mar 5, 2023
@RikkiGibson RikkiGibson added this to the C# 12.0 milestone Mar 5, 2023
@RikkiGibson RikkiGibson self-assigned this Mar 5, 2023
@RikkiGibson RikkiGibson added the Feature - File-Local Types File-local types (file types) label Mar 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants