Skip to content

[libc] Remove unnecessary check in printf floats #95841

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

michaelrj-google
Copy link
Contributor

Fixes #95638

The check was if(unsigned_num >= 0) which will always be true. The
intent was to check for zero, but the for loop inside the if was
already doing that.

Fixes llvm#95638

The check was `if(unsigned_num >= 0)` which will always be true. The
intent was to check for zero, but the `for` loop inside the `if` was
already doing that.
@llvmbot
Copy link
Member

llvmbot commented Jun 17, 2024

@llvm/pr-subscribers-libc

Author: Michael Jones (michaelrj-google)

Changes

Fixes #95638

The check was if(unsigned_num >= 0) which will always be true. The
intent was to check for zero, but the for loop inside the if was
already doing that.


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

1 Files Affected:

  • (modified) libc/src/stdio/printf_core/float_dec_converter.h (+16-19)
diff --git a/libc/src/stdio/printf_core/float_dec_converter.h b/libc/src/stdio/printf_core/float_dec_converter.h
index 666e4c9fa75e1..1237db62e428a 100644
--- a/libc/src/stdio/printf_core/float_dec_converter.h
+++ b/libc/src/stdio/printf_core/float_dec_converter.h
@@ -502,25 +502,22 @@ LIBC_INLINE int convert_float_decimal_typed(Writer *writer,
 
   const size_t positive_blocks = float_converter.get_positive_blocks();
 
-  if (positive_blocks >= 0) {
-    // This loop iterates through the number a block at a time until it finds a
-    // block that is not zero or it hits the decimal point. This is because all
-    // zero blocks before the first nonzero digit or the decimal point are
-    // ignored (no leading zeroes, at least at this stage).
-    int32_t i = static_cast<int32_t>(positive_blocks) - 1;
-    for (; i >= 0; --i) {
-      BlockInt digits = float_converter.get_positive_block(i);
-      if (nonzero) {
-        RET_IF_RESULT_NEGATIVE(float_writer.write_middle_block(digits));
-      } else if (digits != 0) {
-        size_t blocks_before_decimal = i;
-        float_writer.init((blocks_before_decimal * BLOCK_SIZE) +
-                              (has_decimal_point ? 1 : 0) + precision,
-                          blocks_before_decimal * BLOCK_SIZE);
-        float_writer.write_first_block(digits);
-
-        nonzero = true;
-      }
+  // This loop iterates through the number a block at a time until it finds a
+  // block that is not zero or it hits the decimal point. This is because all
+  // zero blocks before the first nonzero digit or the decimal point are
+  // ignored (no leading zeroes, at least at this stage).
+  for (int32_t i = static_cast<int32_t>(positive_blocks) - 1; i >= 0; --i) {
+    BlockInt digits = float_converter.get_positive_block(i);
+    if (nonzero) {
+      RET_IF_RESULT_NEGATIVE(float_writer.write_middle_block(digits));
+    } else if (digits != 0) {
+      size_t blocks_before_decimal = i;
+      float_writer.init((blocks_before_decimal * BLOCK_SIZE) +
+                            (has_decimal_point ? 1 : 0) + precision,
+                        blocks_before_decimal * BLOCK_SIZE);
+      float_writer.write_first_block(digits);
+
+      nonzero = true;
     }
   }
 

// block that is not zero or it hits the decimal point. This is because all
// zero blocks before the first nonzero digit or the decimal point are
// ignored (no leading zeroes, at least at this stage).
for (int32_t i = static_cast<int32_t>(positive_blocks) - 1; i >= 0; --i) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If positive_blocks was 0, then i is assigned INT32_MAX and the loop is entered?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i is signed, so if positive_blocks is 0 then i should be -1 and the loop should be skipped.

@michaelrj-google michaelrj-google merged commit 7b33c5c into llvm:main Jun 18, 2024
8 checks passed
@michaelrj-google michaelrj-google deleted the libcPrintfCleanPosBlocks branch June 18, 2024 18:17
AlexisPerry pushed a commit to llvm-project-tlp/llvm-project that referenced this pull request Jul 9, 2024
Fixes llvm#95638

The check was `if(unsigned_num >= 0)` which will always be true. The
intent was to check for zero, but the `for` loop inside the `if` was
already doing that.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

libc/src/stdio/printf_core/float_dec_converter.h:505: Possible poor error checking ?
3 participants