[lldb] Remove dead code block (NFC)

[xgupta]

The check that max_bit_pos == sign_bit_pos conflicts with the check that sign_bit_pos < max_bit_pos in the block surrounding it.

Originally found by cppcheck -
lldb/source/Utility/Scalar.cpp:756:23: warning: Opposite inner 'if' condition leads to a dead code block. [oppositeInnerCondition]

Fixes #85985

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Shivam Gupta (xgupta)

Changes

Fixes #85985

lldb/source/Utility/Scalar.cpp:756:23: warning: Opposite inner 'if' condition leads to a dead code block. [oppositeInnerCondition]

---

Full diff: https://github.com/llvm/llvm-project/pull/94775.diff

1 Files Affected:

• (modified) lldb/source/Utility/Scalar.cpp (+1-3)

diff --git a/lldb/source/Utility/Scalar.cpp b/lldb/source/Utility/Scalar.cpp
index c70c5e1079918..c680101aa9efa 100644
--- a/lldb/source/Utility/Scalar.cpp
+++ b/lldb/source/Utility/Scalar.cpp
@@ -753,9 +753,7 @@ bool Scalar::SignExtend(uint32_t sign_bit_pos) {
       return false;
 
     case Scalar::e_int:
-      if (max_bit_pos == sign_bit_pos)
-        return true;
-      else if (sign_bit_pos < (max_bit_pos - 1)) {
+      if (sign_bit_pos < (max_bit_pos - 1)) {
         llvm::APInt sign_bit = llvm::APInt::getSignMask(sign_bit_pos + 1);
         llvm::APInt bitwize_and = m_integer & sign_bit;
         if (bitwize_and.getBoolValue()) {

[JDevlieghere]

The change LGTM, but I have a few suggestions regarding the title and description:

• Retitle the PR to something like "[lldb] Remove dead code block (NFC)" or something that conveys the intent/outcome. The modified file and line number are already part of the commit and are just redundant. You could mention it was found by cppcheck if you think that's useful.
• Update the description to explain why this code block is dead. Something along the lines of "The check that max_bit_pos == sign_bit_pos conflicts with the check that sign_bit_pos < max_bit_pos in the block surrounding it" or something like that. Definitely mention the issue but don't just repeat the warning without enough context to make sense of it.

[xgupta]

The change LGTM, but I have a few suggestions regarding the title and description:

Retitle the PR to something like "[lldb] Remove dead code block (NFC)" or something that conveys the intent/outcome. The modified file and line number are already part of the commit and are just redundant. You could mention it was found by cppcheck if you think that's useful.

Update the description to explain why this code block is dead. Something along the lines of "The check that max_bit_pos == sign_bit_pos conflicts with the check that sign_bit_pos < max_bit_pos in the block surrounding it" or something like that. Definitely mention the issue but don't just repeat the warning without enough context to make sense of it.

Thanks, will update other PRs also as you suggested.

[xgupta]

gentle ping!

[JDevlieghere]

LGTM!

[fmayer]

Random drive-by:

The check that max_bit_pos == sign_bit_pos conflicts with the check that sign_bit_pos < max_bit_pos in the block surrounding it.

Should we add an assert to this function then?