[PATCH] D38688: [clang-tidy] Misc redundant expressions checker updated for macros

[Barancsuk Lilla via Phabricator via cfe-commits]

barancsuk added inline comments.


================
Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:510
+// E.g.: from (X == 5) && (X == 5) retrieves 5 and 5
+static void retrieveConstExprFromBothSides(const BinaryOperator *&BinOp,
+                                           BinaryOperatorKind &MainOpcode,
----------------
xazax.hun wrote:
> I would prefer to return a pair of pointer to be returned to output parameters. 
I think, since 'Retrieve'  functions have multiple output parameters, they should remain unchanged. 

'Retrieve' methods are responsible for splitting expressions into parts (e.g.: into opcode and constant integer value) and storing each part into the corresponding output parameter. Therefore it is more reasonable to return whether the operation was successful than returning one of these parameters.


================
Comment at: test/clang-tidy/misc-redundant-expression.cpp:387
   // Should not match.
-  if (X == 10 && Y == 10) return 1;
-  if (X != 10 && X != 12) return 1;
----------------
xazax.hun wrote:
> Why did you remove these test cases?
The above test cases were not removed, just moved into the function `TestLogical`, as they contain only equality operators, and no relational operators.


https://reviews.llvm.org/D38688