miniscript, refactor: Make operator""_mst consteval (re-take) #32564

1. 
   hebasto commented at 1:47 pm on May 19, 2025: member

   Same as #28657, but without the refactoring required to work around fixed MSVC bugs.

   The second commit has been taken from #29167.

2. hebasto added the label Refactoring on May 19, 2025
3. 
   DrahtBot commented at 1:47 pm on May 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/32564.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	hodlinator, sipa

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

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #31713 (miniscript refactor: Remove unique_ptr-indirection (#30866 follow-up) by hodlinator)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

4. DrahtBot added the label CI failed on May 19, 2025
5. DrahtBot removed the label CI failed on May 20, 2025
6. fanquake requested review from sipa on May 20, 2025
7. fanquake assigned darosior on May 20, 2025
8. fanquake unassigned darosior on May 20, 2025
9. fanquake requested review from darosior on May 20, 2025
10. in src/script/miniscript.h:135 in bc4fb79a4e outdated

    134 
    135 public:
    136-    //! Construction function used by the ""_mst operator.
    137-    static consteval Type Make(uint32_t flags) noexcept { return Type(flags); }
    138+    //! The only way to publicly construct a Type is using this literal operator.
    139+    friend constexpr Type operator""_mst(const char* c, size_t l);
    

    ---

    ---

    

    hodlinator commented at 6:53 am on June 25, 2025:

    nits in bc4fb79a4e2ce90f1340328cae033e008227c166: Applied a revert of 63317103c9f2b0635558da814567bb79c17ae851 to the PR base (ff1ee102c4babadd85e4a18234061f10daaa119d) and noticed it brought back the space before the literal name:

    0friend constexpr Type operator"" _mst(const char* c, size_t l);
    

    The space was removed in faf21625652fd0d4bbf9b86fd9ebedb5857505ea, so I guess you manually edited the revert to account for that? Might want to mention it in the commit message.

    You missed preserving the added newline in the definition from faf21625652fd0d4bbf9b86fd9ebedb5857505ea:

    0inline constexpr Type operator""_mst(const char* c, size_t l)
    1{
    

    ---

    

    hebasto commented at 10:35 am on June 25, 2025:

    Thanks! Reworked.
11. in src/script/miniscript.h:157 in b1f7877bb7 outdated

    153@@ -154,12 +154,11 @@ class Type {
    154 };
    155 
    156 //! Literal operator to construct Type objects.
    157-inline consteval Type operator""_mst(const char* c, size_t l)
    158-{
    159-    Type typ{Type::Make(0)};
    160+inline consteval Type operator""_mst(const char* c, size_t l) {
    

    ---

    ---

    

    hodlinator commented at 7:11 am on June 25, 2025:

    nit: Tangential, but consteval/constexpr implies inline AFAIK. Understand if you don’t want to diverge more from the commits you revert/apply in this PR though.

    ---

    

    hebasto commented at 10:37 am on June 25, 2025:

    Correct. I’ve just picked @sipa’s commit.
12. hodlinator approved
13. 
    hodlinator commented at 7:18 am on June 25, 2025: contributor

    ACK b1f7877bb79f72d212d0ad6907ec1057f96693a7

    Cleans up workarounds made for older MSVC versions.

    Mainly code review, but also tried applying the revert on top of PR base myself and found some nits.

14. Revert "miniscript: make operator_mst consteval"

    This reverts commit 63317103c9f2b0635558da814567bb79c17ae851.
    
    operator""_mst has been manually adjusted according to commit
    faf21625652fd0d4bbf9b86fd9ebedb5857505ea

    14052162b1
15. miniscript: Make `operator""_mst` `consteval` a34fb9ad6c
16. hebasto force-pushed on Jun 25, 2025
17. 
    hebasto commented at 10:38 am on June 25, 2025: member

    @hodlinator

    Thank you for the review. Your feedback has been addressed.

18. 
    hodlinator commented at 11:49 am on June 25, 2025: contributor

    re-ACK a34fb9ad6c6cb4ffafdcefefa1ab957a430b69cf
19. 
    sipa commented at 1:38 pm on June 25, 2025: member

    ACK a34fb9ad6c6cb4ffafdcefefa1ab957a430b69cf
20. 
    hebasto commented at 2:03 pm on June 27, 2025: member

    Friendly ping @maflcko @theuni @darosior @fanquake who were reviewing #28657.
21. fanquake merged this on Jul 2, 2025
22. fanquake closed this on Jul 2, 2025

    ---

23. hebasto deleted the branch on Jul 2, 2025

Contributors
hebasto DrahtBot hodlinator sipa

Review Requested
sipa darosior

Labels
Refactoring