-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Parsing assembly qualified type name fails #84118
Comments
Tagging subscribers to this area: @dotnet/area-system-reflection Issue DetailsDescriptionParsing the type name Reproduction StepsTest code: var t = Type.GetType ("System.Int32, , System.Private.CoreLib.dll");
Console.WriteLine (t); Expected behaviorThis should be printed:
Actual behaviorAn exception:
Regression?Yes, the code works fine in .NET 7. Known WorkaroundsNo response Configuration
Other informationIt happened somewhere in this range: edb161a...8d5f520 I'm guessing this is the culprit: #83484 CC @jkotas
|
We're sure? Based purely on history I would have guessed I might have broken it a few months ago in https://github.com/dotnet/runtime/pull/79048/files#diff-6552989a804d7e6b6e1f065a88d8c17221ea6c6ad561d66348d2f85668f6261f. |
It started breaking in this maestro bump: xamarin/xamarin-macios#17888 |
Yes, the behavior regressed with this change. I am working on a fix. The expected behavior is not correct. |
…assembly name Fixes dotnet#84118
… that showed up in code. There's a 'link all' test that's verifying that the IntroducedAttribute is linked away. It does so by verifying that the linked app doesn't have a 'IntroducedAttribute' type - but the test was constructing the fully qualified type name to look for incorrectly: ObjCRuntime.IntroducedAttribute, , Microsoft.iOS Note the double comma: that meant we wouldn't find the type, even if it wasn't linked away. The fix is easy (use a single comma), with one caveat (don't use a constant string, because the linker sees the reference to "ObjCRuntime.IntroducedAttribute" and _helpfully_ preserves it, exactly what we don't want), but it revealed that the tested behavior regressed: a fully linked app wouldn't link away the IntroducedAttribute. So a fix is also needed: properly remove TVAttribute, WatchAttribute and MacCatalystAttribute, which are subclasses of IntroducedAttribute (and what would make the linker keep IntroducedAttribute). Interestingly this showed up because of a bug in the runtime, where parsing the invalid assembly name would now throw an exception (dotnet/runtime#84118).
… that showed up in code. (#18016) There's a 'link all' test that's verifying that the IntroducedAttribute is linked away. It does so by verifying that the linked app doesn't have a 'IntroducedAttribute' type - but the test was constructing the fully qualified type name to look for incorrectly: ObjCRuntime.IntroducedAttribute, , Microsoft.iOS Note the double comma: that meant we wouldn't find the type, even if it wasn't linked away. The fix is easy (use a single comma), with one caveat (don't use a constant string, because the linker sees the reference to "ObjCRuntime.IntroducedAttribute" and _helpfully_ preserves it, exactly what we don't want), but it revealed that the tested behavior regressed: a fully linked app wouldn't link away the IntroducedAttribute. So a fix is also needed: properly remove TVAttribute, WatchAttribute and MacCatalystAttribute, which are subclasses of IntroducedAttribute (and what would make the linker keep IntroducedAttribute). Interestingly this showed up because of a bug in the runtime, where parsing the invalid assembly name would now throw an exception (dotnet/runtime#84118).
Description
Parsing the type name
"System.Int32, , System.Private.CoreLib.dll"
fails in the latest .NET 8 builds.Reproduction Steps
Test code:
Expected behavior
This should be printed:
Actual behavior
An exception:
Regression?
Yes, the code works fine in .NET 7.
Known Workarounds
No response
Configuration
Other information
It happened somewhere in this range: edb161a...8d5f520
I'm guessing this is the culprit: #83484
CC @jkotas
The text was updated successfully, but these errors were encountered: