Skip to content

[libc] Refactor scanf reader to match printf #66023

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
merged 3 commits into from
Sep 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions libc/config/linux/aarch64/entrypoints.txt
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,9 @@ set(TARGET_LIBC_ENTRYPOINTS
libc.src.stdio.snprintf
libc.src.stdio.vsprintf
libc.src.stdio.vsnprintf
#libc.src.stdio.sscanf
#libc.src.stdio.scanf
#libc.src.stdio.fscanf

# sys/mman.h entrypoints
libc.src.sys.mman.madvise
Expand Down
7 changes: 3 additions & 4 deletions libc/config/linux/riscv64/entrypoints.txt
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,9 @@ set(TARGET_LIBC_ENTRYPOINTS
libc.src.stdio.vsnprintf
libc.src.stdio.vfprintf
libc.src.stdio.vprintf
libc.src.stdio.sscanf
libc.src.stdio.scanf
libc.src.stdio.fscanf

# sys/mman.h entrypoints
libc.src.sys.mman.madvise
Expand Down Expand Up @@ -439,10 +442,6 @@ if(LLVM_LIBC_FULL_BUILD)
libc.src.stdio.getc_unlocked
libc.src.stdio.getchar
libc.src.stdio.getchar_unlocked
libc.src.stdio.printf
libc.src.stdio.sscanf
libc.src.stdio.scanf
libc.src.stdio.fscanf
libc.src.stdio.putc
libc.src.stdio.putchar
libc.src.stdio.puts
Expand Down
6 changes: 3 additions & 3 deletions libc/config/linux/x86_64/entrypoints.txt
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,9 @@ set(TARGET_LIBC_ENTRYPOINTS
libc.src.stdio.vsnprintf
libc.src.stdio.vfprintf
libc.src.stdio.vprintf
libc.src.stdio.sscanf
libc.src.stdio.scanf
libc.src.stdio.fscanf

# sys/mman.h entrypoints
libc.src.sys.mman.madvise
Expand Down Expand Up @@ -445,9 +448,6 @@ if(LLVM_LIBC_FULL_BUILD)
libc.src.stdio.getc_unlocked
libc.src.stdio.getchar
libc.src.stdio.getchar_unlocked
libc.src.stdio.sscanf
libc.src.stdio.scanf
libc.src.stdio.fscanf
libc.src.stdio.putc
libc.src.stdio.putchar
libc.src.stdio.puts
Expand Down
4 changes: 2 additions & 2 deletions libc/docs/dev/printf_behavior.rst
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ General Flags:
These compile-time flags will change the behavior of LLVM-libc's printf when it
is compiled. Combinations of flags that are incompatible will be marked.

LIBC_COPT_PRINTF_USE_SYSTEM_FILE
--------------------------------
LIBC_COPT_STDIO_USE_SYSTEM_FILE
-------------------------------
When set, this flag changes fprintf and printf to use the FILE API from the
system's libc, instead of LLVM-libc's internal FILE API. This is set by default
when LLVM-libc is built in overlay mode.
Expand Down
28 changes: 22 additions & 6 deletions libc/src/stdio/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,21 @@ add_entrypoint_object(
libc.src.__support.File.platform_file
)

list(APPEND scanf_deps
libc.src.__support.arg_list
libc.src.stdio.scanf_core.vfscanf_internal
)

if(LLVM_LIBC_FULL_BUILD)
list(APPEND scanf_deps
libc.src.__support.File.file
libc.src.__support.File.platform_file
libc.src.__support.File.platform_stdin
)
else()
list(APPEND scanf_copts "-DLIBC_COPT_STDIO_USE_SYSTEM_FILE")
endif()

add_entrypoint_object(
sscanf
SRCS
Expand All @@ -134,7 +149,6 @@ add_entrypoint_object(
sscanf.h
DEPENDS
libc.src.__support.arg_list
libc.src.stdio.scanf_core.string_reader
libc.src.stdio.scanf_core.reader
libc.src.stdio.scanf_core.scanf_main
)
Expand All @@ -146,8 +160,9 @@ add_entrypoint_object(
HDRS
fscanf.h
DEPENDS
libc.src.__support.arg_list
libc.src.stdio.scanf_core.vfscanf_internal
${scanf_deps}
COMPILE_OPTIONS
${scanf_copts}
)

add_entrypoint_object(
Expand All @@ -157,8 +172,9 @@ add_entrypoint_object(
HDRS
scanf.h
DEPENDS
libc.src.__support.arg_list
libc.src.stdio.scanf_core.vfscanf_internal
${scanf_deps}
COMPILE_OPTIONS
${scanf_copts}
)

add_entrypoint_object(
Expand Down Expand Up @@ -208,7 +224,7 @@ if(LLVM_LIBC_FULL_BUILD)
libc.src.__support.File.platform_stdout
)
else()
list(APPEND printf_copts "-DLIBC_COPT_PRINTF_USE_SYSTEM_FILE")
list(APPEND printf_copts "-DLIBC_COPT_STDIO_USE_SYSTEM_FILE")
endif()

add_entrypoint_object(
Expand Down
6 changes: 3 additions & 3 deletions libc/src/stdio/printf.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@
#include <stdarg.h>
#include <stdio.h>

#ifndef LIBC_COPT_PRINTF_USE_SYSTEM_FILE
#ifndef LIBC_COPT_STDIO_USE_SYSTEM_FILE
#define PRINTF_STDOUT __llvm_libc::stdout
#else // LIBC_COPT_PRINTF_USE_SYSTEM_FILE
#else // LIBC_COPT_STDIO_USE_SYSTEM_FILE
#define PRINTF_STDOUT ::stdout
#endif // LIBC_COPT_PRINTF_USE_SYSTEM_FILE
#endif // LIBC_COPT_STDIO_USE_SYSTEM_FILE

namespace __llvm_libc {

Expand Down
6 changes: 3 additions & 3 deletions libc/src/stdio/printf_core/vfprintf_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
namespace __llvm_libc {

namespace internal {
#ifndef LIBC_COPT_PRINTF_USE_SYSTEM_FILE
#ifndef LIBC_COPT_STDIO_USE_SYSTEM_FILE
LIBC_INLINE int ferror_unlocked(FILE *f) {
return reinterpret_cast<__llvm_libc::File *>(f)->error_unlocked();
}
Expand All @@ -39,7 +39,7 @@ LIBC_INLINE size_t fwrite_unlocked(const void *ptr, size_t size, size_t nmemb,
return reinterpret_cast<__llvm_libc::File *>(f)->write_unlocked(ptr,
size * nmemb);
}
#else // defined(LIBC_COPT_PRINTF_USE_SYSTEM_FILE)
#else // defined(LIBC_COPT_STDIO_USE_SYSTEM_FILE)
LIBC_INLINE int ferror_unlocked(::FILE *f) { return ::ferror_unlocked(f); }

LIBC_INLINE void flockfile(::FILE *f) { ::flockfile(f); }
Expand All @@ -50,7 +50,7 @@ LIBC_INLINE size_t fwrite_unlocked(const void *ptr, size_t size, size_t nmemb,
::FILE *f) {
return ::fwrite_unlocked(ptr, size, nmemb, f);
}
#endif // LIBC_COPT_PRINTF_USE_SYSTEM_FILE
#endif // LIBC_COPT_STDIO_USE_SYSTEM_FILE
} // namespace internal

namespace printf_core {
Expand Down
8 changes: 7 additions & 1 deletion libc/src/stdio/scanf.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@
#include <stdarg.h>
#include <stdio.h>

#ifndef LIBC_COPT_STDIO_USE_SYSTEM_FILE
#define SCANF_STDIN __llvm_libc::stdin
#else // LIBC_COPT_STDIO_USE_SYSTEM_FILE
#define SCANF_STDIN ::stdin
#endif // LIBC_COPT_STDIO_USE_SYSTEM_FILE

namespace __llvm_libc {

LLVM_LIBC_FUNCTION(int, scanf, (const char *__restrict format, ...)) {
Expand All @@ -25,7 +31,7 @@ LLVM_LIBC_FUNCTION(int, scanf, (const char *__restrict format, ...)) {
// destruction automatically.
va_end(vlist);
int ret_val = scanf_core::vfscanf_internal(
reinterpret_cast<::FILE *>(__llvm_libc::stdin), format, args);
reinterpret_cast<::FILE *>(SCANF_STDIN), format, args);
// This is done to avoid including stdio.h in the internals. On most systems
// EOF is -1, so this will be transformed into just "return ret_val".
return (ret_val == -1) ? EOF : ret_val;
Expand Down
40 changes: 10 additions & 30 deletions libc/src/stdio/scanf_core/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,6 @@ add_header_library(
libc.src.__support.CPP.string_view
)

if(NOT (TARGET libc.src.__support.File.file))
# Not all platforms have a file implementation. If file is unvailable,
# then we must skip all the parts that need file.
return()
endif()

add_object_library(
scanf_main
SRCS
Expand All @@ -42,33 +36,14 @@ add_object_library(
libc.src.__support.arg_list
)

add_object_library(
string_reader
SRCS
string_reader.cpp
HDRS
string_reader.h
)

add_object_library(
file_reader
SRCS
file_reader.cpp
HDRS
file_reader.h
DEPENDS
libc.src.__support.File.file
)

add_object_library(
reader
SRCS
reader.cpp
HDRS
reader.h
DEPENDS
.string_reader
.file_reader
libc.src.__support.macros.attributes
)

add_object_library(
Expand Down Expand Up @@ -99,15 +74,20 @@ add_object_library(
libc.src.__support.str_to_float
)

add_object_library(
if(NOT (TARGET libc.src.__support.File.file) AND LLVM_LIBC_FULL_BUILD)
# Not all platforms have a file implementation. If file is unvailable, and a
# full build is requested, then we must skip all file based printf sections.
return()
endif()

add_header_library(
vfscanf_internal
SRCS
vfscanf_internal.cpp
HDRS
vfscanf_internal.h
DEPENDS
.reader
.file_reader
.scanf_main
libc.include.stdio
libc.src.__support.File.file
libc.src.__support.arg_list
)
27 changes: 0 additions & 27 deletions libc/src/stdio/scanf_core/file_reader.cpp

This file was deleted.

39 changes: 0 additions & 39 deletions libc/src/stdio/scanf_core/file_reader.h

This file was deleted.

32 changes: 8 additions & 24 deletions libc/src/stdio/scanf_core/reader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,33 +12,17 @@
namespace __llvm_libc {
namespace scanf_core {

char Reader::getc() {
++cur_chars_read;
if (reader_type == ReaderType::String) {
return string_reader->get_char();
} else {
return file_reader->get_char();
}
}

void Reader::ungetc(char c) {
--cur_chars_read;
if (reader_type == ReaderType::String) {
// The string reader ignores the char c passed to unget since it doesn't
// need to place anything back into a buffer, and modifying the source
// string would be dangerous.
return string_reader->unget_char();
} else {
return file_reader->unget_char(c);
if (rb != nullptr && rb->buff_cur > 0) {
// While technically c should be written back to the buffer, in scanf we
// always write the character that was already there. Additionally, the
// buffer is most likely to contain a string that isn't part of a file,
// which may not be writable.
--(rb->buff_cur);
return;
}
stream_ungetc(static_cast<int>(c), input_stream);
}

bool Reader::has_error() {
if (reader_type == ReaderType::File) {
return file_reader->has_error();
}
return false;
}

} // namespace scanf_core
} // namespace __llvm_libc
Loading