Skip to content

Add an include to xtr1common in ADL.h #7818

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

Closed

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Nov 23, 2023

When including ADL.h in a header that is used from Swift using C++ interop, MSVC complains about a missing definition of remove_reference_t in ADL.h.
Explicitly including <xtr1common> fixes the issue.

@ahoppen ahoppen marked this pull request as ready for review November 23, 2023 05:34
@ahoppen ahoppen changed the title WIP: Add an include to xtr1common in ADL.h Add an include to xtr1common in ADL.h Nov 23, 2023
When including ADL.h in a header that is used from Swift using C++ interop, MSVC complains about a missing definition of `remove_reference_t` in ADL.h.
Explicitly including `<xtr1common>` fixes the issue.
@@ -9,6 +9,9 @@
#ifndef LLVM_ADT_ADL_H
#define LLVM_ADT_ADL_H

#if defined(_MSC_VER) && _MSC_VER < 1937
Copy link
Member Author

Choose a reason for hiding this comment

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

@compnerd You suggested checking for the following to me but that check did not evaluate to true in Windows CI.

#if defined(_MSVC_STL_UPDATE) && _MSVC_STL_UPDATE < 202305l

Based on the _MSVC_STL_UPDATE <-> _MSC_VER mapping, I changed it to a check for _MSC_VER. Does this make sense to you?

Copy link
Member

Choose a reason for hiding this comment

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

That doesn't really work. _MSC_VER is the compiler version, the problem here is the header version, which is _MSVC_STL_UPDATE. I think that we should just figure out what version is in use and use that.

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you have any suggestions I should try?

@ahoppen
Copy link
Member Author

ahoppen commented Nov 29, 2023

Looks like this is no longer needed because we’re no longer importing ADL.h from Swift in the latest version of swiftlang/swift#70008.

@ahoppen ahoppen closed this Nov 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants