llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.07k stars 11.99k forks source link

[libc] Clean up unnecessary function pointers in scanf #115394

Open michaelrj-google opened 6 days ago

michaelrj-google commented 6 days ago

The scanf reader is the abstraction that scanf uses to read from either a file or a string, similar to printf's writer. Unlike printf, however, there are only two variants of scanf: fscanf and sscanf (just scanf is the same as fscanf(stdin, ...)). This means that the stream_getc and stream_ungetc functions don't need to be pointers, they can be declared with using statements based on the current platform.

Files you'll need to edit: libc/src/stdio/scanf_core/vfscanf_internal.h libc/src/stdio/scanf_core/reader.h

What to do: Move the definitions of getc and ungetc in vfscanf_internal into reader.h (along with the associated #if defined logic for each target). Define the stream_getc and stream_ungetc functions with using statements inside of class Reader (or do something equivalent). Clean up the relevant code.

If you have questions or need help, feel free to comment on this issue or reach out via the LLVM discord.

llvmbot commented 6 days ago

@llvm/issue-subscribers-libc

Author: Michael Jones (michaelrj-google)

The scanf reader is the abstraction that scanf uses to read from either a file or a string, similar to printf's writer. Unlike printf, however, there are only two variants of scanf: `fscanf` and `sscanf` (just `scanf` is the same as `fscanf(stdin, ...)`). This means that the `stream_getc` and `stream_ungetc` functions don't need to be pointers, they can be declared with `using` statements based on the current platform. Files you'll need to edit: [libc/src/stdio/scanf_core/vfscanf_internal.h](https://github.com/llvm/llvm-project/blob/main/libc/src/stdio/scanf_core/vfscanf_internal.h) [libc/src/stdio/scanf_core/reader.h](https://github.com/llvm/llvm-project/blob/main/libc/src/stdio/scanf_core/reader.h) What to do: Move the definitions of `getc` and `ungetc` in `vfscanf_internal` into `reader.h` (along with the associated `#if defined` logic for each target). Define the `stream_getc` and `stream_ungetc` functions with `using` statements inside of `class Reader` (or do something equivalent). Clean up the relevant code. If you have questions or need help, feel free to comment on this issue or reach out via the LLVM discord.
llvmbot commented 6 days ago

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. Check that no other contributor has already been assigned to this issue. If you believe that no one is actually working on it despite an assignment, ping the person. After one week without a response, the assignee may be changed.
  2. In the comments of this issue, request for it to be assigned to you, or just create a pull request after following the steps below. Mention this issue in the description of the pull request.
  3. Fix the issue locally.
  4. 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.
  5. Create a Git commit.
  6. Run git clang-format HEAD~1 to format your changes.
  7. Open a pull request to the upstream repository on GitHub. Detailed instructions can be found in GitHub's documentation. Mention this issue in the description of the pull request.

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

llvmbot commented 6 days ago

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

Author: Michael Jones (michaelrj-google)

The scanf reader is the abstraction that scanf uses to read from either a file or a string, similar to printf's writer. Unlike printf, however, there are only two variants of scanf: `fscanf` and `sscanf` (just `scanf` is the same as `fscanf(stdin, ...)`). This means that the `stream_getc` and `stream_ungetc` functions don't need to be pointers, they can be declared with `using` statements based on the current platform. Files you'll need to edit: [libc/src/stdio/scanf_core/vfscanf_internal.h](https://github.com/llvm/llvm-project/blob/main/libc/src/stdio/scanf_core/vfscanf_internal.h) [libc/src/stdio/scanf_core/reader.h](https://github.com/llvm/llvm-project/blob/main/libc/src/stdio/scanf_core/reader.h) What to do: Move the definitions of `getc` and `ungetc` in `vfscanf_internal` into `reader.h` (along with the associated `#if defined` logic for each target). Define the `stream_getc` and `stream_ungetc` functions with `using` statements inside of `class Reader` (or do something equivalent). Clean up the relevant code. If you have questions or need help, feel free to comment on this issue or reach out via the LLVM discord.
shawbyoung commented 6 days ago

Could I be assigned this? @michaelrj-google