upstream: add and use defaulted getters to UniValue #20045

pull kallewoof wants to merge 3 commits into bitcoin:master from kallewoof:202009-defaulted-get changing 13 files +103 −250

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
kallewoof force-pushed on Sep 30, 2020
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:

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
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

Contributors

Labels

RPC/REST/ZMQ Upstream

upstream: add and use defaulted getters to UniValue #20045

Conflicts