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

Enable BOOST_STL_INTERFACES_USE_DEDUCED_THIS on Clang 19 and MSVC 19.41 #68

Merged
merged 1 commit into from
Oct 24, 2024

Conversation

ednolan
Copy link
Contributor

@ednolan ednolan commented Sep 6, 2024

Clang 19 and MSVC 17.11.35222.181 both have support for deducing this that is complete enough to allow the v3 version of this library to work but not complete enough for the compilers to define __cpp_explicit_this_parameter. By adding
BOOST_STL_INTERFACES_ENABLE_DEDUCED_THIS, users of those compilers can explicitly opt into the v3 interface even when their compilers do not advertise support for deducing this.

BOOST_STL_INTERFACES_ENABLE_CONCEPTS is added for symmetry.

Copy link
Collaborator

@tzlaine tzlaine left a comment

Choose a reason for hiding this comment

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

I don't want to have two different macros one called DO_IT and one called DONT_DO_IT. What are the intended semantics when they say different things?

Moreover, this is not the way this is done in Boost. If there is a specific version after which language support is available, then we should check for that specific range or versions. Could you update this so that it automatically detects the MSVC verions in question with defined(__cpp_explicit_this_parameter) || YOUR_THING_HERE?

@tzlaine
Copy link
Collaborator

tzlaine commented Oct 24, 2024

@ednolan Ping. The deadline approaches. :) Could you make the requested changes?

Clang 19 and MSVC 19.41 both have support for deducing this that is
complete enough to allow the v3 version of this library to work but
not complete enough for the compilers to define
__cpp_explicit_this_parameter. This commit enables the v3 version of
this library on compilers with that version or newer. We make use of
BOOST_CLANG_VERSION here because on AppleClang it should provide the
version of upstream Clang that was used for that release.
@ednolan ednolan force-pushed the enolan_enablemacros1 branch from dd5baec to 93d9220 Compare October 24, 2024 04:30
@ednolan ednolan changed the title Add support for BOOST_STL_INTERFACES_ENABLE_CONCEPTS and BOOST_STL_INTERFACES_ENABLE_DEDUCED_THIS Enable BOOST_STL_INTERFACES_USE_DEDUCED_THIS on Clang 19 and MSVC 19.41 Oct 24, 2024
@ednolan
Copy link
Contributor Author

ednolan commented Oct 24, 2024

I made the requested changes, let me know if the most recent commit is good. I've verified that utf_view CI passes with it on both MSVC and Clang: https://github.com/ednolan/utf_view/actions/runs/11492656840

@tzlaine
Copy link
Collaborator

tzlaine commented Oct 24, 2024

Thanks! It will be in the next Boost release.

@tzlaine tzlaine merged commit f89646c into boostorg:develop Oct 24, 2024
5 checks passed
tzlaine added a commit that referenced this pull request Nov 10, 2024
…mode for

the cases where __cpp_explicit_this_parameter is not defined, but the compiler
is known to support it.  Obviously, even if those compiler support it, it's
not available in older C++ versions.

Fixes #72.
Related to #68
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