Skip to content

[6.4.x] ClangImporter: Avoid mirroring protocol members that are declared elsewhere#89451

Open
tshortli wants to merge 3 commits into
swiftlang:release/6.4.xfrom
tshortli:suppress-more-mirrored-imports-6.4.x
Open

[6.4.x] ClangImporter: Avoid mirroring protocol members that are declared elsewhere#89451
tshortli wants to merge 3 commits into
swiftlang:release/6.4.xfrom
tshortli:suppress-more-mirrored-imports-6.4.x

Conversation

@tshortli
Copy link
Copy Markdown
Contributor

@tshortli tshortli commented May 27, 2026

  • Explanation: When an ObjC class in an imported Clang module conforms to an ObjC protocol whose requirement shares a selector with an existing, concrete method already declared on the class or a nearby category, SwiftDeclConverter would still synthesize a "mirrored" method. During name lookup and overload resolution, the mirror implementation could be picked instead of the concrete implementation. Prior to MemberImportVisibility, the fact that the compiler chooses the mirrored method was typically invisible to developers under most circumstances. However, with MemberImportVisibility this behavior resulted in unexpected diagnostics requiring developers to import another module. The fix is to skip mirroring when the interface, or a category in the same module as either the interface or the conformance, already declares a concrete (non-accessor) method with the same selector.
  • Scope: Affects projects that import Clang modules containing categories that conform a type that module does not own to some protocol.
  • Issue/Radar: rdar://166912341
  • Original PR: ClangImporter: Avoid mirroring protocol members that are declared elsewhere #89382
  • Risk: Low. This change removes imported member declarations from some Clang modules, which could theoretically be source breaking. However, it only does so when another module that was imported by the affected Clang module declares those members instead, so source compatibility should be preserved in the vast majority of cases by Clang module re-export conventions.
  • Testing: New compiler tests.
  • Reviewers: @j-hui @hnrklssn

@tshortli
Copy link
Copy Markdown
Contributor Author

@swift-ci please test Linux

tshortli added 3 commits May 27, 2026 10:17
Use -verify-additional-prefix to parameterize matching expected diagnostics
rather than resorting to FileCheck. Also, check swift-ide-test output in the
test to make it easier to see exactly what ClangImporter is doing.
…ewhere.

When an ObjC class in an imported Clang module conforms to an ObjC protocol
whose requirement shares a selector with an existing, concrete method already
declared on the class or a nearby category, SwiftDeclConverter would still
synthesize a "mirrored" method.  During name lookup and overload resolution,
the mirror implementation could be picked instead of the concrete
implementation. Prior to MemberImportVisibility, the fact that the compiler
chooses the mirrored method was typically invisible to developers under most
circumstances. However, with MemberImportVisibility this behavior resulted in
unexpected diagnostics requiring developers to import another module.

Skip mirroring when the interface, or a category in the same module as either
the interface or the conformance, already declares a concrete (non-accessor)
method with the same selector. Property accessors don't count: a property only
exposes the member via property syntax, so the mirror is still needed for
method-call syntax. The existing property-mirroring path already had an
analogous check; the two now share a helper.

Resolves rdar://166912341.
@tshortli tshortli force-pushed the suppress-more-mirrored-imports-6.4.x branch from 204f0cd to 34833eb Compare May 27, 2026 17:17
@tshortli
Copy link
Copy Markdown
Contributor Author

@swift-ci please test

@tshortli tshortli marked this pull request as ready for review May 27, 2026 18:43
@tshortli tshortli requested a review from a team as a code owner May 27, 2026 18:44
@tshortli tshortli enabled auto-merge May 27, 2026 18:44
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.

3 participants