JonasToth added a comment.
> Wiring out the lexical comparison and AST based comparison sounds like an
> interesting idea, however IMHO such a setting is too coarse-grained on the
> file level. My guess would be that it depends from expression (fragment) to
> expression fragment how you want to compare them: for macros lexical
> comparison is better, for arithmetic expressions with many parentheses AST
> based recursive comparison may be a better fit (as implemented also in this
> checker).
Yes, I agree. I think having such a utility would make sense as an future
addition, but I would not like to block on it for several reasons:
- do we know how often it this case even appears?
- is it a real false positive in all cases?
- it can be silence with `// NOLINT` if it actually is a false-positive
My gut feeling is that its rather a corner-case (this late patch to
misc-redundant-expression is somewhat indicative for it) and would block the
patch from landing.
Allowing future room for flexibility is certainly good.
================
Comment at: test/clang-tidy/misc-redundant-expression.cpp:109
#define COND_OP_OTHER_MACRO 9
+#define COND_OP_THIRD_MACRO COND_OP_MACRO
int TestConditional(int x, int y) {
----------------
dkrupp wrote:
> JonasToth wrote:
> > Could you please add a test where the macro is `undef`ed and redefined to
> > something else?
> I am not sure what you exactly suggest here. It should not matter what
> COND_OP_THIRD_MACRO is defined to as we lexically compare tokens as they are
> written in the original source code.
>
> Could you be a bit more specific what test case you would like to add?
*should* not matter is my point, please show that :)
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55125/new/
https://reviews.llvm.org/D55125
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
