Skip to content

[cxx-interop] Support _LIBCPP_PREFERRED_OVERLOAD #82019

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 1 commit into from
Jun 6, 2025

Conversation

Xazax-hun
Copy link
Contributor

Some functions like memchr are defined both in libc++ and libc. Including both would result in ambiguous references at the call sites. This is worked around by an attribute that tells the compiler to prefer one overload over the others. This attribute was not interpreted by Swift. As a result, importing both libc and libc++ and calling such functions resulted in compilation errors due to ambiguous overloads. This PR modifies the lookup logic to exclude the non-preferred Clang functions from the overload set whenever a preferred overload is available.

rdar://152192945

@Xazax-hun Xazax-hun added the c++ interop Feature: Interoperability with C++ label Jun 5, 2025
@Xazax-hun Xazax-hun force-pushed the gaborh/ambiguous-overloads branch 2 times, most recently from 7eabaec to 33658ae Compare June 6, 2025 11:18
Copy link
Contributor

@hnrklssn hnrklssn left a comment

Choose a reason for hiding this comment

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

I'm fine with not implementing the full semantics available in clang, but the differences need to be written down in a comment

if (auto litExpr =
dyn_cast<clang::CXXBoolLiteralExpr>(clangAttr->getCond())) {
if (litExpr->getValue()) {
thisDeclHasPreferredOverload = hasPreferredOverload = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

this keeps checking enable_if until one is true. The semantics of the attribute is the opposite: the function is only available if all of them evaluate to true.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah okay. So are you okay with clarifying this in a comment? I don't think we want to do full enable_if evaluations here.

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah absolutely

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a comment.

Some functions like memchr are defined both in libc++ and libc.
Including both would result in ambiguous references at the call sites.
This is worked around by an attribute that tells the compiler to prefer
one overload over the others. This attribute was not interpreted by
Swift. As a result, importing both libc and libc++ and calling such
functions resulted in compilation errors due to ambiguous overloads.
This PR modifies the lookup logic to exclude the non-preferred Clang
functions from the overload set whenever a preferred overload is
available.

rdar://152192945
@Xazax-hun Xazax-hun force-pushed the gaborh/ambiguous-overloads branch from 33658ae to 27879c7 Compare June 6, 2025 17:05
@Xazax-hun
Copy link
Contributor Author

@swift-ci please smoke test

@Xazax-hun Xazax-hun enabled auto-merge June 6, 2025 17:13
@Xazax-hun Xazax-hun merged commit a2a760a into main Jun 6, 2025
3 checks passed
@Xazax-hun Xazax-hun deleted the gaborh/ambiguous-overloads branch June 6, 2025 22:44
Xazax-hun added a commit that referenced this pull request Jun 9, 2025
Explanation: Some functions are implemented both in libc and libc++.
Clang uses the enable_if attribute to resolve otherwise ambiguous
functions calls. This PR makes the name lookup aware of this attribute.
Issue: rdar://152192945
Risk: Low, only C/C++ APIs with enable_if attributes are affected.
Testing: Regression test added.
Original PR: #82019
Reviewer: @hnrklssn
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.

2 participants