refactor: add [[noreturn]] attributes - #339
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline and AI policy for information on the review process.
If your review is incorrectly listed, please copy-paste |
| ReadField( | ||
| TypeList<LocalType>(), invoke_context, input, ReadDestEmplace(TypeList<LocalType>(), | ||
| [](auto&& ...args) -> const LocalType& { throw LocalType{std::forward<decltype(args)>(args)...}; })); | ||
| [] [[noreturn]] (auto&& ...args) -> const LocalType& { throw LocalType{std::forward<decltype(args)>(args)...}; })); |
There was a problem hiding this comment.
In commit "refactor: add missing [[noreturn]] attributes" (82d5c4c)
It seems like this noreturn might not work because it requires c++23 lambda syntax. It is causing some CI jobs to fail with
/home/runner/work/libmultiprocess/libmultiprocess/include/mp/proxy-types.h:223:16: error: an attribute specifier sequence in this position is a C++23 extension [-Werror,-Wc++23-lambda-attributes]
| ReadField( | ||
| TypeList<LocalType>(), invoke_context, input, ReadDestEmplace(TypeList<LocalType>(), | ||
| [](auto&& ...args) -> const LocalType& { throw LocalType{std::forward<decltype(args)>(args)...}; })); | ||
| [] [[noreturn]] (auto&& ...args) -> const LocalType& { throw LocalType{std::forward<decltype(args)>(args)...}; })); |
There was a problem hiding this comment.
Was this change prompted by the -Wmissing-noreturn compiler flag? If so, could you provide the exact build configuration (system, compiler, extra flags) used to reproduce the warning?
There was a problem hiding this comment.
At least with Apple Clang and -Wmissing-noreturn:
export CXXFLAGS="-Wmissing-noreturn"
cmake -B build -G Ninja
-- The CXX compiler identification is AppleClang 21.0.0.21000101
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD - Success
-- Found Threads: TRUE
-- Found ZLIB: /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/lib/libz.tbd (found version "1.2.12")
-- Performing Test HAVE_PTHREAD_GETNAME_NP
-- Performing Test HAVE_PTHREAD_GETNAME_NP - Success
-- Performing Test HAVE_PTHREAD_THREADID_NP
-- Performing Test HAVE_PTHREAD_THREADID_NP - Success
-- Performing Test HAVE_PTHREAD_GETTHREADID_NP
-- Performing Test HAVE_PTHREAD_GETTHREADID_NP - Failed
-- Configuring done (0.9s)
-- Generating done (0.0s)
-- Build files have been written to: /Users/michael/libmultiprocess/build
cmake --build build --clean-first
[1/1] Cleaning all built files...
Cleaning... 0 files.
[5/7] Building CXX object CMakeFiles/multiprocess.dir/src/mp/proxy.cpp.o
In file included from /Users/michael/libmultiprocess/src/mp/proxy.cpp:8:
/Users/michael/libmultiprocess/include/mp/proxy-types.h:223:13: warning: function 'operator()' could be declared with attribute 'noreturn' [-Wmissing-noreturn]
223 | [](auto&& ...args) -> const LocalType& { throw LocalType{std::forward<decltype(args)>(args)...}; }));
| ^
1 warning generated.
[7/7] Linking CXX static library libmultiprocess.aThere was a problem hiding this comment.
It appears that the behavior of -Wmissing-noreturn depends on the Clang version:
$ env CXX="clang++-21" CXXFLAGS="-Wmissing-noreturn" cmake -B build_clang21
$ cmake --build build_clang21
[6/7] Building CXX object CMakeFiles/multiprocess.dir/src/mp/proxy.cpp.o
In file included from /home/hebasto/dev/libmultiprocess/src/mp/proxy.cpp:8:
/home/hebasto/dev/libmultiprocess/include/mp/proxy-types.h:223:13: warning: function 'operator()' could be declared with attribute 'noreturn' [-Wmissing-noreturn]
223 | [](auto&& ...args) -> const LocalType& { throw LocalType{std::forward<decltype(args)>(args)...}; }));
| ^
1 warning generated.
[7/7] Linking CXX static library libmultiprocess.aHowever,
$ env CXX="clang++-22" CXXFLAGS="-Wmissing-noreturn" cmake -B build_clang22
$ cmake --build build_clang22
[7/7] Linking CXX static library libmultiprocess.aCould this be a regression in Clang?
There was a problem hiding this comment.
Maybe we can just turn off the -Wc++23-lambda-attributes error. It seems like it might be less practically useful than the -Wmissing-noreturn one.
There was a problem hiding this comment.
Could this be a regression in Clang?
Not a regression, but a deliberate change: llvm/llvm-project@3baddbb.
These will be used downstream, see bitcoin/bitcoin#35911.
Suppress `-Wc++23-lambda-attributes` warnings in effected jobs.
```bash
/libmultiprocess/include/mp/proxy-types.h:223:16: warning: an attribute specifier sequence in this position is a C++23 extension [-Wc++23-lambda-attributes]
223 | [] [[noreturn]] (auto&& ...args) -> const LocalType& { throw LocalType{std::forward<decltype(args)>(args)...}; }));
| ^
1 warning generated.
```
82d5c4c to
a779a09
Compare
|
It looks like we will also need to add EDIT: That commit doesn't work for older versions of clang or for MSVC. Updated version is ryanofsky/bitcoin@fd7da34 |
4 participants
These will be used downstream, see bitcoin/bitcoin#35911.