upstream: add and use defaulted getters to UniValue

kallewoof commented at 11:43 am on September 30, 2020: member

The first commit is supposed to go upstream.

The second commit shows how we could use this to clean up the code substantially. (140 lines removed)

kallewoof force-pushed on Sep 30, 2020

in src/univalue/include/univalue.h:178 in 54e6062413 outdated

173@@ -174,6 +174,15 @@ class UniValue {
174     const UniValue& get_obj() const;
175     const UniValue& get_array() const;
176 
177+    bool get(bool default_value) const                             { return isNull() ? default_value : get_bool(); }
178+    const std::string& get(const std::string& default_value) const { return isNull() ? default_value : get_str(); }

MarcoFalke commented at 12:01 pm on September 30, 2020:

0    std::string get(const std::string& default_value) const { return isNull() ? default_value : get_str(); }

This will need to return a copy. Otherwise we’ll get memory issues.

kallewoof commented at 12:13 pm on September 30, 2020:

get_str() also returns a const std::string&, so it should be ok, no?

MarcoFalke commented at 12:21 pm on September 30, 2020:

Its the textbook example from https://youtu.be/lkgszkPnV8g?t=845 ;)

kallewoof commented at 4:33 am on October 1, 2020:

Huh… that looks like a bug in the C++ spec, tbh. Anyway, fixed.

jonatack commented at 9:41 am on October 1, 2020:

Its the textbook example from youtu.be/lkgszkPnV8g?t=845 ;)

Thanks for the good video link :+1:

promag commented at 10:24 am on October 1, 2020:

Yup cool stuff.

MarcoFalke commented at 12:04 pm on September 30, 2020: member

Similar to #19544 from @fjahr

MarcoFalke commented at 12:23 pm on September 30, 2020: member

Also similar to #20017

laanwj added the label Upstream on Sep 30, 2020

laanwj added the label RPC/REST/ZMQ on Sep 30, 2020

DrahtBot commented at 7:17 pm on September 30, 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:

#20017 (rpc: Add RPCContext by promag)
#19825 (rpc: simplify setban and consolidate BanMan functions by dhruv)
#19443 (rpc: Add options argument to listtransactions, with paginatebypointer options by kristapsk)
#18531 (rpc: remove deprecated CRPCCommand constructor by MarcoFalke)
#17356 (RPC: Internal named params by luke-jr)

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.

upstream/univalue: add overloaded defaulted get() 28fd43b4c8

kallewoof force-pushed on Oct 1, 2020

refactor: use UniValue::get() for default case c5f7ee02df

NOMERGE: disable univalue subtree check 73fe905445

kallewoof force-pushed on Oct 1, 2020

in src/rpc/rawtransaction_util.cpp:28 in 73fe905445

29-    if (inputs_in.isNull()) {
30-        inputs = UniValue::VARR;
31-    } else {
32-        inputs = inputs_in.get_array();
33-    }
34+    UniValue inputs = inputs_in.get(UniValue(UniValue::VARR));

kallewoof commented at 4:59 am on October 1, 2020:

@MarcoFalke @practicalswift I assume this is also broken?

in src/univalue/include/univalue.h:184 in 73fe905445

179+    const std::string get(const char* default_value) const         { return isNull() ? default_value : get_str(); }
180+    int get(int default_value) const                               { return isNull() ? default_value : get_int(); }
181+    int64_t get(int64_t default_value) const                       { return isNull() ? default_value : get_int64(); }
182+    uint64_t get(uint64_t default_value) const                     { return isNull() ? default_value : (uint64_t)get_int64(); }
183+    double get(double default_value) const                         { return isNull() ? default_value : get_real(); }
184+    const UniValue& get(const UniValue& default_value) const       { return isNull() ? default_value : default_value.isArray() ? get_array() : get_obj(); }

kallewoof commented at 5:02 am on October 1, 2020:

@MarcoFalke @practicalswift Specifically, is this line broken? I would hate to return a copy of the entire UniValue here.

kallewoof commented at 7:18 am on October 1, 2020: member

@promag’s PR is, again, superior. :) Closing.

kallewoof closed this on Oct 1, 2020

kallewoof deleted the branch on Oct 1, 2020

DrahtBot locked this on Feb 15, 2022

upstream: add and use defaulted getters to UniValue #20045

Conflicts