[PATCH] D130510: Missing tautological compare warnings due to unary operators

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D130510#3727096 <https://reviews.llvm.org/D130510#3727096>, @rtrieu wrote:

> This patch has been moving back and forth between `IsIntegerLiteralConstantExpr` and `getIntegerLiteralSubexpressionValue`.  The first function is preexisting and the second one is a new function.  The final patch seems to settle on using just `getIntegerLiteralSubexpressionValue`.  Can you explain why the existing function does not meet your needs?  It wasn't clear from the update messages why you went that way.

The existing function returns whether the expression is an ICE (sort of), the replacement function calculates the actual value and returns an optional APInt so you can perform operations on it directly. That said, a future refactoring could probably remove `IsIntegerLiteralConstantExpr()` and replace it with `getIntegerLiteralSubexpressionValue()`, but the only caller of `IsIntegerLiteralConstantExpr()` doesn't actually need the value at that point.

> Besides that, there is added support for multiple unary operators, but only minus is tested.  Each one should have at least a positive and a negative test to show it is supported.

Good catch! I agree those tests should be added.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130510/new/

https://reviews.llvm.org/D130510