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

Properly cope with ordering issues when importing clang decls during extension binding #78731

Open
beccadax opened this issue Jan 18, 2025 · 0 comments
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. triage needed This issue needs more specific labels

Comments

@beccadax
Copy link
Contributor

Description

In #78697, we worked around a bug where binding an extension of a swift_wrapper/swift_newtype typedef before the underlying type's _ObjectiveCBridgeable extension has been bound would incorrectly make the newtype non-bridgeable. However, the fix taken here is specific to String and hardcodes knowledge that it is Foundation which makes that type bridgeable. This is probably okay since _ObjectiveCBridgeable is compiler-internal, but it would be better to find a more principled fix for the bug.

One possible way to address it would be to defer all of the logic associated with transferKnown() until later in compilation—perhaps when the list of conformances is first used—in hopes that this will be late enough that all extensions will be bound. However, it's not clear whether all of the associated logic can be delayed long enough to pull that off.

Reproduction

  1. Revert Work around Foundation NS_TYPED_ENUM bug #78697's changes to SwiftDeclConverter::importSwiftNewtype() in ImportDecl.cpp, leaving the test changes intact.
  2. Run the lit test ClangImporter/newtype_conformance.swift

Expected behavior

The test should pass. Instead, absent the hack added in #78697, it will fail on the z as NSString line.

Environment

First observed circa 6b2fb2e, but this bug may have been latent in the compiler for a long time since it only affects Foundation in practice.

Additional information

An alternative solution would be to make swift::bindExtensions() defer binding either extensions to Clang-imported types, or extensions which don't have "special" conformances like _ObjectiveCBridgeable, until a second pass. However, this is still a bit of a hack and it would probably incur a minor performance penalty in projects that would not otherwise be affected by this bug.

Apple engineers: see rdar://142693093 for the original occurrence of this bug.

@beccadax beccadax added bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. triage needed This issue needs more specific labels labels Jan 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. triage needed This issue needs more specific labels
Projects
None yet
Development

No branches or pull requests

1 participant