[PATCH] D60445: Change cast to dyn_cast to be consistent with other casts within same scope.

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 15 14:30:40 PDT 2019


dblaikie added inline comments.


================
Comment at: llvm/lib/MC/MCExpr.cpp:764
       if (const MCTargetExpr *L = dyn_cast<MCTargetExpr>(ABE->getLHS()))
-        if (const MCTargetExpr *R = cast<MCTargetExpr>(ABE->getRHS())) {
+        if (const MCTargetExpr *R = dyn_cast<MCTargetExpr>(ABE->getRHS())) {
           switch (ABE->getOpcode()) {
----------------
If this dyn_cast is never false (if the original code never hit an assert) then the 'if' condition should be removed, and the cast should remain as a cast (not a dyn_cast)

If there are cases where the RHS is not an MCTargetExpr (even though the LHS is) - then this change should include a test case that demonstrates/exercises that case.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60445





More information about the llvm-commits mailing list