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.

Sign up for GitHub

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

Jump to bottom

Enable detection for libc++7.0's deprecated <experimental/string_view> #91

Closed
wants to merge 4 commits into from
Closed

Enable detection for libc++7.0's deprecated <experimental/string_view> #91

wants to merge 4 commits into from

Conversation

gnaggnoyil
Copy link

Libc++ 7.0 keeps <experimental/string_view> header but deprecates it with an #error. This patch is intended to fix this problem. Issue #82 should be fixed after applying this patch.

@knejadfard knejadfard mentioned this pull request Mar 30, 2018
Closed
@knejadfard
Copy link

Confirming that this change fixes the issue for me. My compiler details:

clang version 7.0.0 (git@github.com:llvm-mirror/clang.git 297171608bc38595e88b07f52e496cf88c818d42) (git@github.com:llvm-mirror/llvm.git 805514b4e0bf71c6a5c6430e422809e2868b2c52)
Target: x86_64-unknown-linux-gnu
Thread model: posix

libcxx revision: 4e177b9
Boost version: 1.66.0

@rwols
Copy link

rwols commented Apr 2, 2018

I'm guessing this is fixed with b5b17a6 now?

@jbeich
Copy link

jbeich commented Aug 6, 2018

No, Asio tries to use <experimental/string_view> in C++14 mode which is default for Clang 6 or later.

jbeich
jbeich reviewed Aug 8, 2018
View reviewed changes
include/boost/asio/detail/config.hpp Outdated
# define BOOST_ASIO_HAS_STD_EXPERIMENTAL_STRING_VIEW 1
# if defined(_LIBCPP_LFTS_STRING_VIEW)
# define BOOST_ASIO_HAS_STD_EXPERIMENTAL_STRING_VIEW 1
# define BOOST_ASIO_HAS_STD_STRING_VIEW 1
Copy link

Choose a reason for hiding this comment

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

Redundant given separate __has_include(<string_view>) conditional later.

include/boost/asio/detail/config.hpp Outdated
#if !defined(BOOST_ASIO_HAS_STD_STRING_VIEW)
# if !defined(BOOST_ASIO_DISABLE_STD_STRING_VIEW)
# if defined(__clang__)
# if (__cplusplus >= 201402)
# if __has_include(<experimental/string_view>)
# define BOOST_ASIO_HAS_STD_STRING_VIEW 1
# define BOOST_ASIO_HAS_STD_EXPERIMENTAL_STRING_VIEW 1
# if defined(_LIBCPP_LFTS_STRING_VIEW)
Copy link

Choose a reason for hiding this comment

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

_LIBCPP_LFTS_STRING_VIEW is the guard defined by #include <experimental/string_view> but #include cannot be used without bumping into #error on libc++ >= 7.

@gnaggnoyil
Copy link
Author

gnaggnoyil commented Aug 17, 2018 •
edited
Loading

Updated header detection logic. Now config.hpp should correctly detect the correct <string_view> header to use no matter in which version Clang/libc++ is, and in which C++ standard mode when compiling.

dec1
dec1 reviewed Sep 20, 2018
View reviewed changes
Copy link

@dec1 dec1 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

shouldn't this be if (__clang_major__ < 7) ?

@bebuch
Copy link

bebuch commented Oct 15, 2018

Confirming that this patch fixes the issue for me.

Tested with boost develop branch and with LLVM 7, LLVM 6 and GCC 8.2.1.

$ g++ --version
g++ (GCC) 8.2.1 20181012
Copyright (C) 2018 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

$ clang++ --version
clang version 7.0.0 (http://llvm.org/git/clang.git 0513b409d5e34b2d2a28ae21b6d620cc52de0e57) (http://llvm.org/git/llvm.git 65ce2e56889af84e8be8e311f484a4dfe4b62d7a)
Target: x86_64-unknown-linux-gnu
Thread model: posix

$ clang++-6 --version
clang version 6.0.0 (http://llvm.org/git/clang.git ff0c0d8ab3e316bb6e2741fedb3b545e198eab7a) (http://llvm.org/git/llvm.git 089d4c0c490687db6c75f1d074e99c4d42936a50)
Target: x86_64-unknown-linux-gnu
Thread model: posix

Please merge!

Can you also merge this into the last release and provide an update (1.68.1)? Asio and Beast are hardly usable with clang 7 without that patch.

bebuch
bebuch approved these changes Oct 25, 2018
View reviewed changes
Copy link

@bebuch bebuch left a comment

Choose a reason for hiding this comment

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

Tested with boost develop branch and with LLVM 7, LLVM 6 and GCC 8.2.1.

@michaelwegner
Copy link

Merging this would be greatly appreciated!

@bebuch
Copy link

bebuch commented Nov 6, 2018

fixed by 43874d5, please close

@gnaggnoyil gnaggnoyil closed this Nov 7, 2018
@mellery451 mellery451 mentioned this pull request Feb 25, 2019
Closed
@cjones051073 cjones051073 mentioned this pull request Apr 29, 2019
Merged
@theopolis theopolis mentioned this pull request Jun 21, 2019
Closed
@dec1 dec1 mentioned this pull request Sep 9, 2019
Closed
@ghost ghost mentioned this pull request Dec 30, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dec1 dec1 dec1 left review comments

@jbeich jbeich jbeich left review comments

@bebuch bebuch bebuch approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@gnaggnoyil @knejadfard @rwols @jbeich @bebuch @michaelwegner @dec1