refactor: modernize outdated trait patterns using helper aliases (C++14/C++17) #31904

1. 
   l0rinc commented at 1:31 pm on February 19, 2025: contributor

   The use of std::underlying_type_t<T> or std::is_enum_v<T> (and similar ones, introduced in C++14) replace the typename std::underlying_type<T>::type and std::is_enum<T>::value constructs (available in C++11).

   The _t and _v helper alias templates offer a more concise way to extract the type and value directly.

   I’ve modified the instances I found in the codebase one-by-one (noticed them while investigating #31868), and afterwards extracted scripted diff commits to do the trivial ones automatically. The last commit contains the values that were easier done manually.

   I’ve excluded changes from src/bench/nanobench.h, src/leveldb, src/minisketch, src/span.h, src/sync.h and src/test/fuzz/FuzzedDataProvider.h - let me know if you think they should be included instead.

   A few of the code changes can also be reproduced by clang-tidy (but not all of them):

   0cmake -B build -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DBUILD_BENCH=ON -DBUILD_FUZZ_BINARY=ON -DBUILD_FOR_FUZZING=ON && cmake --build build -j$(nproc)
   1run-clang-tidy -quiet -p build -j $(nproc) -checks='-*,modernize-type-traits' -fix $(git grep -lE '::(value|type)' ./src ':(exclude)src/bench/nanobench.h' ':(exclude)src/leveldb' ':(exclude)src/minisketch' ':(exclude)src/span.h' ':(exclude)src/sync.h')
   

2. 
   DrahtBot commented at 1:31 pm on February 19, 2025: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Code Coverage & Benchmarks

   For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31904.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   Stale ACK	purpleKarrot

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

3. DrahtBot added the label Refactoring on Feb 19, 2025
4. l0rinc force-pushed on Feb 19, 2025
5. 
   DrahtBot commented at 1:36 pm on February 19, 2025: contributor

   🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/37469627519

   Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

   • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

   • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

   • An intermittent issue.

   Leave a comment here, if you need help tracking down a confusing failure.

6. DrahtBot added the label CI failed on Feb 19, 2025
7. l0rinc force-pushed on Feb 19, 2025
8. l0rinc force-pushed on Feb 19, 2025
9. in src/randomenv.cpp:76 in f7148eb8d2 outdated

   75-    static_assert(!std::is_same<typename std::decay<T>::type, const char*>::value, "Calling operator<<(CSHA512, const char*) is probably not what you want");
   76-    static_assert(!std::is_same<typename std::decay<T>::type, const unsigned char*>::value, "Calling operator<<(CSHA512, const unsigned char*) is probably not what you want");
   77+    static_assert(!std::is_same<std::decay_t<T>, char*>::value, "Calling operator<<(CSHA512, char*) is probably not what you want");
   78+    static_assert(!std::is_same<std::decay_t<T>, unsigned char*>::value, "Calling operator<<(CSHA512, unsigned char*) is probably not what you want");
   79+    static_assert(!std::is_same<std::decay_t<T>, const char*>::value, "Calling operator<<(CSHA512, const char*) is probably not what you want");
   80+    static_assert(!std::is_same<std::decay_t<T>, const unsigned char*>::value, "Calling operator<<(CSHA512, const unsigned char*) is probably not what you want");
   

   ---

   ---

   

   purpleKarrot commented at 2:11 pm on February 19, 2025:

   You can use std::is_same_v here!

   ---

   

   l0rinc commented at 2:16 pm on February 19, 2025:

   I am, just doing it commit-by-commit: https://github.com/bitcoin/bitcoin/pull/31904/files#diff-4b1ae2ce314ef1f2d4cb4f2dbf7a87a0c299c942782fa734e9e80fed8324faa6R76
10. 
    purpleKarrot commented at 2:17 pm on February 19, 2025: contributor

    This change could be automated with clang-tidy’s modernize-type-traits check. There are more places that can be modernized with this.
11. in src/serialize.h:223 in b920e6c03f outdated

    219@@ -220,13 +220,13 @@ const Out& AsBase(const In& x)
    220     template <typename Stream>                                                                      \
    221     void Serialize(Stream& s) const                                                                 \
    222     {                                                                                               \
    223-        static_assert(std::is_same<const cls&, decltype(*this)>::value, "Serialize type mismatch"); \
    224+        static_assert(std::is_same_v<const cls&, decltype(*this)>, "Serialize type mismatch"); \
    

    ---

    ---

    

    purpleKarrot commented at 2:36 pm on February 19, 2025:

    check formatting.

    ---

    

    l0rinc commented at 2:39 pm on February 19, 2025:

    Thanks, fixed
12. l0rinc force-pushed on Feb 19, 2025
13. 
    l0rinc commented at 2:40 pm on February 19, 2025: contributor

    @purpleKarrot, you can check the PR with clang-tidy if it helps, but it won’t find all of the values changed here. And even if it does, we need to simplify the review, we can’t just rely on clang-tidy blindly. Added the command that I used to check the changes via modernize-type-traits - let me know if you find other ones that I haven’t covered.
14. 
    purpleKarrot commented at 2:51 pm on February 19, 2025: contributor

    ACK.

    Changes LGTM. You may want to drop the reference to C++14 from the title, as std::is_same_v is actually not available until C++17. I you want, you could go full C++20 by replacing std::is_same_v with std::same_as.

15. l0rinc renamed this:
    refactor: modernize outdated trait patterns using helper aliases (C++14)
    refactor: modernize outdated trait patterns using helper aliases (C++14/C++17)
    on Feb 19, 2025
16. 
    l0rinc commented at 2:59 pm on February 19, 2025: contributor

    Thanks for the review and the C++17 suggestion @purpleKarrot. If you agree with the changes, please add the latest hash after your ack, otherwise it’s just a concept ack. Edit: the std::same_as change should probably be a different PR, that’s not just cosmetics anymore.
17. 
    purpleKarrot commented at 3:02 pm on February 19, 2025: contributor

    ACK 7abc6e1f4b4aca4c8c576ff0cc27d68f90e42d28
18. in src/test/fuzz/FuzzedDataProvider.h:1 in 7abc6e1f4b

    ---

    ---

    

    maflcko commented at 3:14 pm on February 19, 2025:

    pls submit those upstream

    ---

    

    l0rinc commented at 3:20 pm on February 19, 2025:

    Haven’t noticed the header, will push a PR to https://github.com/llvm/llvm-project/blob/main/compiler-rt/include/fuzzer/FuzzedDataProvider.h#L206

    ---

    

    l0rinc commented at 7:50 pm on February 19, 2025:

    Pushed https://github.com/llvm/llvm-project/pull/127811 and reverted the file here, thanks!

    ---

    

    l0rinc commented at 9:47 am on February 21, 2025:

    Add back the merged https://github.com/llvm/llvm-project/pull/127811/files#diff-bf90ae5bcd280af8b1305512d76ab1b81d0521bbc5d251f60bc0d44e790f16c0 (last commit makes sure it’s exactly the same as the upstream version)
19. DrahtBot removed the label CI failed on Feb 19, 2025
20. l0rinc force-pushed on Feb 19, 2025
21. scripted-diff: modernize outdated trait patterns - types

    The use of e.g. `std::underlying_type_t<T>` replaces the older `typename std::underlying_type<T>::type`.
    The `_t` helper alias template (such as `std::underlying_type_t<T>`) introduced in C++14 offers a cleaner and more concise way to extract the type directly.
    See https://en.cppreference.com/w/cpp/types/underlying_type for details.
    
    -BEGIN VERIFY SCRIPT-
    sed -i -E 's/(typename )?(std::[a-z_]+)(<[^<>]+>)::type\b/\2_t\3/g' $(git grep -l '::type' ./src ':(exclude)src/bench/nanobench.h' ':(exclude)src/leveldb' ':(exclude)src/minisketch' ':(exclude)src/span.h' ':(exclude)src/sync.h')
    -END VERIFY SCRIPT-

    8327889f35
22. scripted-diff: modernize outdated trait patterns - values

    See https://en.cppreference.com/w/cpp/types/is_enum for more details.
    
    -BEGIN VERIFY SCRIPT-
    sed -i -E 's/(std::[a-z_]+)(<[^<>]+>)::value\b/\1_v\2/g' $(git grep -l '::value' ./src ':(exclude)src/bench/nanobench.h' ':(exclude)src/minisketch' ':(exclude)src/span.h')
    -END VERIFY SCRIPT-

    ab2b67fce2
23. refactor: modernize remaining outdated trait patterns 4cd95a2921
24. l0rinc force-pushed on Feb 21, 2025

Contributors
l0rinc DrahtBot purpleKarrot maflcko

Labels
Refactoring