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

[tools/tests] Fix bug in 'link all' test and the resulting regression that showed up in code. #18016

Merged
merged 1 commit into from
Apr 11, 2023

Commits on Apr 10, 2023

  1. [tools/tests] Fix bug in 'link all' test and the resulting regression…

    … 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).
    rolfbjarne committed Apr 10, 2023
    Configuration menu
    Copy the full SHA
    b474324 View commit details
    Browse the repository at this point in the history