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

Clang 16 regression causes compilation error in ssl_stream in C++20 mode #2648

Closed
vient opened this issue Mar 2, 2023 · 7 comments
Closed

Comments

@vient
Copy link

vient commented Mar 2, 2023

Hello, I want to notify you about llvm/llvm-project#60749

This is a problem in Clang itself but they decided that it is not a release blocker. Clang 16 will be released in a week and provided code is not compiling. Is it possible to change something on Beast side? It seems that the bug requires very specific conditions.

Sorry for inconvenience, we want to switch to LLVM 16 asap and I'm not sure what to do in light of this bug.

@vient vient changed the title Compilation error with Clang 16 Clang 16 regression causes compilation error in ssl_stream in C++20 mode Mar 2, 2023
@klemens-morgenstern klemens-morgenstern added this to the 1.83 milestone Mar 6, 2023
@vient
Copy link
Author

vient commented Apr 18, 2023

FYI
For our purposes we just made this small patch, after that Beast compiles fine

diff --git a/boost/beast/ssl/ssl_stream.hpp b/boost/beast/ssl/ssl_stream.hpp
index d7ce7c1..51cc9d0 100644
--- a/boost/beast/ssl/ssl_stream.hpp
+++ b/boost/beast/ssl/ssl_stream.hpp
@@ -673,7 +673,7 @@ public:
         ssl_stream<SyncStream>& stream,
         boost::system::error_code& ec);

-    template<class AsyncStream, BOOST_BEAST_ASYNC_TPARAM1 TeardownHandler>
+    template<class AsyncStream, class TeardownHandler>
     friend
     void
     async_teardown(
@@ -697,7 +697,7 @@ teardown(
 }

 template<class AsyncStream,
-        BOOST_BEAST_ASYNC_TPARAM1 TeardownHandler = net::default_completion_token_t<beast::executor_type<AsyncStream>>>
+        class TeardownHandler = net::default_completion_token_t<beast::executor_type<AsyncStream>>>
 void
 async_teardown(
     boost::beast::role_type role,

Loosening requirements is not good but Clang bug lies exactly in applying concepts in template list so removing concepts helps. In original issue someone mentioned that you can also move concept to requires clause, like template<Concept x> to template<class x> requires Concept(x) or something similar, but I did not manage to get working version of this.

williamspatrick added a commit to williamspatrick/beast that referenced this issue May 12, 2023
We are observing the following in openbmc/bmcweb with clang-16.

```
/usr/local/include/boost/beast/websocket/impl/read.hpp:695:17: error: call to 'async_teardown' is ambiguous
```

This was reported in boostorg#2648 and this change was suggested
as a fix.

Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
geissonator pushed a commit to openbmc/openbmc-build-scripts that referenced this issue May 12, 2023
Add a patch to work around the issue discussed in boostorg/beast#2648
and observed in bmcweb testing.  This patch is only necessary in our
Docker container, and not Yocto, because we only compile with clang
or clang-tidy in the Docker environment.

Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
Change-Id: I9e68c74fd8ee1421bf9e49f9f6ce497c8b05a436
@Tectu
Copy link

Tectu commented Aug 22, 2023

Am I correct that a corresponding fix did not make it into 1.83 despite the assigned milestone?

@stromnet
Copy link

stromnet commented Sep 5, 2023

Archlinux has bumped to clang 16 now, causing this error to pop up on my local development machine. clang15 package is available in extras, but would be nice to be able to build with clang 16 too.

@Tectu
Copy link

Tectu commented Sep 5, 2023

Similar issue over at FreeBSD and most likely several other places too. I'm not involved in boost development but maybe this would justify a patch release (i.e. boost 1.83.1)?

@klemens-morgenstern
Copy link
Collaborator

Yeah, I forgot to merge in time. Issue is fixed in develop. Really sorry about that.

@Tectu
Copy link

Tectu commented Sep 5, 2023

Yeah, I forgot to merge in time. Issue is fixed in develop. Really sorry about that.

Could you share a link to the particular commit so downstream package maintainers could potentially ship the patch if no 1.83.1 release is planned to address this issue (is there?)?

@vient
Copy link
Author

vient commented Sep 5, 2023

72c2eeb

fluffykhv added a commit to fluffykhv/freebsd-ports-boost that referenced this issue Sep 8, 2023
The beast module fails to build with clang16+/ gcc13+, it was fixed in boostorg/beast@72c2eeb
See more in GitHub issue: boostorg/beast#2648 (comment)

While here, sync OS versions checks with ports to apply llvm usage
@ashtum ashtum closed this as completed Jan 3, 2024
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

No branches or pull requests

5 participants