Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert macros in op.c to inline functions. #18

Open
11 tasks
schwern opened this issue Oct 15, 2015 · 0 comments
Open
11 tasks

Convert macros in op.c to inline functions. #18

schwern opened this issue Oct 15, 2015 · 0 comments

Comments

@schwern
Copy link
Contributor

schwern commented Oct 15, 2015

Candidates include...

  • OpTYPE_set
  • DEFER_OP
  • POP_DEFERRED_OP
  • INIT_OPSLOT
  • RETURN_UNLIMITED_NUMBER
  • DEFER
  • IS_AND_OP
  • IS_OR_OP
  • HV_OR_SCALARHV
  • MAX_ARGS_OP
  • retsetpvs

These should all be only used by op.c, so you can modify them if you like. They should be replaced with static inline functions with the name lower cased. There should be no need to put them into embed.fnc.

For example...

#define IS_AND_OP(o)   (o->op_type == OP_AND)

Becomes...

static inline bool is_and_op(OP *o) { return o->op_type == OP_AND }

But as @johndeppe pointed out, IS_AND_OP(op) and IS_OR_OP(op) are just OP_TYPE_IS_NN(op, OP_AND) and OP_TYPE_IS_NN(op, OP_OR). They're used very infrequently, they're poorly documented, so it's better to delete them and use OP_TYPE_IS_NN().

Remember to...

  • Use the inline branch
  • One macro per commit.
    • Use your judgement if they're small.
  • Run at least "make -j5 minitest" for each commit.
  • Make sure the macro has no side effects.
  • Preserve the macro's visibility (or lack thereof).
    • That means declare it static.
  • grep -r MACRO_NAME to make sure nothing else is using it.
    • There shouldn't be. If there is, please comment below and we'll figure it out.
  • Replacement functions should be declared static inline and the name lower cased.
    • #define IS_AND_OP(o) becomes static inline is_and_op(OP *o)

You don't have to do them all to make a pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant