add more keylet host functions, helper macros #5522
Conversation
| @@ -51,6 +51,10 @@ setData( | |||
| return ssz; | |||
| } | |||
|
|
|||
| #pragma push_macro("EXTRACT_ARG_UINT32") | |||
| #undef EXTRACT_ARG_UINT32 | |||
| #define EXTRACT_ARG_UINT32(i) (params->data[i].of.i32) | |||
There was a problem hiding this comment.
This macro doesn't save any space, don't see any good reason for it to be introduced.
There was a problem hiding this comment.
Easier to understand what's going on with it IMO
| @@ -68,6 +72,42 @@ getData(InstanceWrapper const* rt, int32_t src, int32_t ssz) | |||
| return data; | |||
| } | |||
|
|
|||
| #pragma push_macro("EXTRACT_ARG_BLOB") | |||
| #undef EXTRACT_ARG_BLOB | |||
| #define EXTRACT_ARG_BLOB(i) \ | |||
There was a problem hiding this comment.
These macroses (EXTRACT...) doesn't use any specific macro functionality, so they must be functions
There was a problem hiding this comment.
Used as macros because of error handling - otherwise you have to have special error handling for that. I'd ideally prefer a templated function version, but I can't get that to work properly - I plan on updating to that when I get it working (which would be in a separate PR).
| @@ -23,6 +23,46 @@ | |||
|
|
|||
| namespace ripple { | |||
|
|
|||
| #pragma push_macro("ACCOUNT_PARAM") | |||
There was a problem hiding this comment.
Better to revert this full change, it will make much more difficult to debug.
There was a problem hiding this comment.
I think this makes this file much easier to read/reason about - before, I had to do all this analysis myself
| @@ -51,6 +51,10 @@ setData( | |||
| return ssz; | |||
| } | |||
|
|
|||
| #pragma push_macro("EXTRACT_ARG_UINT32") | |||
There was a problem hiding this comment.
You don't need to push and pop macros inside cpp file, just #undef will be fine
There was a problem hiding this comment.
It's done in other cpp files in the repo, would rather stick with the norms
| // function wrappers | ||
| #pragma push_macro("KEYLET_WRAPPER") | ||
| #undef KEYLET_WRAPPER | ||
| #define KEYLET_WRAPPER(FUNC, i, ...) \ |
There was a problem hiding this comment.
Please use macro only for name concatenation, and make the body as another general function / template
There was a problem hiding this comment.
Needed for the function name
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## ripple/smart-escrow #5522 +/- ##
=====================================================
- Coverage 78.7% 78.5% -0.2%
=====================================================
Files 825 825
Lines 73098 73311 +213
Branches 8548 8684 +136
=====================================================
+ Hits 57553 57584 +31
- Misses 15545 15727 +182
... and 7 files with indirect coverage changes 🚀 New features to boost your workflow:
|
|
This PR will be greatly simplified by #5534 so going to wait on that (whatever its end form becomes) |
High Level Overview of Change
Context of Change
Type of Change
.gitignore, formatting, dropping support for older tooling)API Impact
libxrplchange (any change that may affectlibxrplor dependents oflibxrpl)