[libc] Refactor scanf reader to match printf (#66023)

In a previous patch, the printf writer was rewritten to use a single
writer class with a buffer and a callback hook. This patch refactors
scanf's reader to match conceptually.

Author: michaelrj-google

libc/config/linux/aarch64/entrypoints.txt

@@ -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

libc/config/linux/riscv64/entrypoints.txt

@@ -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
@@ -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

libc/config/linux/x86_64/entrypoints.txt

@@ -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
@@ -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

libc/docs/dev/printf_behavior.rst

@@ -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.

libc/src/stdio/CMakeLists.txt

@@ -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
@@ -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
 )
@@ -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(
@@ -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(
@@ -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(

libc/src/stdio/printf.cpp

@@ -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 {
 

libc/src/stdio/printf_core/vfprintf_internal.h

@@ -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();
 }
@@ -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); }
@@ -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 {

libc/src/stdio/scanf.cpp

@@ -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, ...)) {
@@ -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;

libc/src/stdio/scanf_core/CMakeLists.txt

@@ -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
@@ -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(
@@ -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
 )

libc/src/stdio/scanf_core/file_reader.cpp

@@ -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