Skip to content

[5.9.0][Macros] Always consider pre-macro-expansion conformances as subsumed by other conformance entry kinds, before considering availability. #67884

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

Merged

Conversation

hborla
Copy link
Member

@hborla hborla commented Aug 11, 2023

  • Explanation: Extension macros that add conformances will have a placeholder ConformanceEntry so that the compiler can reason about which conformances will be added prior to macro expansion, and an explicit ConformanceEntry is added after the macro is expanded. However, the ranking between these conformances was considering availability before the conformance entry kind; if the macro-expanded conformance is less available than the always-available placeholder entry, the placeholder entry would be chosen. This lead to the conformance being dropped on the floor in serialization and SILGen, because the placeholder entry is just a placeholder, and when we create a NormalProtocolConformance for it, it's not complete. The fix is to swap the order of the availability check and the check for pre-macro-expansion placeholder conformance entries.
  • Scope: Only impacts extension macros that introduce conformances.
  • Risk: Very low. The change only swaps the order of two existing checks inside ConformanceLookupTable::compareConformances.
  • Testing: Added a new test. Without this change, the test results in a linker error (because SILGen dropped the macro-introduced conformance).
  • Issue: rdar://113569289
  • Reviewer: @slavapestov
  • Main branch PR: [Macros] Always consider pre-macro-expansion conformances as subsumed by other conformance entry kinds, before considering availability. #67882
  • 5.9 PR: [5.9][Macros] Always consider pre-macro-expansion conformances as subsumed by other conformance entry kinds, before considering availability. #67883

other conformance entry kinds, before considering availability.

(cherry picked from commit 309e340)
@hborla hborla requested a review from a team as a code owner August 11, 2023 02:06
@hborla
Copy link
Member Author

hborla commented Aug 11, 2023

@swift-ci please test

@hborla hborla merged commit 4ad0b12 into swiftlang:release/5.9.0 Aug 11, 2023
@hborla hborla deleted the 5.9.0-extension-macro-availability branch August 11, 2023 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants