Skip to content
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

Add bit width length modifier to printf #81685

Closed
2 of 12 tasks
michaelrj-google opened this issue Feb 13, 2024 · 7 comments · Fixed by #82461
Closed
2 of 12 tasks

Add bit width length modifier to printf #81685

michaelrj-google opened this issue Feb 13, 2024 · 7 comments · Fixed by #82461
Assignees
Labels
good first issue https://github.com/llvm/llvm-project/contribute libc

Comments

@michaelrj-google
Copy link
Contributor

michaelrj-google commented Feb 13, 2024

Specified in C23 on page 349 of this PDF: https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3096.pdf

C23 adds a new length modifier to printf: w_N_ and wf_N_ which allow the user to specify an exact bit width for an integer conversion. To support this, LLVM-libc needs to add support to the printf parser and converter, as well as the internal data types. Below is a (hopefully complete) list of tasks that will add this support. Feel free to reach out if you need help, you can find me as michaelrj on discord, or you can just leave a comment on this bug.

Code:

In core_structs.h:

  • add w and wf to the LengthModifier enum
  • add a member to FormatSection to hold the bit width (size_t would be an appropriate type)
  • add both of these to the operator==.

In parser.h

  • add w and wf to the parse_length_modifier function. This may also involve changing it to return a struct that contains a LengthModifier and a number.
  • add the new length modifiers to the switch statement in get_next_section
  • add the new length modifiers to the switch statement in get_type_desc

In converter_utils.h

  • add w and wf to apply_length_modifier (if you've already defined a struct that is a LengthModifier and a number, this would probably be another good place to use it).
  • remember that the number of bits doesn't have to be a power of two, and can be any number (though you can assume it's small enough to fit in an intmax_t)

In write_int_converter.h

  • add w and wf to the switch statement in convert_write_int

Testing:

In sprintf_test.cpp

  • add tests for the length modifiers for the format specifiers that support them. This can be a new test, or added to the existing integer type tests.

In PrintfMatcher.cpp

  • Add support for printing the new length modifiers (feel free to ask for help with this, it can be fiddly)

In parser_test.cpp

  • add a test case for each of the new length modifiers
@michaelrj-google michaelrj-google added good first issue https://github.com/llvm/llvm-project/contribute libc labels Feb 13, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 13, 2024

@llvm/issue-subscribers-libc

Author: None (michaelrj-google)

Specified in C23 on page 349 of this PDF: https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3096.pdf

C23 adds a new length modifier to printf: w_N_ and wf_N_ which allow the user to specify an exact bit width for an integer conversion. To support this, LLVM-libc needs to add support to the printf parser and converter, as well as the internal data types. Below is a (hopefully complete) list of tasks that will add this support. Feel free to reach out if you need help, you can find me as michaelrj on discord, or you can just leave a comment on this bug.

Code:

In core_structs.h:

  • add w and wf to the LengthModifier enum
  • add a member to FormatSection to hold the bit width (size_t would be an appropriate type)
  • add both of these to the operator==.

In parser.h

  • add w and wf to the parse_length_modifier function. This may also involve changing it to return a struct that contains a LengthModifier and a number.
  • add the new length modifiers to the switch statement in get_next_section
  • add the new length modifiers to the switch statement in get_type_desc

In converter_utils.h

  • add w and wf to apply_length_modifier (if you've already defined a struct that is a LengthModifier and a number, this would probably be another good place to use it).
  • remember that the number of bits doesn't have to be a power of two, and can be any number (though you can assume it's small enough to fit in an intmax_t)

In write_int_converter.h

  • add w and wf to the switch statement in convert_write_int

Testing:

In sprintf_test.cpp

  • add tests for the length modifiers for the format specifiers that support them. This can be a new test, or added to the existing integer type tests.

In PrintfMatcher.cpp

  • Add support for printing the new length modifiers (feel free to ask for help with this, it can be fiddly)

In parser_test.cpp

  • add a test case for each of the new length modifiers

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 13, 2024

Hi!

This issue may be a good introductory issue for people new to working on LLVM. If you would like to work on this issue, your first steps are:

  1. In the comments of the issue, request for it to be assigned to you.
  2. Fix the issue locally.
  3. Run the test suite locally. Remember that the subdirectories under test/ create fine-grained testing targets, so you can e.g. use make check-clang-ast to only run Clang's AST tests.
  4. Create a Git commit.
  5. Run git clang-format HEAD~1 to format your changes.
  6. Open a pull request to the upstream repository on GitHub. Detailed instructions can be found in GitHub's documentation.

If you have any further questions about this issue, don't hesitate to ask via a comment in the thread below.

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 13, 2024

@llvm/issue-subscribers-good-first-issue

Author: None (michaelrj-google)

Specified in C23 on page 349 of this PDF: https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3096.pdf

C23 adds a new length modifier to printf: w_N_ and wf_N_ which allow the user to specify an exact bit width for an integer conversion. To support this, LLVM-libc needs to add support to the printf parser and converter, as well as the internal data types. Below is a (hopefully complete) list of tasks that will add this support. Feel free to reach out if you need help, you can find me as michaelrj on discord, or you can just leave a comment on this bug.

Code:

In core_structs.h:

  • add w and wf to the LengthModifier enum
  • add a member to FormatSection to hold the bit width (size_t would be an appropriate type)
  • add both of these to the operator==.

In parser.h

  • add w and wf to the parse_length_modifier function. This may also involve changing it to return a struct that contains a LengthModifier and a number.
  • add the new length modifiers to the switch statement in get_next_section
  • add the new length modifiers to the switch statement in get_type_desc

In converter_utils.h

  • add w and wf to apply_length_modifier (if you've already defined a struct that is a LengthModifier and a number, this would probably be another good place to use it).
  • remember that the number of bits doesn't have to be a power of two, and can be any number (though you can assume it's small enough to fit in an intmax_t)

In write_int_converter.h

  • add w and wf to the switch statement in convert_write_int

Testing:

In sprintf_test.cpp

  • add tests for the length modifiers for the format specifiers that support them. This can be a new test, or added to the existing integer type tests.

In PrintfMatcher.cpp

  • Add support for printing the new length modifiers (feel free to ask for help with this, it can be fiddly)

In parser_test.cpp

  • add a test case for each of the new length modifiers

@omprakaash
Copy link
Contributor

Hey I am new to LLVM, and was wondering if I could take up this issue.

@EugeneZelenko
Copy link
Contributor

@omprakaash: Just create pull request and mention it on this page.

@SahilPatidar
Copy link
Contributor

@omprakaash,
are you working on this?

@omprakaash
Copy link
Contributor

Yep. I am. Should be done pretty soon.

michaelrj-google pushed a commit that referenced this issue Mar 29, 2024
Resolves #81685. This adds support for wN and wfN length modifiers in
fprintf.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue https://github.com/llvm/llvm-project/contribute libc
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants