diff --git a/DEVELOPERS.md b/DEVELOPERS.md index 47e6a299fa..48cf9a60e3 100644 --- a/DEVELOPERS.md +++ b/DEVELOPERS.md @@ -199,6 +199,74 @@ rz_core_wrap.cxx:32103:61: error: assigning to 'RzDebugReasonType' from incompat * Add a single space after the `//` when writing inline comments: +* Don't overuse macros! + Moving syntactically repetitive code patterns up to 3 lines into a macro is fine. + But anything that hides non-trivial semantics is not allowed. + It makes debugging really hard, hides code logic, and screws test coverage reports. + + Examples: + ```c + // OK: If the member `ops` is very often accessed in the code, + // it is fine to simplify the syntax with a macro. + #define GET_OP_N(n) insn->details->ops[n] + ``` + + ```c + // OK: Repetitive but **simple** initialization patterns. + #define TOKEN(_type, _pat) \ + do { \ + RzAsmTokenPattern *pat = RZ_NEW0(RzAsmTokenPattern); \ + pat->type = RZ_ASM_TOKEN_##_type; \ + pat->pattern = rz_str_dup(_pat); \ + rz_pvector_push(pvec, pat); \ + } while (0) + + // [...] + void set_tokens() { + RzVector *pvec = rz_pvector_new(); + TOKEN(RZ_ASM_TOKEN_REGISTER, "(ptr)"); + TOKEN(RZ_ASM_TOKEN_OPERATOR, "(\\[)|(\\])"); + TOKEN(RZ_ASM_TOKEN_SEPARATOR, "(\\s+)"); + } + ``` + + ```c + // OK: Repetitive but **simple** function definitions implemented for each **type**. + // This case is rare! + // + // In C++ or other languages this would be done with templates or generics. + // C doesn't have this, so macros are fine in this case. + #define DEF_TEMPLATE_FCN(T) \ + T template_like_function() { \ + return (T) (sizeof(T) * sizeof(T)); \ + } + DEF_TEMPLATE_FCN(ut8); + DEF_TEMPLATE_FCN(ut16); + DEF_TEMPLATE_FCN(ut32); + DEF_TEMPLATE_FCN(ut64); + ``` + + ```c + // NOT OK: This hides semantics. + // Implement the case in a static function instead and call it. + #define REPETITIVE_CASE(n) \ + int i = some_fcn(n); \ + i <<= 8; \ + i &= 0x80000; \ + ret = i * other_fcn(n); \ + break; + + // [...] + switch(x) { + case 1: + REPETITIVE_CASE(1) + case 2: + REPETITIVE_CASE(2) + case 3: + REPETITIVE_CASE(3) + } + ``` + ```c int sum = 0; // set sum to 0 ``` diff --git a/doc/asm_strings.md b/doc/asm_strings.md index 667c89f819..063372d66e 100644 --- a/doc/asm_strings.md +++ b/doc/asm_strings.md @@ -153,20 +153,16 @@ static RZ_OWN RzPVector /**/ *get_token_patterns() { // Mnemonic pattern TOKEN(RZ_ASM_TOKEN_MNEMONIC, "^((while)|(inc)|(dec)|(trap)|(nop)|(invalid)|(loop))"); - rz_pvector_push(pvec, pat); // ptr pattern TOKEN(RZ_ASM_TOKEN_REGISTER, "(ptr)"); - rz_pvector_push(pvec, pat); // Reference pattern // Matches a single bracket TOKEN(RZ_ASM_TOKEN_OPERATOR, "(\\[)|(\\])"); - rz_pvector_push(pvec, pat); // Separator pattern TOKEN(RZ_ASM_TOKEN_SEPARATOR, "(\\s+)"); - rz_pvector_push(pvec, pat); return pvec; }