Jump to conversation
Unresolved conversations (3)
@lntue lntue Feb 27, 2024
nit: I wonder if using a `switch` for the inner `if`'s would make it easier to read / separate cases for the outer `if`.
Outdated
libc/src/stdio/printf_core/fixed_converter.h
@nickdesaulniers nickdesaulniers Feb 27, 2024
At some point, it might be worthwhile to add something akin to std::clamp to libc/src/__support/CPP/algorithm.h.
.../fuzzing/stdio/printf_fixed_conv_fuzz.cpp
@nickdesaulniers nickdesaulniers Feb 26, 2024
If you put these four values in a helper class, then you could have a method that took `to_conv`; looked at `conv_name`, then initialized these 4 values.
Outdated
libc/src/stdio/printf_core/fixed_converter.h
michaelrj-google
Resolved conversations (12)
@nickdesaulniers nickdesaulniers Feb 26, 2024
Perhaps a todo about using fixed point literals rather than hex?
Outdated
libc/test/src/stdio/sprintf_test.cpp
@nickdesaulniers nickdesaulniers Feb 26, 2024
`sign_char` isn't used until a bit later. Can you move this block down closer to where it's first used?
Outdated
libc/src/stdio/printf_core/fixed_converter.h
@nickdesaulniers nickdesaulniers Feb 26, 2024
Stale comment as of https://github.com/llvm/llvm-project/pull/82707/commits/553c7d9a267465d0eb3bf068454db62e0af01c97
Outdated
libc/test/src/stdio/sprintf_test.cpp
@nickdesaulniers nickdesaulniers Feb 26, 2024
consider making `i` a `uint32_t` as well.
Outdated
libc/src/stdio/printf_core/fixed_converter.h
@nickdesaulniers nickdesaulniers Feb 26, 2024
Do you want to wrap this in ```c #ifndef NDEBUG ``` then?
libc/src/stdio/printf_core/fixed_converter.h
michaelrj-google
@nickdesaulniers nickdesaulniers Feb 26, 2024
DRY; consider making a `clamp` helper function.
Outdated
.../fuzzing/stdio/printf_fixed_conv_fuzz.cpp
@nickdesaulniers nickdesaulniers Feb 26, 2024
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements
Outdated
.../fuzzing/stdio/printf_fixed_conv_fuzz.cpp
@nickdesaulniers nickdesaulniers Feb 26, 2024
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements
Outdated
.../fuzzing/stdio/printf_fixed_conv_fuzz.cpp
@nickdesaulniers nickdesaulniers Feb 26, 2024
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements
Outdated
.../fuzzing/stdio/printf_fixed_conv_fuzz.cpp
michaelrj-google nickdesaulniers
@nickdesaulniers nickdesaulniers Feb 26, 2024
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements
Outdated
.../fuzzing/stdio/printf_fixed_conv_fuzz.cpp
@nickdesaulniers nickdesaulniers Feb 26, 2024
Combine the initial declaration and these assignments.
Outdated
.../fuzzing/stdio/printf_fixed_conv_fuzz.cpp
@nickdesaulniers nickdesaulniers Feb 26, 2024
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements
Outdated
.../fuzzing/stdio/printf_fixed_conv_fuzz.cpp