-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[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
Conversation
7eabaec
to
33658ae
Compare
There was a problem hiding this 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; |
There was a problem hiding this comment.
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
.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah absolutely
There was a problem hiding this comment.
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
33658ae
to
27879c7
Compare
@swift-ci please smoke test |
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
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