-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Unblock type forwarding scenarios involving extensions #79905
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
Conversation
Fixes dotnet#79894 This is a temporary fix to unblock the scenario. A proper fix will require significantly more implementation and validation work.
| CheckForwarderEmit(source1, source2, "NS.Forwarded", "NS.Forwarded+Inner"); | ||
| } | ||
|
|
||
| [ClrOnlyFact] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: it doesn't seem like this needs to only be run on Clr hosts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a note to #79615 (comment) to audit this class for conditional facts that don't need to be conditional.
| // https://github.com/dotnet/roslyn/issues/78963 - The grouping and marker types should be among the forwarded types since they are public. | ||
| // They also should be among exported types for library built from source1. Type symbols | ||
| // representing extensions from the language perspective should not be in either set. | ||
| CheckForwarderEmit(source1, source2, "Extensions"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the extension members are not forwarded in this temporary fix? That sounds like that wouldn't actually "unblock" anyone since they would get a similar effect just by removing the TypeForwardedToAttribute
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the extension members are not forwarded in this temporary fix?
Since no one references them in executable code, that should be fine because design time binds to the real declaration wherever it was moved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That sounds like that wouldn't actually "unblock" anyone since they would get a similar effect just by removing the TypeForwardedToAttribute
Note that we are not forwarding grouping and marker types, but the attribute is on the enclosing type and that type is forwarded
* upstream/main: (275 commits) Update yml files to use community branch (dotnet#79950) [main] Source code updates from dotnet/dotnet (dotnet#79920) Allow MEF components to supply assembly path resolvers (dotnet#79218) update auto merge config (dotnet#79949) Update PublishData.json (dotnet#79946) Add comment Add vb tests VB side Fix results streamed by ForAttributeWithMetadataName with multiple parts chore: deleted accitenly committed local file. bugfix: generated the format methods as public as it is with the related properties. tests: updated resources accordingly to the changes. Revert "Sneak in a sneaky test IVT" Revert "Missed one" Missed one Sneak in a sneaky test IVT Export the analyzer loader for the Misc workspace so the Razor generator has a consistent ALC Update localization branch to 17.14 Reorder checks to report issue as early as possible Avoid crash from Script class instantiation (dotnet#79879) Unblock type forwarding scenarios involving extensions (dotnet#79905) ...
Fixes #79894
This is a temporary fix to unblock the scenario. A proper fix will require significantly more implementation and validation work.