refactor: Drop deprecated space in operator""_mst #31267

1. 
   maflcko commented at 10:10 am on November 11, 2024: member

   The space is deprecated according to https://en.cppreference.com/w/cpp/language/user_literal#Literal_operators and compilers will start to warn about this. For example, GCC-15 should warn when compiling under C++23, according to https://gcc.gnu.org/onlinedocs/gcc/C_002b_002b-Dialect-Options.html#index-Wdeprecated-literal-operator and Clang-20 will do so by default, according to https://clang.llvm.org/docs/DiagnosticsReference.html#wdeprecated-literal-operator.

   Fix it by removing the unused and deprecated space.

   Also, fix the iwyu include list, while touching the module.

2. 
   DrahtBot commented at 10:10 am on November 11, 2024: 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/31267.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	TheCharlatan, l0rinc

   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 Nov 11, 2024
4. 
   TheCharlatan commented at 10:27 am on November 11, 2024: contributor

   0git grep -i 'operator"" '
   1src/test/fuzz/miniscript.cpp:using miniscript::operator"" _mst;
   2src/test/miniscript_tests.cpp:using miniscript::operator"" _mst;
   

   Might want to correct those two imports as well?

5. refactor: Drop deprecated space in operator""_mst faf2162565
6. maflcko force-pushed on Nov 11, 2024
7. fanquake added this to the milestone 29.0 on Nov 11, 2024
8. TheCharlatan approved
9. 
   TheCharlatan commented at 1:10 pm on November 11, 2024: contributor

   ACK faf21625652fd0d4bbf9b86fd9ebedb5857505ea
10. in src/test/miniscript_tests.cpp:296 in faf2162565

    292@@ -293,7 +293,7 @@ class TestSignatureChecker : public BaseSignatureChecker {
    293 
    294 using Fragment = miniscript::Fragment;
    295 using NodeRef = miniscript::NodeRef<CPubKey>;
    296-using miniscript::operator"" _mst;
    297+using miniscript::operator""_mst;
    

    ---

    ---

    

    l0rinc commented at 1:19 pm on November 11, 2024:

    nit: given the focus on miniscript here, we might as well use the whole namespace like we do for e.g. std::literals:

    0using namespace miniscript;
    

    ---

    

    maflcko commented at 1:40 pm on November 11, 2024:

    An explicit namespace can avoid ambiguity and naming collisions, or inconsistencies arising from it, so I’ll leave this as-is for now. std::literals is different, because there can’t be any collisions (as explained in the first link I provided in the pull request description).
11. in src/test/miniscript_tests.cpp:294 in faf2162565

    292@@ -293,7 +293,7 @@ class TestSignatureChecker : public BaseSignatureChecker {
    293 
    294 using Fragment = miniscript::Fragment;
    

    ---

    ---

    

    l0rinc commented at 1:22 pm on November 11, 2024:

    nit: Fragment seems unused

    ---

    

    maflcko commented at 1:45 pm on November 11, 2024:

    May do if I retouch, or if clang-tidy is patched to find this. Ref:

    0$ git grep 'using' src/.clang-tidy
    1src/.clang-tidy:misc-unused-using-decls,
    

12. l0rinc approved
13. 
    l0rinc commented at 1:25 pm on November 11, 2024: contributor

    ACK faf21625652fd0d4bbf9b86fd9ebedb5857505ea
14. 
    fanquake commented at 1:29 pm on November 11, 2024: member

    Clang-20 will do so by default, according to https://clang.llvm.org/docs/DiagnosticsReference.html#wdeprecated-literal-operator.

    Checked that main is doing this:

    0In file included from /bitcoin/src/test/miniscript_tests.cpp:17:
    1/bitcoin/src/script/miniscript.h:153:34: warning: identifier '_mst' preceded by whitespace in a literal operator declaration is deprecated [-Wdeprecated-literal-operator]
    2  153 | inline consteval Type operator"" _mst(const char* c, size_t l) {
    3      |                       ~~~~~~~~~~~^~~~
    4      |                       operator""_mst
    5/bitcoin/src/test/miniscript_tests.cpp:296:30: warning: identifier '_mst' preceded by whitespace in a literal operator declaration is deprecated [-Wdeprecated-literal-operator]
    6  296 | using miniscript::operator"" _mst;
    7      |                   ~~~~~~~~~~~^~~~
    8      |                   operator""_mst
    92 warnings generated.
    

15. fanquake merged this on Nov 11, 2024
16. fanquake closed this on Nov 11, 2024

    ---

17. fanquake referenced this in commit 41c2adea9c on Nov 11, 2024
18. maflcko deleted the branch on Nov 11, 2024
19. TheCharlatan referenced this in commit a73b2bd0f0 on Nov 14, 2024
20. 
    hebasto commented at 10:35 pm on November 16, 2024: member

    Post-merge ACK faf21625652fd0d4bbf9b86fd9ebedb5857505ea.

Contributors
maflcko DrahtBot TheCharlatan l0rinc fanquake hebasto

Labels
Refactoring

Milestone
29.0