[libc-commits] [libc] 00f5aaf - [libc]: Clean up unnecessary function pointers in scanf (#121215)
via libc-commits
libc-commits at lists.llvm.org
Thu Feb 20 10:08:30 PST 2025
Author: Vinay Deshmukh
Date: 2025-02-20T10:08:26-08:00
New Revision: 00f5aaf841cbd6e2df9158538e36d66632af9cd5
URL: https://github.com/llvm/llvm-project/commit/00f5aaf841cbd6e2df9158538e36d66632af9cd5
DIFF: https://github.com/llvm/llvm-project/commit/00f5aaf841cbd6e2df9158538e36d66632af9cd5.diff
LOG: [libc]: Clean up unnecessary function pointers in scanf (#121215)
Resolves #115394
1. Move definitions of cross-platform `getc` `ungetc` to `reader.h`.
2. Remove function pointer members to define them once per platform in
`.h`
3. Built in overlay mode in macOS m1
4. Remove `reader.cpp` as it's empty now
Also, full build doesn't yet build on macos m1 AFAIK
Added:
Modified:
libc/src/stdio/scanf_core/CMakeLists.txt
libc/src/stdio/scanf_core/reader.h
libc/src/stdio/scanf_core/vfscanf_internal.h
Removed:
libc/src/stdio/scanf_core/reader.cpp
################################################################################
diff --git a/libc/src/stdio/scanf_core/CMakeLists.txt b/libc/src/stdio/scanf_core/CMakeLists.txt
index a8935d464417c..35b8b3d318a9f 100644
--- a/libc/src/stdio/scanf_core/CMakeLists.txt
+++ b/libc/src/stdio/scanf_core/CMakeLists.txt
@@ -54,15 +54,26 @@ add_object_library(
libc.src.__support.arg_list
)
-add_object_library(
+if(LIBC_TARGET_OS_IS_GPU)
+add_header_library(
+ reader
+ HDRS
+ reader.h
+ DEPENDS
+ libc.src.__support.macros.attributes
+)
+elseif((TARGET libc.src.__support.File.file) OR (NOT LLVM_LIBC_FULL_BUILD))
+add_header_library(
reader
- SRCS
- reader.cpp
HDRS
reader.h
DEPENDS
libc.src.__support.macros.attributes
+ libc.hdr.types.FILE
+ libc.src.__support.File.file
+ ${use_system_file}
)
+endif()
add_object_library(
converter
diff --git a/libc/src/stdio/scanf_core/reader.cpp b/libc/src/stdio/scanf_core/reader.cpp
deleted file mode 100644
index ec1f5c098dc7a..0000000000000
--- a/libc/src/stdio/scanf_core/reader.cpp
+++ /dev/null
@@ -1,29 +0,0 @@
-//===-- Reader definition for scanf -----------------------------*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#include "src/stdio/scanf_core/reader.h"
-#include "src/__support/macros/config.h"
-#include <stddef.h>
-
-namespace LIBC_NAMESPACE_DECL {
-namespace scanf_core {
-
-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;
- }
- stream_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 f984fd9378910..a545a605ff150 100644
--- a/libc/src/stdio/scanf_core/reader.h
+++ b/libc/src/stdio/scanf_core/reader.h
@@ -9,15 +9,68 @@
#ifndef LLVM_LIBC_SRC_STDIO_SCANF_CORE_READER_H
#define LLVM_LIBC_SRC_STDIO_SCANF_CORE_READER_H
+#include "hdr/types/FILE.h"
+
+#ifndef LIBC_COPT_STDIO_USE_SYSTEM_FILE
+#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>
namespace LIBC_NAMESPACE_DECL {
namespace scanf_core {
-
-using StreamGetc = int (*)(void *);
-using StreamUngetc = void (*)(int, void *);
+// 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
+// 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 reader_internal
// This is intended to be either a raw string or a buffer syncronized with the
// file's internal buffer.
@@ -29,24 +82,15 @@ 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,
- ReadBuffer *stream_buffer = nullptr)
- : rb(stream_buffer), input_stream(stream), stream_getc(stream_getc_in),
- stream_ungetc(stream_ungetc_in) {}
+ 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
@@ -59,12 +103,23 @@ class Reader {
return output;
}
// This should reset the buffer if applicable.
- return static_cast<char>(stream_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
// 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;
+ }
+ reader_internal::ungetc(static_cast<int>(c), input_stream);
+ }
LIBC_INLINE size_t chars_read() { return cur_chars_read; }
};
diff --git a/libc/src/stdio/scanf_core/vfscanf_internal.h b/libc/src/stdio/scanf_core/vfscanf_internal.h
index 67126431fcded..84d074711b8fb 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
@@ -103,7 +74,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;
More information about the libc-commits
mailing list