Skip to content

Conversation

@egorzhdan
Copy link
Contributor

This brings back e95f6a3 after it got reverted in dd809c6.

Please see the original PR for motivation: #82496.

This fixes the following issue with the original PR:

AppKit renames some of its C constants via API Notes, e.g. NSUpArrowFunctionKey is renamed into NSEvent.SpecialKey.upArrow.rawValue.

In this example, NSEvent is an existing type, but SpecialKey is not, so the Swift compiler synthesizes an empty type named SpecialKey.

The logic that was added to support import-as-member for namespaces did not account for this: it assumed that if a nested type wasn't found, then the name must be invalid.

rdar://154783494

@egorzhdan egorzhdan requested a review from zoecarver as a code owner July 23, 2025 12:29
@egorzhdan egorzhdan added the c++ interop Feature: Interoperability with C++ label Jul 23, 2025
@egorzhdan
Copy link
Contributor Author

@swift-ci please test

swiftlang/llvm-project#11051

@egorzhdan
Copy link
Contributor Author

@swift-ci please test

swiftlang/llvm-project#11051

@egorzhdan
Copy link
Contributor Author

swiftlang/llvm-project#11051

@swift-ci please test

@egorzhdan egorzhdan force-pushed the egorzhdan/allow-qual-swift-name-reland branch from 6076c38 to 4d6d9a7 Compare July 23, 2025 16:34
@egorzhdan
Copy link
Contributor Author

swiftlang/llvm-project#11051

@swift-ci please test

@j-hui
Copy link
Contributor

j-hui commented Jul 24, 2025

The logic that was added to support import-as-member for namespaces did not account for this: it assumed that if a nested type wasn't found, then the name must be invalid.

I'm a little bit confused by this. Was this logic on the Swift side or the Clang side? And what was changed between the original patches and these re-landed patches?

The only difference I see with respect to the original patch is this, on the Swift side:

  if (!parentContext)
     return nullptr;

And the Clang side re-land looks identical to before. But I thought the problem would stem from the Clang side, since the issue required us to revert the Clang side as well. Or did I miss a Clang-side difference?

…w.rawValue`

AppKit renames some of its C constants via API Notes, e.g. `NSUpArrowFunctionKey` is renamed into `NSEvent.SpecialKey.upArrow.rawValue`.

In this example, `NSEvent` is an existing type, but `SpecialKey` is not, so the Swift compiler synthesizes an empty type named `SpecialKey`.

The logic that was added to support import-as-member for namespaces did not account for this: it assumed that if a nested type wasn't found, then the name must be invalid.

rdar://154783494
@egorzhdan egorzhdan force-pushed the egorzhdan/allow-qual-swift-name-reland branch from 4d6d9a7 to e4af21f Compare July 24, 2025 11:47
@egorzhdan
Copy link
Contributor Author

swiftlang/llvm-project#11051

@swift-ci please test

@egorzhdan
Copy link
Contributor Author

@j-hui good question, the split between a Clang-side change and a Swift-side change is indeed confusing here.

The problematic part of this change was actually on the Swift side: the incorrect diagnostic (imported declaration 'NSUpArrowFunctionKey' could not be mapped to 'NSEvent.SpecialKey.upArrow.rawValue') is emitted by Swift. However, Clang also has some validation logic for these attributes, and if the validation failed, the attribute is removed from the decl and not passed over to Swift.

The Clang-side change was correct, so I'm bringing it back as-is.

The Swift-side change rejected attributes that were previously accepted and are present in the SDK, so I made it more permissive this time.

@egorzhdan egorzhdan merged commit ba88078 into swiftlang:main Jul 25, 2025
5 checks passed
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.

3 participants