Replace boost::variant with std::variant #20480

1. 
   MarcoFalke commented at 5:32 pm on November 24, 2020: member

   Now that we can use std::variant from the vanilla standard library, drop the third-party boost variant dependency
2. MarcoFalke added the label Refactoring on Nov 24, 2020
3. MarcoFalke force-pushed on Nov 24, 2020
4. 
   DrahtBot commented at 7:44 pm on November 24, 2020: member

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

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #20832 (rpc: Better error messages for invalid addresses by eilx2)
   • #18194 (Bugfix: GUI: Remove broken ability to edit the address field in the sending address book by luke-jr)
   • #15294 ([moveonly] wallet: Extract RipeMd160 by Empact)

   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.

5. 
   practicalswift commented at 8:02 pm on November 24, 2020: contributor

   Concept ACK
6. in src/rpc/util.cpp:212 in faccd6d836 outdated

   209@@ -209,65 +210,52 @@ CTxDestination AddAndGetMultisigDestination(const int required, const std::vecto
   210     return dest;
   211 }
   212 
   213-class DescribeAddressVisitor : public boost::static_visitor<UniValue>
   

   ---

   ---

   

   promag commented at 11:23 pm on November 25, 2020:

   Have you considered the “overload” approach from https://en.cppreference.com/w/cpp/utility/variant/visit (looks like it may be included in c++20, https://wg21.link/P0051)?

   ---

   

   MarcoFalke commented at 8:33 am on November 26, 2020:

   That’d be dangerous because it allows for implicit conversions and non-exhaustive visitors. Bugs like #17924 would be harder to find if the overload approach was used.

   ---

   

   promag commented at 9:08 am on November 26, 2020:

   Understood, thanks for the explanation.
7. 
   promag commented at 11:23 pm on November 25, 2020: member

   Concept ACK.
8. MarcoFalke force-pushed on Nov 26, 2020
9. 
   RandyMcMillan commented at 3:47 pm on November 26, 2020: contributor

   Concept ACK :)
10. 
    theStack commented at 11:24 pm on November 26, 2020: member

    Concept ACK 🚀
11. 
    jonasschnelli commented at 10:46 am on December 1, 2020: contributor

    Concept ACK
12. 
    laanwj commented at 12:56 pm on December 3, 2020: member

    Concept ACK
13. MarcoFalke force-pushed on Dec 21, 2020
14. in src/rpc/util.cpp:213 in fa1b3b6419 outdated

    209@@ -209,65 +210,52 @@ CTxDestination AddAndGetMultisigDestination(const int required, const std::vecto
    210     return dest;
    211 }
    212 
    213-class DescribeAddressVisitor : public boost::static_visitor<UniValue>
    214-{
    215-public:
    216-    explicit DescribeAddressVisitor() {}
    217+constexpr auto DescribeAddressVisitor = [](const auto& dest) -> UniValue {
    

    ---

    ---

    

    fjahr commented at 1:15 pm on January 1, 2021:

    Could you comment on why you chose to change the approach here but not on DescribeWalletAddressVisitor?

    ---

    

    MarcoFalke commented at 11:34 am on January 4, 2021:

    No reason. Simplfied diff by using the same approach everywhere.
15. 
    fjahr commented at 1:17 pm on January 1, 2021: member

    Concept ACK

    Code looks good, I had started looking into similar changes before I found this.

16. MarcoFalke referenced this in commit bc8ada1c15 on Jan 4, 2021
17. MarcoFalke force-pushed on Jan 4, 2021
18. MarcoFalke force-pushed on Jan 4, 2021
19. MarcoFalke force-pushed on Jan 4, 2021
20. MarcoFalke force-pushed on Jan 4, 2021
21. 
    MarcoFalke commented at 3:00 pm on January 4, 2021: member

    Addressed feedback by @fjahr
22. sidhujag referenced this in commit 2e6fb6f50e on Jan 4, 2021
23. in src/script/standard.h:12 in fa5ab4b7e4 outdated

     8@@ -9,7 +9,7 @@
     9 #include <script/interpreter.h>
    10 #include <uint256.h>
    11 
    12-#include <boost/variant.hpp>
    13+#include <variant>
    

    ---

    ---

    

    fjahr commented at 10:51 pm on January 4, 2021:

    nit: could probably be sorted below string

    ---

    

    MarcoFalke commented at 9:17 am on January 5, 2021:

    Thanks, sorted includes
24. 
    fjahr commented at 10:53 pm on January 4, 2021: member

    Code review ACK fa5ab4b7e4f609a1309503e3b3c5317ebf81a7e9

    Thanks, the simplifications made this very straight forward to review IMO.

25. Replace boost::variant with std::variant faa8f68943
26. MarcoFalke force-pushed on Jan 5, 2021
27. 
    fjahr commented at 11:18 pm on January 5, 2021: member

    Code review ACK faa8f68943615785a2855676cf96e0e96f3cc6bd

    Only changes were sorting of includes and switching of CTxDestination from typedef to type alias.

28. fanquake approved
29. 
    fanquake commented at 4:05 am on January 11, 2021: member

    ACK faa8f68943615785a2855676cf96e0e96f3cc6bd
30. fanquake merged this on Jan 11, 2021
31. fanquake closed this on Jan 11, 2021

    ---

32. MarcoFalke deleted the branch on Jan 11, 2021
33. sidhujag referenced this in commit 3b0248f814 on Jan 11, 2021
34. MarkLTZ referenced this in commit 6de0d15f61 on Feb 16, 2021
35. DrahtBot locked this on Aug 16, 2022

Contributors
MarcoFalke DrahtBot practicalswift promag RandyMcMillan theStack jonasschnelli laanwj fjahr fanquake

Labels
Refactoring