[libc-commits] [libc] [libc]: Clean up unnecessary function pointers in scanf (PR #121215)
Vinay Deshmukh via libc-commits
libc-commits at lists.llvm.org
Sat Jan 18 16:48:00 PST 2025
https://github.com/vinay-deshmukh updated https://github.com/llvm/llvm-project/pull/121215
>From 885d874655d9fb58a74b6c3ecec262278952953d Mon Sep 17 00:00:00 2001
From: Vinay Deshmukh <32487576+vinay-deshmukh at users.noreply.github.com>
Date: Mon, 23 Dec 2024 13:20:20 +0530
Subject: [PATCH 01/14] Initial moves
---
libc/src/stdio/scanf_core/reader.cpp | 61 +++++++++++++++++++-
libc/src/stdio/scanf_core/reader.h | 26 +--------
libc/src/stdio/scanf_core/vfscanf_internal.h | 2 +-
3 files changed, 64 insertions(+), 25 deletions(-)
diff --git a/libc/src/stdio/scanf_core/reader.cpp b/libc/src/stdio/scanf_core/reader.cpp
index ec1f5c098dc7a6..cc09f47709d6b8 100644
--- a/libc/src/stdio/scanf_core/reader.cpp
+++ b/libc/src/stdio/scanf_core/reader.cpp
@@ -10,9 +10,67 @@
#include "src/__support/macros/config.h"
#include <stddef.h>
+#include "src/__support/File/file.h"
+
namespace LIBC_NAMESPACE_DECL {
namespace scanf_core {
+namespace internal {
+
+#if defined(LIBC_TARGET_ARCH_IS_GPU)
+// The GPU build provides FILE access through the host operating system's
+// library. So here we simply use the public entrypoints like in the SYSTEM_FILE
+// interface. Entrypoints should normally not call others, this is an exception.
+// FIXME: We do not acquire any locks here, so this is not thread safe.
+LIBC_INLINE int getc(void *f) {
+ return LIBC_NAMESPACE::getc(reinterpret_cast<::FILE *>(f));
+}
+
+LIBC_INLINE void ungetc(int c, void *f) {
+ LIBC_NAMESPACE::ungetc(c, reinterpret_cast<::FILE *>(f));
+}
+
+#elif !defined(LIBC_COPT_STDIO_USE_SYSTEM_FILE)
+
+LIBC_INLINE int getc(void *f) {
+ unsigned char c;
+ auto result =
+ reinterpret_cast<LIBC_NAMESPACE::File *>(f)->read_unlocked(&c, 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<LIBC_NAMESPACE::File *>(f)->ungetc_unlocked(c);
+}
+
+#else // defined(LIBC_COPT_STDIO_USE_SYSTEM_FILE)
+
+// Since ungetc_unlocked isn't always available, we don't acquire the lock for
+// system files.
+LIBC_INLINE int getc(void *f) { return ::getc(reinterpret_cast<::FILE *>(f)); }
+
+LIBC_INLINE void ungetc(int c, void *f) {
+ ::ungetc(c, reinterpret_cast<::FILE *>(f));
+}
+#endif // LIBC_COPT_STDIO_USE_SYSTEM_FILE
+
+} // namespace internal
+
+char Reader::getc() {
+ ++cur_chars_read;
+ if (rb != nullptr) {
+ char output = rb->buffer[rb->buff_cur];
+ ++(rb->buff_cur);
+ return output;
+ }
+ // This should reset the buffer if applicable.
+ return static_cast<char>(internal::getc(input_stream));
+}
+
void Reader::ungetc(char c) {
--cur_chars_read;
if (rb != nullptr && rb->buff_cur > 0) {
@@ -23,7 +81,8 @@ void Reader::ungetc(char c) {
--(rb->buff_cur);
return;
}
- stream_ungetc(static_cast<int>(c), input_stream);
+ internal::ungetc(static_cast<int>(c), input_stream);
}
+
} // namespace scanf_core
} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/stdio/scanf_core/reader.h b/libc/src/stdio/scanf_core/reader.h
index f984fd93789109..e29ea6ea7d8917 100644
--- a/libc/src/stdio/scanf_core/reader.h
+++ b/libc/src/stdio/scanf_core/reader.h
@@ -16,9 +16,6 @@
namespace LIBC_NAMESPACE_DECL {
namespace scanf_core {
-using StreamGetc = int (*)(void *);
-using StreamUngetc = void (*)(int, void *);
-
// This is intended to be either a raw string or a buffer syncronized with the
// file's internal buffer.
struct ReadBuffer {
@@ -29,38 +26,21 @@ struct ReadBuffer {
class Reader {
ReadBuffer *rb;
-
void *input_stream = nullptr;
-
- // TODO: Remove these unnecessary function pointers
- StreamGetc stream_getc = nullptr;
- StreamUngetc stream_ungetc = nullptr;
-
size_t cur_chars_read = 0;
public:
// TODO: Set buff_len with a proper constant
LIBC_INLINE Reader(ReadBuffer *string_buffer) : rb(string_buffer) {}
- LIBC_INLINE Reader(void *stream, StreamGetc stream_getc_in,
- StreamUngetc stream_ungetc_in,
+ LIBC_INLINE Reader(void *stream,
ReadBuffer *stream_buffer = nullptr)
- : rb(stream_buffer), input_stream(stream), stream_getc(stream_getc_in),
- stream_ungetc(stream_ungetc_in) {}
+ : rb(stream_buffer), input_stream(stream){}
// This returns the next character from the input and advances it by one
// character. When it hits the end of the string or file it returns '\0' to
// signal to stop parsing.
- LIBC_INLINE char getc() {
- ++cur_chars_read;
- if (rb != nullptr) {
- char output = rb->buffer[rb->buff_cur];
- ++(rb->buff_cur);
- return output;
- }
- // This should reset the buffer if applicable.
- return static_cast<char>(stream_getc(input_stream));
- }
+ char getc();
// This moves the input back by one character, placing c into the buffer if
// this is a file reader, else c is ignored.
diff --git a/libc/src/stdio/scanf_core/vfscanf_internal.h b/libc/src/stdio/scanf_core/vfscanf_internal.h
index 67126431fcded5..c96c672bc6fac5 100644
--- a/libc/src/stdio/scanf_core/vfscanf_internal.h
+++ b/libc/src/stdio/scanf_core/vfscanf_internal.h
@@ -103,7 +103,7 @@ LIBC_INLINE int vfscanf_internal(::FILE *__restrict stream,
const char *__restrict format,
internal::ArgList &args) {
internal::flockfile(stream);
- scanf_core::Reader reader(stream, &internal::getc, internal::ungetc);
+ scanf_core::Reader reader(stream);
int retval = scanf_core::scanf_main(&reader, format, args);
if (retval == 0 && internal::ferror_unlocked(stream))
retval = EOF;
>From 12f2ae071b882e25d47b635579583dbcf3886e22 Mon Sep 17 00:00:00 2001
From: Vinay Deshmukh <32487576+vinay-deshmukh at users.noreply.github.com>
Date: Fri, 27 Dec 2024 21:47:08 +0530
Subject: [PATCH 02/14] Remove duplicate definition
---
libc/src/stdio/scanf_core/vfscanf_internal.h | 29 --------------------
1 file changed, 29 deletions(-)
diff --git a/libc/src/stdio/scanf_core/vfscanf_internal.h b/libc/src/stdio/scanf_core/vfscanf_internal.h
index c96c672bc6fac5..84d074711b8fb6 100644
--- a/libc/src/stdio/scanf_core/vfscanf_internal.h
+++ b/libc/src/stdio/scanf_core/vfscanf_internal.h
@@ -38,14 +38,6 @@ LIBC_INLINE void flockfile(::FILE *) { return; }
LIBC_INLINE void funlockfile(::FILE *) { return; }
-LIBC_INLINE int getc(void *f) {
- return LIBC_NAMESPACE::getc(reinterpret_cast<::FILE *>(f));
-}
-
-LIBC_INLINE void ungetc(int c, void *f) {
- LIBC_NAMESPACE::ungetc(c, reinterpret_cast<::FILE *>(f));
-}
-
LIBC_INLINE int ferror_unlocked(::FILE *f) { return LIBC_NAMESPACE::ferror(f); }
#elif !defined(LIBC_COPT_STDIO_USE_SYSTEM_FILE)
@@ -58,21 +50,6 @@ LIBC_INLINE void funlockfile(FILE *f) {
reinterpret_cast<LIBC_NAMESPACE::File *>(f)->unlock();
}
-LIBC_INLINE int getc(void *f) {
- unsigned char c;
- auto result =
- reinterpret_cast<LIBC_NAMESPACE::File *>(f)->read_unlocked(&c, 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<LIBC_NAMESPACE::File *>(f)->ungetc_unlocked(c);
-}
-
LIBC_INLINE int ferror_unlocked(FILE *f) {
return reinterpret_cast<LIBC_NAMESPACE::File *>(f)->error_unlocked();
}
@@ -85,12 +62,6 @@ LIBC_INLINE void flockfile(::FILE *) { return; }
LIBC_INLINE void funlockfile(::FILE *) { return; }
-LIBC_INLINE int getc(void *f) { return ::getc(reinterpret_cast<::FILE *>(f)); }
-
-LIBC_INLINE void ungetc(int c, void *f) {
- ::ungetc(c, reinterpret_cast<::FILE *>(f));
-}
-
LIBC_INLINE int ferror_unlocked(::FILE *f) { return ::ferror(f); }
#endif // LIBC_COPT_STDIO_USE_SYSTEM_FILE
>From 6236e59ed06eb4e333533a092a423372e9f8d6e1 Mon Sep 17 00:00:00 2001
From: Vinay Deshmukh <32487576+vinay-deshmukh at users.noreply.github.com>
Date: Fri, 27 Dec 2024 21:52:04 +0530
Subject: [PATCH 03/14] Doesn't build but should in CI
---
libc/src/stdio/scanf_core/reader.cpp | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/libc/src/stdio/scanf_core/reader.cpp b/libc/src/stdio/scanf_core/reader.cpp
index cc09f47709d6b8..ccfcd220e19c21 100644
--- a/libc/src/stdio/scanf_core/reader.cpp
+++ b/libc/src/stdio/scanf_core/reader.cpp
@@ -6,11 +6,15 @@
//
//===----------------------------------------------------------------------===//
+#include "hdr/types/FILE.h"
#include "src/stdio/scanf_core/reader.h"
#include "src/__support/macros/config.h"
+#include "src/__support/File/file.h"
+
#include <stddef.h>
-#include "src/__support/File/file.h"
+
+
namespace LIBC_NAMESPACE_DECL {
namespace scanf_core {
>From f3b8d7f06befde1a7a3f6aa7e7e506a40334bcfb Mon Sep 17 00:00:00 2001
From: Vinay Deshmukh <32487576+vinay-deshmukh at users.noreply.github.com>
Date: Fri, 27 Dec 2024 22:05:53 +0530
Subject: [PATCH 04/14] clang-format patch
---
libc/src/stdio/scanf_core/reader.cpp | 9 +++------
libc/src/stdio/scanf_core/reader.h | 7 +++----
2 files changed, 6 insertions(+), 10 deletions(-)
diff --git a/libc/src/stdio/scanf_core/reader.cpp b/libc/src/stdio/scanf_core/reader.cpp
index ccfcd220e19c21..e2dcdd5e1e05a6 100644
--- a/libc/src/stdio/scanf_core/reader.cpp
+++ b/libc/src/stdio/scanf_core/reader.cpp
@@ -6,16 +6,13 @@
//
//===----------------------------------------------------------------------===//
-#include "hdr/types/FILE.h"
#include "src/stdio/scanf_core/reader.h"
-#include "src/__support/macros/config.h"
+#include "hdr/types/FILE.h"
#include "src/__support/File/file.h"
+#include "src/__support/macros/config.h"
#include <stddef.h>
-
-
-
namespace LIBC_NAMESPACE_DECL {
namespace scanf_core {
@@ -51,7 +48,7 @@ LIBC_INLINE void ungetc(int c, void *f) {
reinterpret_cast<LIBC_NAMESPACE::File *>(f)->ungetc_unlocked(c);
}
-#else // defined(LIBC_COPT_STDIO_USE_SYSTEM_FILE)
+#else // defined(LIBC_COPT_STDIO_USE_SYSTEM_FILE)
// Since ungetc_unlocked isn't always available, we don't acquire the lock for
// system files.
diff --git a/libc/src/stdio/scanf_core/reader.h b/libc/src/stdio/scanf_core/reader.h
index e29ea6ea7d8917..c8b113b6157956 100644
--- a/libc/src/stdio/scanf_core/reader.h
+++ b/libc/src/stdio/scanf_core/reader.h
@@ -33,14 +33,13 @@ class Reader {
// TODO: Set buff_len with a proper constant
LIBC_INLINE Reader(ReadBuffer *string_buffer) : rb(string_buffer) {}
- LIBC_INLINE Reader(void *stream,
- ReadBuffer *stream_buffer = nullptr)
- : rb(stream_buffer), input_stream(stream){}
+ LIBC_INLINE Reader(void *stream, ReadBuffer *stream_buffer = nullptr)
+ : rb(stream_buffer), input_stream(stream) {}
// This returns the next character from the input and advances it by one
// character. When it hits the end of the string or file it returns '\0' to
// signal to stop parsing.
- char getc();
+ char getc();
// This moves the input back by one character, placing c into the buffer if
// this is a file reader, else c is ignored.
>From bfe6b86d3547c25346d5dd2faca1a2677489ca70 Mon Sep 17 00:00:00 2001
From: Vinay Deshmukh <32487576+vinay-deshmukh at users.noreply.github.com>
Date: Fri, 27 Dec 2024 22:33:40 +0530
Subject: [PATCH 05/14] Put overlay include last
---
libc/src/stdio/scanf_core/reader.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/libc/src/stdio/scanf_core/reader.cpp b/libc/src/stdio/scanf_core/reader.cpp
index e2dcdd5e1e05a6..680c3b768f5d9e 100644
--- a/libc/src/stdio/scanf_core/reader.cpp
+++ b/libc/src/stdio/scanf_core/reader.cpp
@@ -7,10 +7,11 @@
//===----------------------------------------------------------------------===//
#include "src/stdio/scanf_core/reader.h"
-#include "hdr/types/FILE.h"
#include "src/__support/File/file.h"
#include "src/__support/macros/config.h"
+#include "hdr/types/FILE.h"
+
#include <stddef.h>
namespace LIBC_NAMESPACE_DECL {
>From 1f9e27cfc8596057212a67713045f678f61cada6 Mon Sep 17 00:00:00 2001
From: Vinay Deshmukh <32487576+vinay-deshmukh at users.noreply.github.com>
Date: Sat, 28 Dec 2024 20:11:20 +0530
Subject: [PATCH 06/14] Attempt to fix linker error
From:
https://github.com/llvm/llvm-project/actions/runs/12518682283/job/34921546556?pr=121215
Error was:
```
/usr/bin/ld: libc/src/stdio/scanf_core/CMakeFiles/libc.src.stdio.scanf_core.reader.dir/./reader.cpp.o: in function `__llvm_libc_20_0_0_git::scanf_core::Reader::getc()':
reader.cpp:(.text._ZN22__llvm_libc_20_0_0_git10scanf_core6Reader4getcEv+0x38): undefined reference to `__llvm_libc_20_0_0_git::File::read_unlocked(void*, unsigned long)'
/usr/bin/ld: libc/src/stdio/scanf_core/CMakeFiles/libc.src.stdio.scanf_core.reader.dir/./reader.cpp.o: in function `__llvm_libc_20_0_0_git::scanf_core::Reader::ungetc(char)':
reader.cpp:(.text._ZN22__llvm_libc_20_0_0_git10scanf_core6Reader6ungetcEc+0x2c): undefined reference to `__llvm_libc_20_0_0_git::File::ungetc_unlocked(int)'
/usr/bin/ld: libc/test/src/stdio/libc.test.src.stdio.sscanf_test.__unit__.__build__: hidden symbol `_ZN22__llvm_libc_20_0_0_git4File13read_unlockedEPvm' isn't defined
/usr/bin/ld: final link failed: bad value
clang++: error: linker command failed with exit code 1 (use -v to see invocation)
```
---
libc/src/stdio/scanf_core/CMakeLists.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/libc/src/stdio/scanf_core/CMakeLists.txt b/libc/src/stdio/scanf_core/CMakeLists.txt
index a8935d464417c2..8016ac4ec7c58f 100644
--- a/libc/src/stdio/scanf_core/CMakeLists.txt
+++ b/libc/src/stdio/scanf_core/CMakeLists.txt
@@ -62,6 +62,7 @@ add_object_library(
reader.h
DEPENDS
libc.src.__support.macros.attributes
+ libc.hdr.types.FILE
)
add_object_library(
>From 0abb5c3a69d9d8d4045ada78172045f387e977af Mon Sep 17 00:00:00 2001
From: Vinay Deshmukh <32487576+vinay-deshmukh at users.noreply.github.com>
Date: Sat, 28 Dec 2024 22:34:15 +0530
Subject: [PATCH 07/14] try reorder
---
libc/src/stdio/scanf_core/reader.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/libc/src/stdio/scanf_core/reader.cpp b/libc/src/stdio/scanf_core/reader.cpp
index 680c3b768f5d9e..e8ccf000bd4533 100644
--- a/libc/src/stdio/scanf_core/reader.cpp
+++ b/libc/src/stdio/scanf_core/reader.cpp
@@ -7,13 +7,13 @@
//===----------------------------------------------------------------------===//
#include "src/stdio/scanf_core/reader.h"
-#include "src/__support/File/file.h"
#include "src/__support/macros/config.h"
-#include "hdr/types/FILE.h"
-
#include <stddef.h>
+#include "src/__support/File/file.h"
+#include "hdr/types/FILE.h"
+
namespace LIBC_NAMESPACE_DECL {
namespace scanf_core {
>From 13f418d3e449d199fabec1ffe545dc6e0f2a12c7 Mon Sep 17 00:00:00 2001
From: Vinay Deshmukh <32487576+vinay-deshmukh at users.noreply.github.com>
Date: Tue, 7 Jan 2025 20:50:53 -0500
Subject: [PATCH 08/14] Move getc back to header
---
libc/src/stdio/scanf_core/reader.cpp | 11 -----------
libc/src/stdio/scanf_core/reader.h | 11 ++++++++++-
2 files changed, 10 insertions(+), 12 deletions(-)
diff --git a/libc/src/stdio/scanf_core/reader.cpp b/libc/src/stdio/scanf_core/reader.cpp
index e8ccf000bd4533..6de47779e5d191 100644
--- a/libc/src/stdio/scanf_core/reader.cpp
+++ b/libc/src/stdio/scanf_core/reader.cpp
@@ -62,17 +62,6 @@ LIBC_INLINE void ungetc(int c, void *f) {
} // namespace internal
-char Reader::getc() {
- ++cur_chars_read;
- if (rb != nullptr) {
- char output = rb->buffer[rb->buff_cur];
- ++(rb->buff_cur);
- return output;
- }
- // This should reset the buffer if applicable.
- return static_cast<char>(internal::getc(input_stream));
-}
-
void Reader::ungetc(char c) {
--cur_chars_read;
if (rb != nullptr && rb->buff_cur > 0) {
diff --git a/libc/src/stdio/scanf_core/reader.h b/libc/src/stdio/scanf_core/reader.h
index c8b113b6157956..79972fb2470215 100644
--- a/libc/src/stdio/scanf_core/reader.h
+++ b/libc/src/stdio/scanf_core/reader.h
@@ -39,7 +39,16 @@ class Reader {
// This returns the next character from the input and advances it by one
// character. When it hits the end of the string or file it returns '\0' to
// signal to stop parsing.
- char getc();
+ LIBC_INLINE char getc() {
+ ++cur_chars_read;
+ if (rb != nullptr) {
+ char output = rb->buffer[rb->buff_cur];
+ ++(rb->buff_cur);
+ return output;
+ }
+ // This should reset the buffer if applicable.
+ return static_cast<char>(internal::getc(input_stream));
+ }
// This moves the input back by one character, placing c into the buffer if
// this is a file reader, else c is ignored.
>From fe6f638eb7e935b5eb424dee2f1c30dee56460af Mon Sep 17 00:00:00 2001
From: Vinay Deshmukh <32487576+vinay-deshmukh at users.noreply.github.com>
Date: Mon, 13 Jan 2025 19:17:00 -0500
Subject: [PATCH 09/14] WIP ???
---
libc/src/stdio/scanf_core/CMakeLists.txt | 30 ++++++++++
libc/src/stdio/scanf_core/reader.cpp | 64 ++------------------
libc/src/stdio/scanf_core/reader.h | 74 +++++++++++++++++++++++-
3 files changed, 107 insertions(+), 61 deletions(-)
diff --git a/libc/src/stdio/scanf_core/CMakeLists.txt b/libc/src/stdio/scanf_core/CMakeLists.txt
index 8016ac4ec7c58f..d646f3777d9d3e 100644
--- a/libc/src/stdio/scanf_core/CMakeLists.txt
+++ b/libc/src/stdio/scanf_core/CMakeLists.txt
@@ -54,6 +54,32 @@ add_object_library(
libc.src.__support.arg_list
)
+if(LIBC_TARGET_OS_IS_GPU)
+# TODO: fix this
+add_object_library(
+ reader
+ SRCS
+ reader.cpp
+ HDRS
+ reader.h
+ DEPENDS
+ libc.src.__support.macros.attributes
+ libc.src.stdio.getc
+ libc.src.stdio.ungetc
+)
+elseif(NOT (TARGET libc.src.__support.File.file) )
+add_object_library(
+ reader
+ SRCS
+ reader.cpp
+ HDRS
+ reader.h
+ DEPENDS
+ libc.src.__support.macros.attributes
+ libc.hdr.types.FILE
+ ${use_system_file}
+)
+elseif((LLVM_LIBC_FULL_BUILD))
add_object_library(
reader
SRCS
@@ -62,9 +88,13 @@ add_object_library(
reader.h
DEPENDS
libc.src.__support.macros.attributes
+ libc.src.__support.File.file
libc.hdr.types.FILE
+ ${use_system_file}
)
+endif()
+
add_object_library(
converter
SRCS
diff --git a/libc/src/stdio/scanf_core/reader.cpp b/libc/src/stdio/scanf_core/reader.cpp
index 6de47779e5d191..13bac24838b72a 100644
--- a/libc/src/stdio/scanf_core/reader.cpp
+++ b/libc/src/stdio/scanf_core/reader.cpp
@@ -11,69 +11,13 @@
#include <stddef.h>
-#include "src/__support/File/file.h"
-#include "hdr/types/FILE.h"
+// TODO: only include one of these depending on build mode
+// #include "hdr/types/FILE.h"
+// #include "src/__support/File/file.h"
+
namespace LIBC_NAMESPACE_DECL {
namespace scanf_core {
-namespace internal {
-
-#if defined(LIBC_TARGET_ARCH_IS_GPU)
-// The GPU build provides FILE access through the host operating system's
-// library. So here we simply use the public entrypoints like in the SYSTEM_FILE
-// interface. Entrypoints should normally not call others, this is an exception.
-// FIXME: We do not acquire any locks here, so this is not thread safe.
-LIBC_INLINE int getc(void *f) {
- return LIBC_NAMESPACE::getc(reinterpret_cast<::FILE *>(f));
-}
-
-LIBC_INLINE void ungetc(int c, void *f) {
- LIBC_NAMESPACE::ungetc(c, reinterpret_cast<::FILE *>(f));
-}
-
-#elif !defined(LIBC_COPT_STDIO_USE_SYSTEM_FILE)
-
-LIBC_INLINE int getc(void *f) {
- unsigned char c;
- auto result =
- reinterpret_cast<LIBC_NAMESPACE::File *>(f)->read_unlocked(&c, 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<LIBC_NAMESPACE::File *>(f)->ungetc_unlocked(c);
-}
-
-#else // defined(LIBC_COPT_STDIO_USE_SYSTEM_FILE)
-
-// Since ungetc_unlocked isn't always available, we don't acquire the lock for
-// system files.
-LIBC_INLINE int getc(void *f) { return ::getc(reinterpret_cast<::FILE *>(f)); }
-
-LIBC_INLINE void ungetc(int c, void *f) {
- ::ungetc(c, reinterpret_cast<::FILE *>(f));
-}
-#endif // LIBC_COPT_STDIO_USE_SYSTEM_FILE
-
-} // namespace internal
-
-void Reader::ungetc(char c) {
- --cur_chars_read;
- 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;
- }
- internal::ungetc(static_cast<int>(c), input_stream);
-}
-
} // namespace scanf_core
} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/stdio/scanf_core/reader.h b/libc/src/stdio/scanf_core/reader.h
index 79972fb2470215..1b8d430379a9a0 100644
--- a/libc/src/stdio/scanf_core/reader.h
+++ b/libc/src/stdio/scanf_core/reader.h
@@ -9,13 +9,74 @@
#ifndef LLVM_LIBC_SRC_STDIO_SCANF_CORE_READER_H
#define LLVM_LIBC_SRC_STDIO_SCANF_CORE_READER_H
+// has the macro subsitution failure
+// #ifdef LIBC_FULL_BUILD
+#include "src/__support/File/file.h"
+// #endif
+
#include "src/__support/macros/attributes.h" // For LIBC_INLINE
#include "src/__support/macros/config.h"
#include <stddef.h>
+// includes stdio.h
+#include "hdr/types/FILE.h"
+
+
+#if !defined(LIBC_COPT_STDIO_USE_SYSTEM_FILE)
+
+
+#endif
+
namespace LIBC_NAMESPACE_DECL {
namespace scanf_core {
+
+namespace internal {
+
+#if defined(LIBC_TARGET_ARCH_IS_GPU)
+// The GPU build provides FILE access through the host operating system's
+// library. So here we simply use the public entrypoints like in the SYSTEM_FILE
+// interface. Entrypoints should normally not call others, this is an exception.
+// FIXME: We do not acquire any locks here, so this is not thread safe.
+LIBC_INLINE int getc(void *f) {
+ return LIBC_NAMESPACE::getc(reinterpret_cast<::FILE *>(f));
+}
+
+LIBC_INLINE void ungetc(int c, void *f) {
+ LIBC_NAMESPACE::ungetc(c, reinterpret_cast<::FILE *>(f));
+}
+
+#elif !defined(LIBC_COPT_STDIO_USE_SYSTEM_FILE)
+
+LIBC_INLINE int getc(void *f) {
+ unsigned char c;
+ auto result =
+ reinterpret_cast<LIBC_NAMESPACE::File *>(f)->read_unlocked(&c, 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<LIBC_NAMESPACE::File *>(f)->ungetc_unlocked(c);
+}
+
+#else // defined(LIBC_COPT_STDIO_USE_SYSTEM_FILE)
+
+// Since ungetc_unlocked isn't always available, we don't acquire the lock for
+// system files.
+LIBC_INLINE int getc(void *f) { return ::getc(reinterpret_cast<::FILE *>(f)); }
+
+LIBC_INLINE void ungetc(int c, void *f) {
+ ::ungetc(c, reinterpret_cast<::FILE *>(f));
+}
+#endif // LIBC_COPT_STDIO_USE_SYSTEM_FILE
+
+} // namespace internal
+
+
// This is intended to be either a raw string or a buffer syncronized with the
// file's internal buffer.
struct ReadBuffer {
@@ -52,7 +113,18 @@ class Reader {
// This moves the input back by one character, placing c into the buffer if
// this is a file reader, else c is ignored.
- void ungetc(char c);
+ LIBC_INLINE void ungetc(char c) {
+ --cur_chars_read;
+ 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;
+ }
+ internal::ungetc(static_cast<int>(c), input_stream);
+ }
LIBC_INLINE size_t chars_read() { return cur_chars_read; }
};
>From cbe1ab0631043d1257c5935c78afceb20d65a1c5 Mon Sep 17 00:00:00 2001
From: Vinay Deshmukh <32487576+vinay-deshmukh at users.noreply.github.com>
Date: Sat, 18 Jan 2025 18:43:12 -0500
Subject: [PATCH 10/14] it works with COMPILE_OPTIONS param
---
libc/src/stdio/scanf_core/CMakeLists.txt | 16 +++++++++-------
libc/src/stdio/scanf_core/reader.h | 3 +++
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/libc/src/stdio/scanf_core/CMakeLists.txt b/libc/src/stdio/scanf_core/CMakeLists.txt
index d646f3777d9d3e..c85c223c9cbb90 100644
--- a/libc/src/stdio/scanf_core/CMakeLists.txt
+++ b/libc/src/stdio/scanf_core/CMakeLists.txt
@@ -56,10 +56,10 @@ add_object_library(
if(LIBC_TARGET_OS_IS_GPU)
# TODO: fix this
-add_object_library(
+add_header_library(
reader
- SRCS
- reader.cpp
+ # SRCS
+ # reader.cpp
HDRS
reader.h
DEPENDS
@@ -68,15 +68,17 @@ add_object_library(
libc.src.stdio.ungetc
)
elseif(NOT (TARGET libc.src.__support.File.file) )
-add_object_library(
+add_header_library(
reader
- SRCS
- reader.cpp
+ # SRCS
+ # reader.cpp
HDRS
reader.h
DEPENDS
libc.src.__support.macros.attributes
libc.hdr.types.FILE
+ libc.src.__support.File.file
+ COMPILE_OPTIONS
${use_system_file}
)
elseif((LLVM_LIBC_FULL_BUILD))
@@ -90,7 +92,7 @@ add_object_library(
libc.src.__support.macros.attributes
libc.src.__support.File.file
libc.hdr.types.FILE
- ${use_system_file}
+ # ${use_system_file}
)
endif()
diff --git a/libc/src/stdio/scanf_core/reader.h b/libc/src/stdio/scanf_core/reader.h
index 1b8d430379a9a0..fc4659c6b0bc6c 100644
--- a/libc/src/stdio/scanf_core/reader.h
+++ b/libc/src/stdio/scanf_core/reader.h
@@ -30,6 +30,9 @@
namespace LIBC_NAMESPACE_DECL {
namespace scanf_core {
+#ifdef LIBC_COPT_STDIO_USE_SYSTEM_FILE
+#endif
+
namespace internal {
>From 09194c20c7542380aab4f15724675a3bc15a6d4d Mon Sep 17 00:00:00 2001
From: Vinay Deshmukh <32487576+vinay-deshmukh at users.noreply.github.com>
Date: Sat, 18 Jan 2025 19:02:45 -0500
Subject: [PATCH 11/14] clean header file
---
libc/src/stdio/scanf_core/reader.h | 17 +----------------
1 file changed, 1 insertion(+), 16 deletions(-)
diff --git a/libc/src/stdio/scanf_core/reader.h b/libc/src/stdio/scanf_core/reader.h
index fc4659c6b0bc6c..8a826999882b10 100644
--- a/libc/src/stdio/scanf_core/reader.h
+++ b/libc/src/stdio/scanf_core/reader.h
@@ -9,31 +9,16 @@
#ifndef LLVM_LIBC_SRC_STDIO_SCANF_CORE_READER_H
#define LLVM_LIBC_SRC_STDIO_SCANF_CORE_READER_H
-// has the macro subsitution failure
-// #ifdef LIBC_FULL_BUILD
#include "src/__support/File/file.h"
-// #endif
-
#include "src/__support/macros/attributes.h" // For LIBC_INLINE
#include "src/__support/macros/config.h"
-#include <stddef.h>
-
-// includes stdio.h
#include "hdr/types/FILE.h"
+#include <stddef.h>
-#if !defined(LIBC_COPT_STDIO_USE_SYSTEM_FILE)
-
-
-#endif
namespace LIBC_NAMESPACE_DECL {
namespace scanf_core {
-
-#ifdef LIBC_COPT_STDIO_USE_SYSTEM_FILE
-#endif
-
-
namespace internal {
#if defined(LIBC_TARGET_ARCH_IS_GPU)
>From a459d1178ed88cc396e84a2504f9a1117de7cceb Mon Sep 17 00:00:00 2001
From: Vinay Deshmukh <32487576+vinay-deshmukh at users.noreply.github.com>
Date: Sat, 18 Jan 2025 19:04:00 -0500
Subject: [PATCH 12/14] use same logic as vfscanf_internal
---
libc/src/stdio/scanf_core/CMakeLists.txt | 16 +---------------
1 file changed, 1 insertion(+), 15 deletions(-)
diff --git a/libc/src/stdio/scanf_core/CMakeLists.txt b/libc/src/stdio/scanf_core/CMakeLists.txt
index c85c223c9cbb90..1b61292301814a 100644
--- a/libc/src/stdio/scanf_core/CMakeLists.txt
+++ b/libc/src/stdio/scanf_core/CMakeLists.txt
@@ -67,7 +67,7 @@ add_header_library(
libc.src.stdio.getc
libc.src.stdio.ungetc
)
-elseif(NOT (TARGET libc.src.__support.File.file) )
+elseif( (TARGET libc.src.__support.File.file) OR (NOT LLVM_LIBC_FULL_BUILD) )
add_header_library(
reader
# SRCS
@@ -81,20 +81,6 @@ add_header_library(
COMPILE_OPTIONS
${use_system_file}
)
-elseif((LLVM_LIBC_FULL_BUILD))
-add_object_library(
- reader
- SRCS
- reader.cpp
- HDRS
- reader.h
- DEPENDS
- libc.src.__support.macros.attributes
- libc.src.__support.File.file
- libc.hdr.types.FILE
- # ${use_system_file}
-)
-
endif()
add_object_library(
>From 6144b003223d2b706b7e416ff34399a40cab3ff0 Mon Sep 17 00:00:00 2001
From: Vinay Deshmukh <32487576+vinay-deshmukh at users.noreply.github.com>
Date: Sat, 18 Jan 2025 19:16:27 -0500
Subject: [PATCH 13/14] avoid name lookup ambiguity
---
libc/src/stdio/scanf_core/reader.h | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/libc/src/stdio/scanf_core/reader.h b/libc/src/stdio/scanf_core/reader.h
index 8a826999882b10..71bb7f0f9b428a 100644
--- a/libc/src/stdio/scanf_core/reader.h
+++ b/libc/src/stdio/scanf_core/reader.h
@@ -19,7 +19,11 @@
namespace LIBC_NAMESPACE_DECL {
namespace scanf_core {
-namespace internal {
+// We use the name "reader_internal" over "internal" because
+// "internal" causes name lookups in files that include the current header to be ambigious
+// i.e. `internal::foo` in those files, will try to lookup in `LIBC_NAMESPACE::scanf_core::internal` over `LIBC_NAMESPACE::internal`
+// for e.g., `internal::ArgList` in `libc/src/stdio/scanf_core/scanf_main.h`
+namespace reader_internal {
#if defined(LIBC_TARGET_ARCH_IS_GPU)
// The GPU build provides FILE access through the host operating system's
@@ -62,7 +66,7 @@ LIBC_INLINE void ungetc(int c, void *f) {
}
#endif // LIBC_COPT_STDIO_USE_SYSTEM_FILE
-} // namespace internal
+} // namespace reader_internal
// This is intended to be either a raw string or a buffer syncronized with the
@@ -96,7 +100,7 @@ class Reader {
return output;
}
// This should reset the buffer if applicable.
- return static_cast<char>(internal::getc(input_stream));
+ return static_cast<char>(reader_internal::getc(input_stream));
}
// This moves the input back by one character, placing c into the buffer if
@@ -111,7 +115,7 @@ class Reader {
--(rb->buff_cur);
return;
}
- internal::ungetc(static_cast<int>(c), input_stream);
+ reader_internal::ungetc(static_cast<int>(c), input_stream);
}
LIBC_INLINE size_t chars_read() { return cur_chars_read; }
>From 97ae0745861caf3fa6b7b2d7d4dc69f8ad647fc4 Mon Sep 17 00:00:00 2001
From: Vinay Deshmukh <32487576+vinay-deshmukh at users.noreply.github.com>
Date: Sat, 18 Jan 2025 19:47:27 -0500
Subject: [PATCH 14/14] clang-format patch
---
libc/src/stdio/scanf_core/reader.cpp | 1 -
libc/src/stdio/scanf_core/reader.h | 11 +++++------
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/libc/src/stdio/scanf_core/reader.cpp b/libc/src/stdio/scanf_core/reader.cpp
index 13bac24838b72a..2c0bc17f6b2aa0 100644
--- a/libc/src/stdio/scanf_core/reader.cpp
+++ b/libc/src/stdio/scanf_core/reader.cpp
@@ -15,7 +15,6 @@
// #include "hdr/types/FILE.h"
// #include "src/__support/File/file.h"
-
namespace LIBC_NAMESPACE_DECL {
namespace scanf_core {
diff --git a/libc/src/stdio/scanf_core/reader.h b/libc/src/stdio/scanf_core/reader.h
index 71bb7f0f9b428a..a1a8314182da73 100644
--- a/libc/src/stdio/scanf_core/reader.h
+++ b/libc/src/stdio/scanf_core/reader.h
@@ -9,20 +9,20 @@
#ifndef LLVM_LIBC_SRC_STDIO_SCANF_CORE_READER_H
#define LLVM_LIBC_SRC_STDIO_SCANF_CORE_READER_H
+#include "hdr/types/FILE.h"
#include "src/__support/File/file.h"
#include "src/__support/macros/attributes.h" // For LIBC_INLINE
#include "src/__support/macros/config.h"
-#include "hdr/types/FILE.h"
#include <stddef.h>
-
namespace LIBC_NAMESPACE_DECL {
namespace scanf_core {
// We use the name "reader_internal" over "internal" because
-// "internal" causes name lookups in files that include the current header to be ambigious
-// i.e. `internal::foo` in those files, will try to lookup in `LIBC_NAMESPACE::scanf_core::internal` over `LIBC_NAMESPACE::internal`
-// for e.g., `internal::ArgList` in `libc/src/stdio/scanf_core/scanf_main.h`
+// "internal" causes name lookups in files that include the current header to be
+// ambigious i.e. `internal::foo` in those files, will try to lookup in
+// `LIBC_NAMESPACE::scanf_core::internal` over `LIBC_NAMESPACE::internal` for
+// e.g., `internal::ArgList` in `libc/src/stdio/scanf_core/scanf_main.h`
namespace reader_internal {
#if defined(LIBC_TARGET_ARCH_IS_GPU)
@@ -68,7 +68,6 @@ LIBC_INLINE void ungetc(int c, void *f) {
} // namespace reader_internal
-
// This is intended to be either a raw string or a buffer syncronized with the
// file's internal buffer.
struct ReadBuffer {
More information about the libc-commits
mailing list