This is an archive of the discontinued LLVM Phabricator instance.

[libc] move to combined integer converter
ClosedPublic

Authored by michaelrj on Aug 5 2022, 3:07 PM.

Details

Summary

The functions converting integers into decimal, hexadecimal, and octal,
are all very similar. This patch moves to a combined converter to save
code size.

Diff Detail

Event Timeline

michaelrj created this revision.Aug 5 2022, 3:07 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptAug 5 2022, 3:07 PM
michaelrj requested review of this revision.Aug 5 2022, 3:07 PM
michaelrj updated this revision to Diff 450876.Aug 8 2022, 10:56 AM

rebase to move integer to string tests out of this patch.

LGTM modulo the changes suggested on D131301.

sivachandra added inline comments.Aug 9 2022, 10:49 PM
libc/src/stdio/printf_core/int_converter.h
74–88

A way to avoid this ugliness would be to move this over to an inline function like this:

cpp:optional<cpp::StringView> int_to_str(char conv_name) {
  if (to_lower(conv_name) == 'x') {
    return IntegerToString<uintmax_t, 16>::convert(
        num, bufref, is_lower(to_conv.conv_name));
  } else if (conv_name == 'o') {
    return IntegerToString<uintmax_t, 8>::convert(num, bufref, true);
  } else {
    return IntegerToString<uintmax_t, 10>::convert(num, bufref, true);
  }
}

And then use it as:

auto str = int_to_string(to_conv.conv_name);
if (!str)
  return INT_CONVERSION_ERROR;

Also, feel free to add operator-> to cpp:optional so that you can call str->size() and str->data().

michaelrj marked an inline comment as done.

move optional handling to separate inline function

sivachandra accepted this revision.Aug 10 2022, 12:14 PM
sivachandra added inline comments.
libc/src/stdio/printf_core/int_converter.h
73

Nit: just do if (!str)

This revision is now accepted and ready to land.Aug 10 2022, 12:14 PM
michaelrj updated this revision to Diff 451621.Aug 10 2022, 1:44 PM
michaelrj marked an inline comment as done.

address comment

This revision was landed with ongoing or failed builds.Aug 10 2022, 2:48 PM
This revision was automatically updated to reflect the committed changes.
libc/src/stdio/printf_core/int_converter.h