4 ms·
Oof, all the macros are broken: #define is_space(x) (x == ' ' || x == '\n') #define is_parens(x) (x == '(' || x == ')') Should be #define is_spa
by kindfellow92 9y ago
Oof, all the macros are broken:
#define is_space(x) (x == ' ' || x == '\n')
#define is_parens(x) (x == '(' || x == ')')
Should be
#define is_space(x) ((x) == ' ' || (x) == '\n')
#define is_parens(x) ((x) == '(' || (x) == ')')
Probably doesn’t matter in practice for this. It could end up being a nasty source of bug later on in the project.
- geofft 9y agoThat still evaluates x twice, which can also be a source of bugs. I usually take one of two approaches: either decide that this is a weekend hack and using the macros whenever the expansion isn't obvious in my head is a sign of too much complexity, or use this GCC extension: #define is_space(x) ({ typeof(x) y = x; y == ' ' || y = '\n'; }) (Or in this case, turn it into an actual function and let the compiler figure out optimization.)
- kindfellow92 9y agoGCC extensions make my brain hurt :( Should just use C++ at that point: template<class T> bool is_space(const T & x) { return x == ‘ ‘ || x == ‘\n’; }
- pjmlp 9y agoEven better if you make it: template<class T> constexpr bool is_space(const T & x) { return x == ' ' || x == '\n'; } Debuggable, type safe and same performance as straight C code.
- IvanVergiliev 9y agoAnd throw an `inline` in there just to be more likely to end up with something macro-like.
- atilaneves 9y ago`constexpr` is implicitly `inline`: http://en.cppreference.com/w/cpp/language/inline http://en.cppreference.com/w/cpp/language/inline
- gpderetta 9y agoorthogonally, templates are also implicitly inline. Then again, inline does not mean what most people think.
- kindfellow92 9y ago“inline” is more about linkage than about actual inlining. The compiler will in-line regardless of that attribute.
- kindfellow92 9y agoConstexpr is not necessary. Only required if you want to ensure that a variable is initialized with a pre-computed value. The compiler will optimize in either case if it can, regardless of the constexpr attribute.
- pjmlp 9y ago> Only required if you want to ensure that a variable is initialized with a pre-computed value Yep.
- klmr 9y agoThe function can be trivially implemented in C, since you don’t need a generic input argument type. No idea why OP chose macros in the first place.
- unwind 9y agoI'd argue that is_space() should be isspace(), the standard from <ctypes.h>. Pretty sure it wouldn't make the semantics of the language worse, but it's one less wheel re-invented and makes the code a smidgen easier to read since there's one less concept to learn in it. Also one less thing to debug ...
- unwind 9y agoHeh. Make that <ctype.h>, of course. D'oh. :)