[clang-tools-extra] [libc] Refactor scanf reader to match printf (PR #66023)

[via cfe-commits]

https://github.com/michaelrj-google resolved 
https://github.com/llvm/llvm-project/pull/66023
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [libc] Refactor scanf reader to match printf (PR #66023)

[via cfe-commits]

https://github.com/michaelrj-google resolved 
https://github.com/llvm/llvm-project/pull/66023
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [libc] Refactor scanf reader to match printf (PR #66023)

[via cfe-commits]

https://github.com/michaelrj-google resolved 
https://github.com/llvm/llvm-project/pull/66023
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [libc] Refactor scanf reader to match printf (PR #66023)

[via cfe-commits]

https://github.com/michaelrj-google resolved 
https://github.com/llvm/llvm-project/pull/66023
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [libc] Refactor scanf reader to match printf (PR #66023)

[via cfe-commits]

https://github.com/michaelrj-google resolved 
https://github.com/llvm/llvm-project/pull/66023
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [libc] Refactor scanf reader to match printf (PR #66023)

[Siva Chandra via cfe-commits]

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

sivachandra wrote:

I think it was removed because it is listed on line 130 also.

https://github.com/llvm/llvm-project/pull/66023
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [libc] Refactor scanf reader to match printf (PR #66023)

[Siva Chandra via cfe-commits]

@@ -9,15 +9,57 @@
 #ifndef LLVM_LIBC_SRC_STDIO_SCANF_CORE_VFSCANF_INTERNAL_H
 #define LLVM_LIBC_SRC_STDIO_SCANF_CORE_VFSCANF_INTERNAL_H
 
+#include "src/__support/File/file.h"
 #include "src/__support/arg_list.h"
+#include "src/stdio/scanf_core/reader.h"
+#include "src/stdio/scanf_core/scanf_main.h"
 
 #include 
 
 namespace __llvm_libc {
+
+namespace internal {
+#ifndef LIBC_COPT_SCANF_USE_SYSTEM_FILE

sivachandra wrote:

Nit: Add a line before and after.

https://github.com/llvm/llvm-project/pull/66023
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [libc] Refactor scanf reader to match printf (PR #66023)

[Siva Chandra via cfe-commits]

@@ -220,6 +220,20 @@ add_libc_test(
 libc.src.stdio.vprintf
 )
 
+
+if(LLVM_LIBC_FULL_BUILD)
+  # In fullbuild mode, fscanf's tests use the internal FILE for other 
functions.
+  list(APPEND fscanf_test_deps
+   libc.src.stdio.fclose
+   libc.src.stdio.ferror
+   libc.src.stdio.fopen
+   libc.src.stdio.fwrite
+  )
+else()
+# Else in overlay mode they use the system's FILE.
+ set(fscanf_test_copts "-DLIBC_COPT_SCANF_USE_SYSTEM_FILE")

sivachandra wrote:

This opt should likely be named as `LIBC_COPT_STDIO_USE_SYSTEM_FILE` and shared 
between `printf`, `scanf` and all other stdio pieces which can work with both 
system file or libc's own file.

https://github.com/llvm/llvm-project/pull/66023
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [libc] Refactor scanf reader to match printf (PR #66023)

[Siva Chandra via cfe-commits]

https://github.com/sivachandra commented:

Few nits, but otherwise OK.

https://github.com/llvm/llvm-project/pull/66023
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [libc] Refactor scanf reader to match printf (PR #66023)

[Siva Chandra via cfe-commits]

https://github.com/sivachandra edited 
https://github.com/llvm/llvm-project/pull/66023
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [libc] Refactor scanf reader to match printf (PR #66023)

[Siva Chandra via cfe-commits]

@@ -9,15 +9,57 @@
 #ifndef LLVM_LIBC_SRC_STDIO_SCANF_CORE_VFSCANF_INTERNAL_H
 #define LLVM_LIBC_SRC_STDIO_SCANF_CORE_VFSCANF_INTERNAL_H
 
+#include "src/__support/File/file.h"
 #include "src/__support/arg_list.h"
+#include "src/stdio/scanf_core/reader.h"
+#include "src/stdio/scanf_core/scanf_main.h"
 
 #include 
 
 namespace __llvm_libc {
+
+namespace internal {
+#ifndef LIBC_COPT_SCANF_USE_SYSTEM_FILE
+LIBC_INLINE int getc(void *f) {
+  unsigned char c;
+  auto result = reinterpret_cast<__llvm_libc::File *>(f)->read_unlocked(, 1);
+  size_t r = result.value;
+  if (result.has_error() || r != 1) {
+return '\0';
+  }
+  return c;
+}
+
+LIBC_INLINE void ungetc(int c, void *f) {
+  reinterpret_cast<__llvm_libc::File *>(f)->ungetc(c);
+}
+
+LIBC_INLINE int ferror_unlocked(FILE *f) {
+  return reinterpret_cast<__llvm_libc::File *>(f)->error_unlocked();
+}
+#else  // defined(LIBC_COPT_PRINTF_USE_SYSTEM_FILE)

sivachandra wrote:

Same.

https://github.com/llvm/llvm-project/pull/66023
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [libc] Refactor scanf reader to match printf (PR #66023)

[Joseph Huber via cfe-commits]

https://github.com/jhuber6 commented:

The changes make sense to me overall with some nits.

https://github.com/llvm/llvm-project/pull/66023
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [libc] Refactor scanf reader to match printf (PR #66023)

[Joseph Huber via cfe-commits]

@@ -9,44 +9,61 @@
 #ifndef LLVM_LIBC_SRC_STDIO_SCANF_CORE_READER_H
 #define LLVM_LIBC_SRC_STDIO_SCANF_CORE_READER_H
 
-#include "src/stdio/scanf_core/file_reader.h"
-#include "src/stdio/scanf_core/string_reader.h"
+#include "src/__support/macros/attributes.h" // For LIBC_INLINE
 #include 
 
 namespace __llvm_libc {
 namespace scanf_core {
 
-enum class ReaderType { String, File };
+using StreamGetc = int (*)(void *);
+using StreamUngetc = void (*)(int, void *);

jhuber6 wrote:

Could we do this with templates instead? Then we'd presumably just specialize 
it for whether or not the user it `sscanf` or `fscanf` in the implementation.

https://github.com/llvm/llvm-project/pull/66023
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [libc] Refactor scanf reader to match printf (PR #66023)

[Joseph Huber via cfe-commits]

@@ -126,6 +126,10 @@ set(TARGET_LIBC_ENTRYPOINTS
 libc.src.stdio.snprintf
 libc.src.stdio.vsprintf
 libc.src.stdio.vsnprintf
+#TODO: Check if scanf can be enabled on aarch64

jhuber6 wrote:

Seems like this should be tested first.

https://github.com/llvm/llvm-project/pull/66023
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [libc] Refactor scanf reader to match printf (PR #66023)

[Joseph Huber via cfe-commits]

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

jhuber6 wrote:

Does this accidentally delete `printf`?

https://github.com/llvm/llvm-project/pull/66023
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits