Skip to content

[cxx-interop] Add friend operators to the lookup table properly #70325

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
merged 3 commits into from
Dec 13, 2023

Conversation

egorzhdan
Copy link
Contributor

Previously, friend operators declared in C++ classes were added to the lookup table when the class is being imported.

The operators were added to the wrong lookup table if the class is declared in a C++ namespace. Since a namespace can span across multiple Clang modules, its contents should be added to a translation unit level lookup table, not to a module level lookup table.

This change makes sure we add friend operators to the lookup table earlier, when we are actually building the lookup table. Note that this is not possible for class template instantiations, because those are instantiated later, so for templates we still handle friend operators when importing the instantiation.

rdar://116349899

@egorzhdan egorzhdan added the c++ interop Feature: Interoperability with C++ label Dec 8, 2023
@@ -8,6 +8,10 @@
// CHECK: mutating func callAsFunction(_ x: Int32) -> Int32
// CHECK: mutating func callAsFunction(_ x: Int32, _ y: Int32) -> Int32
// CHECK: }
// CHECK: func == (lhs: LoadableIntWrapper, rhs: LoadableIntWrapper) -> Bool
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A nice side effect of this change is that friend operators now show up in the module interface.

@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test

Previously, `friend` operators declared in C++ classes were added to the lookup table when the class is being imported.

The operators were added to the wrong lookup table if the class is declared in a C++ namespace. Since a namespace can span across multiple Clang modules, its contents should be added to a translation unit level lookup table, not to a module level lookup table.

This change makes sure we add `friend` operators to the lookup table earlier, when we are actually building the lookup table. Note that this is not possible for class template instantiations, because those are instantiated later, so for templates we still handle `friend` operators when importing the instantiation.

rdar://116349899
@egorzhdan egorzhdan force-pushed the egorzhdan/friend-operator-lookup branch from e387670 to c8017e7 Compare December 8, 2023 18:49
@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test

@egorzhdan
Copy link
Contributor Author

Command Output (stderr):
--
/Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/swift/test/Interop/Cxx/stdlib/print-libcxx-module-interface.swift:6:16: error: CHECK-NEXT: is not on the line after the previous match
// CHECK-NEXT: enum __1 {
               ^
<stdin>:141:2: note: 'next' match was here
 enum __1 {
 ^
<stdin>:100:11: note: previous match ended here
enum std {
          ^
<stdin>:101:1: note: non-matching line after previous match is here
 struct exception {
^

Input file: <stdin>
Check file: /Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/swift/test/Interop/Cxx/stdlib/print-libcxx-module-interface.swift

-dump-input=help explains the following input dump.

Input was:
<<<<<<
        .
        .
        .
      136:  struct __throw_with_nested<_Tp, _Up, > { 
      137:  } 
      138:  @available(*, unavailable, message: "Un-specialized class templates are not currently supported. Please use a specialization of this type.") 
      139:  struct __can_dynamic_cast<_From, _To> { 
      140:  } 
      141:  enum __1 { 
next:6      !~~~~~~~~~  error: match on wrong line

libc++ has decls that are declared outside of `inline namespace __1`, which might be printed before it.
@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test

@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test macOS

1 similar comment
@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test macOS

@egorzhdan egorzhdan merged commit 01670cd into main Dec 13, 2023
@egorzhdan egorzhdan deleted the egorzhdan/friend-operator-lookup branch December 13, 2023 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ interop Feature: Interoperability with C++
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant