Skip to content
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

[libc] Implement strfromd() and strfroml() #86113

Merged
merged 1 commit into from Mar 22, 2024

Conversation

vinayakdsci
Copy link
Contributor

@vinayakdsci vinayakdsci commented Mar 21, 2024

Follow up to #85438.

Implements the functions strfromd() and strfroml() introduced in C23, and unifies the testing framework for strfrom*() functions.

@llvmbot llvmbot added the libc label Mar 21, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 21, 2024

@llvm/pr-subscribers-libc

Author: Vinayak Dev (vinayakdsci)

Changes

Follow up to #85438.

Implements the functions strfromd() and strfroml() introduced in C23, and makes changes in strfromf_test.cpp to make all the strfrom*_test files consistent with each other.


Patch is 27.84 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/86113.diff

12 Files Affected:

  • (modified) libc/config/linux/x86_64/entrypoints.txt (+2)
  • (modified) libc/spec/stdc.td (+2)
  • (modified) libc/src/stdlib/CMakeLists.txt (+20)
  • (added) libc/src/stdlib/strfromd.cpp (+42)
  • (added) libc/src/stdlib/strfromd.h (+21)
  • (modified) libc/src/stdlib/strfromf.h (+1-1)
  • (added) libc/src/stdlib/strfroml.cpp (+44)
  • (added) libc/src/stdlib/strfroml.h (+22)
  • (modified) libc/test/src/stdlib/CMakeLists.txt (+20)
  • (added) libc/test/src/stdlib/strfromd_test.cpp (+224)
  • (modified) libc/test/src/stdlib/strfromf_test.cpp (+51-46)
  • (added) libc/test/src/stdlib/strfroml_test.cpp (+159)
diff --git a/libc/config/linux/x86_64/entrypoints.txt b/libc/config/linux/x86_64/entrypoints.txt
index c216f43496270b..e863ba56604e77 100644
--- a/libc/config/linux/x86_64/entrypoints.txt
+++ b/libc/config/linux/x86_64/entrypoints.txt
@@ -180,7 +180,9 @@ set(TARGET_LIBC_ENTRYPOINTS
     libc.src.stdlib.qsort_r
     libc.src.stdlib.rand
     libc.src.stdlib.srand
+    libc.src.stdlib.strfromd
     libc.src.stdlib.strfromf
+    libc.src.stdlib.strfroml
     libc.src.stdlib.strtod
     libc.src.stdlib.strtof
     libc.src.stdlib.strtol
diff --git a/libc/spec/stdc.td b/libc/spec/stdc.td
index 920036adfed5c1..79c65738142436 100644
--- a/libc/spec/stdc.td
+++ b/libc/spec/stdc.td
@@ -956,7 +956,9 @@ def StdC : StandardSpec<"stdc"> {
           FunctionSpec<"rand", RetValSpec<IntType>, [ArgSpec<VoidType>]>,
           FunctionSpec<"srand", RetValSpec<VoidType>, [ArgSpec<UnsignedIntType>]>,
 
+          FunctionSpec<"strfromd", RetValSpec<IntType>, [ArgSpec<CharRestrictedPtr>, ArgSpec<SizeTType>, ArgSpec<ConstCharRestrictedPtr>, ArgSpec<DoubleType>]>,
           FunctionSpec<"strfromf", RetValSpec<IntType>, [ArgSpec<CharRestrictedPtr>, ArgSpec<SizeTType>, ArgSpec<ConstCharRestrictedPtr>, ArgSpec<FloatType>]>,
+          FunctionSpec<"strfroml", RetValSpec<IntType>, [ArgSpec<CharRestrictedPtr>, ArgSpec<SizeTType>, ArgSpec<ConstCharRestrictedPtr>, ArgSpec<LongDoubleType>]>,
 
           FunctionSpec<"strtof", RetValSpec<FloatType>, [ArgSpec<ConstCharRestrictedPtr>, ArgSpec<CharRestrictedPtrPtr>]>,
           FunctionSpec<"strtod", RetValSpec<DoubleType>, [ArgSpec<ConstCharRestrictedPtr>, ArgSpec<CharRestrictedPtrPtr>]>,
diff --git a/libc/src/stdlib/CMakeLists.txt b/libc/src/stdlib/CMakeLists.txt
index 22f7f990fb08a8..a2b16e73f35321 100644
--- a/libc/src/stdlib/CMakeLists.txt
+++ b/libc/src/stdlib/CMakeLists.txt
@@ -62,6 +62,26 @@ add_entrypoint_object(
     .str_from_util
 )
 
+add_entrypoint_object(
+  strfromd
+  SRCS
+    strfromd.cpp
+  HDRS
+    strfromd.h
+  DEPENDS
+    .str_from_util
+)
+
+add_entrypoint_object(
+  strfroml
+  SRCS
+    strfroml.cpp
+  HDRS
+    strfroml.h
+  DEPENDS
+    .str_from_util
+)
+
 add_header_library(
   str_from_util
   HDRS
diff --git a/libc/src/stdlib/strfromd.cpp b/libc/src/stdlib/strfromd.cpp
new file mode 100644
index 00000000000000..1d02a7ad112442
--- /dev/null
+++ b/libc/src/stdlib/strfromd.cpp
@@ -0,0 +1,42 @@
+//===-- Implementation of strfromd ------------------------------*- 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/stdlib/strfromd.h"
+#include "src/stdlib/str_from_util.h"
+
+#include <stdarg.h>
+#include <stddef.h>
+
+namespace LIBC_NAMESPACE {
+
+LLVM_LIBC_FUNCTION(int, strfromd,
+                   (char *__restrict s, size_t n, const char *__restrict format,
+                    double fp)) {
+  LIBC_ASSERT(s != nullptr);
+
+  printf_core::FormatSection section =
+      internal::parse_format_string(format, fp);
+  printf_core::WriteBuffer wb(s, (n > 0 ? n - 1 : 0));
+  printf_core::Writer writer(&wb);
+
+  int result = 0;
+  if (section.has_conv)
+    result = internal::strfromfloat_convert<double>(&writer, section);
+  else
+    result = writer.write(section.raw_string);
+
+  if (result < 0)
+    return result;
+
+  if (n > 0)
+    wb.buff[wb.buff_cur] = '\0';
+
+  return writer.get_chars_written();
+}
+
+} // namespace LIBC_NAMESPACE
diff --git a/libc/src/stdlib/strfromd.h b/libc/src/stdlib/strfromd.h
new file mode 100644
index 00000000000000..d2c3fefb6300dc
--- /dev/null
+++ b/libc/src/stdlib/strfromd.h
@@ -0,0 +1,21 @@
+//===-- Implementation header for strfromd ------------------------*- 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_STDLIB_STRFROMD_H
+#define LLVM_LIBC_SRC_STDLIB_STRFROMD_H
+
+#include <stddef.h>
+
+namespace LIBC_NAMESPACE {
+
+int strfromd(char *__restrict s, size_t n, const char *__restrict format,
+             double fp);
+
+} // namespace LIBC_NAMESPACE
+
+#endif // LLVM_LIBC_SRC_STDLIB_STRFROMD_H
diff --git a/libc/src/stdlib/strfromf.h b/libc/src/stdlib/strfromf.h
index b551a58af05a3e..492c2c33cf080b 100644
--- a/libc/src/stdlib/strfromf.h
+++ b/libc/src/stdlib/strfromf.h
@@ -18,4 +18,4 @@ int strfromf(char *__restrict s, size_t n, const char *__restrict format,
 
 } // namespace LIBC_NAMESPACE
 
-#endif // LLVM_LIBC_SRC_STDLIB_STRTOF_H
+#endif // LLVM_LIBC_SRC_STDLIB_STRFROMF_H
diff --git a/libc/src/stdlib/strfroml.cpp b/libc/src/stdlib/strfroml.cpp
new file mode 100644
index 00000000000000..40459651353077
--- /dev/null
+++ b/libc/src/stdlib/strfroml.cpp
@@ -0,0 +1,44 @@
+//===-- Implementation of strfroml ------------------------------*- 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/stdlib/strfroml.h"
+#include "src/stdlib/str_from_util.h"
+
+#include <stdarg.h>
+#include <stddef.h>
+
+namespace LIBC_NAMESPACE {
+
+LLVM_LIBC_FUNCTION(int, strfroml,
+                   (char *__restrict s, size_t n, const char *__restrict format,
+                    long double fp)) {
+  LIBC_ASSERT(s != nullptr);
+
+  printf_core::FormatSection section =
+      internal::parse_format_string(format, fp);
+  section.length_modifier = printf_core::LengthModifier::L;
+
+  printf_core::WriteBuffer wb(s, (n > 0 ? n - 1 : 0));
+  printf_core::Writer writer(&wb);
+
+  int result = 0;
+  if (section.has_conv)
+    result = internal::strfromfloat_convert<long double>(&writer, section);
+  else
+    result = writer.write(section.raw_string);
+
+  if (result < 0)
+    return result;
+
+  if (n > 0)
+    wb.buff[wb.buff_cur] = '\0';
+
+  return writer.get_chars_written();
+}
+
+} // namespace LIBC_NAMESPACE
diff --git a/libc/src/stdlib/strfroml.h b/libc/src/stdlib/strfroml.h
new file mode 100644
index 00000000000000..05f939e187b885
--- /dev/null
+++ b/libc/src/stdlib/strfroml.h
@@ -0,0 +1,22 @@
+
+//===-- Implementation header for strfroml ------------------------*- 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_STDLIB_STRFROML_H
+#define LLVM_LIBC_SRC_STDLIB_STRFROML_H
+
+#include <stddef.h>
+
+namespace LIBC_NAMESPACE {
+
+int strfroml(char *__restrict s, size_t n, const char *__restrict format,
+             long double fp);
+
+} // namespace LIBC_NAMESPACE
+
+#endif // LLVM_LIBC_SRC_STDLIB_STRFROML_H
diff --git a/libc/test/src/stdlib/CMakeLists.txt b/libc/test/src/stdlib/CMakeLists.txt
index cb42bc56f51c5f..79113d943d1ae4 100644
--- a/libc/test/src/stdlib/CMakeLists.txt
+++ b/libc/test/src/stdlib/CMakeLists.txt
@@ -178,6 +178,26 @@ add_libc_test(
     libc.src.stdlib.strfromf
 )
 
+add_libc_test(
+  strfromd_test
+  SUITE
+    libc-stdlib-tests
+  SRCS
+    strfromd_test.cpp
+  DEPENDS
+    libc.src.stdlib.strfromd
+)
+
+add_libc_test(
+  strfroml_test
+  SUITE
+    libc-stdlib-tests
+  SRCS
+    strfroml_test.cpp
+  DEPENDS
+    libc.src.stdlib.strfroml
+)
+
 add_libc_test(
   abs_test
   SUITE
diff --git a/libc/test/src/stdlib/strfromd_test.cpp b/libc/test/src/stdlib/strfromd_test.cpp
new file mode 100644
index 00000000000000..60fadca597569b
--- /dev/null
+++ b/libc/test/src/stdlib/strfromd_test.cpp
@@ -0,0 +1,224 @@
+//===-- Unittests for strfromd --------------------------------------------===//
+//
+// 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/stdlib/strfromd.h"
+#include "test/UnitTest/Test.h"
+
+// Use when the buffsize is sufficient to hold all of the result string,
+// including the null byte.
+#define ASSERT_STREQ_LEN_STRFROMD(actual_written, actual_str, expected_str)    \
+  EXPECT_EQ(actual_written, static_cast<int>(sizeof(expected_str) - 1));       \
+  EXPECT_STREQ(actual_str, expected_str);
+
+TEST(LlvmLibcStrfromdTest, FloatDecimalFormat) {
+  char buff[500];
+  int written;
+
+  written = LIBC_NAMESPACE::strfromd(buff, 99, "%f", 1.0);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "1.000000");
+
+  written = LIBC_NAMESPACE::strfromd(buff, 99, "%F", -1.0);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "-1.000000");
+
+  written = LIBC_NAMESPACE::strfromd(buff, 99, "%f", -1.234567);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "-1.234567");
+
+  written = LIBC_NAMESPACE::strfromd(buff, 99, "%f", 0.0);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "0.000000");
+
+  written = LIBC_NAMESPACE::strfromd(buff, 99, "%f", 1.5);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "1.500000");
+
+  written = LIBC_NAMESPACE::strfromd(buff, 499, "%f", 1e300);
+  ASSERT_STREQ_LEN_STRFROMD(
+      written, buff,
+      "100000000000000005250476025520442024870446858110815915491585411551180245"
+      "798890819578637137508044786404370444383288387817694252323536043057564479"
+      "218478670698284838720092657580373783023379478809005936895323497079994508"
+      "111903896764088007465274278014249457925878882005684283811566947219638686"
+      "5459400540160.000000");
+
+  written = LIBC_NAMESPACE::strfromd(buff, 99, "%f", 0.1);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "0.100000");
+
+  written = LIBC_NAMESPACE::strfromd(buff, 99, "%f", 1234567890123456789.0);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "1234567890123456768.000000");
+
+  written = LIBC_NAMESPACE::strfromd(buff, 99, "%f", 9999999999999.99);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "9999999999999.990234");
+
+  written = LIBC_NAMESPACE::strfromd(buff, 99, "%f", 0.1);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "0.100000");
+
+  written = LIBC_NAMESPACE::strfromd(buff, 99, "%f", 1234567890123456789.0);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "1234567890123456768.000000");
+
+  written = LIBC_NAMESPACE::strfromd(buff, 99, "%f", 9999999999999.99);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "9999999999999.990234");
+
+  // Precision Tests
+  written = LIBC_NAMESPACE::strfromd(buff, 100, "%.2f", 9999999999999.99);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "9999999999999.99");
+
+  written = LIBC_NAMESPACE::strfromd(buff, 100, "%.1f", 9999999999999.99);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "10000000000000.0");
+
+  written = LIBC_NAMESPACE::strfromd(buff, 100, "%.5f", 1.25);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "1.25000");
+
+  written = LIBC_NAMESPACE::strfromd(buff, 100, "%.0f", 1.25);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "1");
+
+  written = LIBC_NAMESPACE::strfromd(buff, 100, "%.20f", 1.234e-10);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "0.00000000012340000000");
+}
+
+TEST(LlvmLibcStrfromdTest, FloatHexExpFormat) {
+  char buff[101];
+  int written;
+
+  written = LIBC_NAMESPACE::strfromd(buff, 100, "%a", 1.0);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "0x1p+0");
+
+  written = LIBC_NAMESPACE::strfromd(buff, 100, "%A", -1.0);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "-0X1P+0");
+
+  written = LIBC_NAMESPACE::strfromd(buff, 100, "%a", -0x1.abcdef12345p0);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "-0x1.abcdef12345p+0");
+
+  written = LIBC_NAMESPACE::strfromd(buff, 100, "%A", 0x1.abcdef12345p0);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "0X1.ABCDEF12345P+0");
+
+  written = LIBC_NAMESPACE::strfromd(buff, 100, "%a", 0.0);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "0x0p+0");
+
+  written = LIBC_NAMESPACE::strfromd(buff, 100, "%a", 1.0e100);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "0x1.249ad2594c37dp+332");
+
+  written = LIBC_NAMESPACE::strfromd(buff, 100, "%a", 0.1);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "0x1.999999999999ap-4");
+}
+
+TEST(LlvmLibcStrfromdTest, FloatDecimalExpFormat) {
+  char buff[101];
+  int written;
+
+  written = LIBC_NAMESPACE::strfromd(buff, 100, "%e", 1.0);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "1.000000e+00");
+
+  written = LIBC_NAMESPACE::strfromd(buff, 100, "%E", -1.0);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "-1.000000E+00");
+
+  written = LIBC_NAMESPACE::strfromd(buff, 100, "%e", -1.234567);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "-1.234567e+00");
+
+  written = LIBC_NAMESPACE::strfromd(buff, 100, "%e", 0.0);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "0.000000e+00");
+
+  written = LIBC_NAMESPACE::strfromd(buff, 100, "%e", 1.5);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "1.500000e+00");
+
+  written = LIBC_NAMESPACE::strfromd(buff, 100, "%e", 1e300);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "1.000000e+300");
+
+  written = LIBC_NAMESPACE::strfromd(buff, 100, "%e", 1234567890123456789.0);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "1.234568e+18");
+
+  // Precision Tests
+  written = LIBC_NAMESPACE::strfromd(buff, 100, "%.1e", 1.0);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "1.0e+00");
+
+  written = LIBC_NAMESPACE::strfromd(buff, 100, "%.1e", 1.99);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "2.0e+00");
+
+  written = LIBC_NAMESPACE::strfromd(buff, 100, "%.1e", 9.99);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "1.0e+01");
+}
+
+TEST(LlvmLibcStrfromdTest, FloatDecimalAutoFormat) {
+  char buff[120];
+  int written;
+
+  written = LIBC_NAMESPACE::strfromd(buff, 100, "%g", 1234567890123456789.0);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "1.23457e+18");
+
+  written = LIBC_NAMESPACE::strfromd(buff, 100, "%g", 9999990000000.00);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "9.99999e+12");
+
+  written = LIBC_NAMESPACE::strfromd(buff, 100, "%g", 9999999000000.00);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "1e+13");
+
+  written = LIBC_NAMESPACE::strfromd(buff, 100, "%g", 0xa.aaaaaaaaaaaaaabp-7);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "0.0833333");
+
+  written = LIBC_NAMESPACE::strfromd(buff, 100, "%g", 0.00001);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "1e-05");
+
+  // Precision Tests
+  written = LIBC_NAMESPACE::strfromd(buff, 100, "%.0g", 0.0);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "0");
+
+  written = LIBC_NAMESPACE::strfromd(buff, 100, "%.2g", 0.1);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "0.1");
+
+  written = LIBC_NAMESPACE::strfromd(buff, 100, "%.2g", 1.09);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "1.1");
+
+  written = LIBC_NAMESPACE::strfromd(buff, 100, "%.15g", 22.25);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "22.25");
+
+  written = LIBC_NAMESPACE::strfromd(buff, 100, "%.20g", 1.234e-10);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "1.2340000000000000814e-10");
+}
+
+TEST(LlvmLibcStrfromdTest, ImproperFormatString) {
+  char buff[100];
+  int written;
+
+  written = LIBC_NAMESPACE::strfromd(
+      buff, 37, "A simple string with no conversions.", 1.0);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff,
+                            "A simple string with no conversions.");
+
+  written = LIBC_NAMESPACE::strfromd(
+      buff, 37, "%A simple string with one conversion, should overwrite.", 1.0);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff, "0X1P+0");
+
+  written =
+      LIBC_NAMESPACE::strfromd(buff, 74,
+                               "A simple string with one conversion in %A "
+                               "between, writes string as it is",
+                               1.0);
+  ASSERT_STREQ_LEN_STRFROMD(
+      written, buff,
+      "A simple string with one conversion in %A between, "
+      "writes string as it is");
+
+  written = LIBC_NAMESPACE::strfromd(
+      buff, 36, "A simple string with one conversion", 1.0);
+  ASSERT_STREQ_LEN_STRFROMD(written, buff,
+                            "A simple string with one conversion");
+}
+
+// Test the result when the buffsize is not sufficient to hold
+// the result string.
+TEST(LlvmLibcStrfromdTest, InsufficientBuffsize) {
+  char buff[20];
+  int written;
+
+  written = LIBC_NAMESPACE::strfromd(buff, 5, "%f", 1234567890.0);
+  EXPECT_EQ(written, 17);
+  ASSERT_STREQ(buff, "1234");
+
+  written = LIBC_NAMESPACE::strfromd(buff, 5, "%.5f", 1.05);
+  EXPECT_EQ(written, 7);
+  ASSERT_STREQ(buff, "1.05");
+
+  written = LIBC_NAMESPACE::strfromd(buff, 0, "%g", 1.0);
+  EXPECT_EQ(written, 1);
+}
diff --git a/libc/test/src/stdlib/strfromf_test.cpp b/libc/test/src/stdlib/strfromf_test.cpp
index c5489f5f3af2ac..86edeaab853443 100644
--- a/libc/test/src/stdlib/strfromf_test.cpp
+++ b/libc/test/src/stdlib/strfromf_test.cpp
@@ -9,29 +9,22 @@
 #include "src/stdlib/strfromf.h"
 #include "test/UnitTest/Test.h"
 
+#define ASSERT_STREQ_LEN_STRFROMF(actual_written, actual_str, expected_str)    \
+  EXPECT_EQ(actual_written, static_cast<int>(sizeof(expected_str) - 1));       \
+  EXPECT_STREQ(actual_str, expected_str);
+
 TEST(LlvmLibcStrfromfTest, DecimalFloatFormat) {
   char buff[100];
   int written;
 
   written = LIBC_NAMESPACE::strfromf(buff, 16, "%f", 1.0);
-  EXPECT_EQ(written, 8);
-  ASSERT_STREQ(buff, "1.000000");
+  ASSERT_STREQ_LEN_STRFROMF(written, buff, "1.000000");
 
   written = LIBC_NAMESPACE::strfromf(buff, 20, "%f", 1234567890.0);
-  EXPECT_EQ(written, 17);
-  ASSERT_STREQ(buff, "1234567936.000000");
-
-  written = LIBC_NAMESPACE::strfromf(buff, 5, "%f", 1234567890.0);
-  EXPECT_EQ(written, 17);
-  ASSERT_STREQ(buff, "1234");
+  ASSERT_STREQ_LEN_STRFROMF(written, buff, "1234567936.000000");
 
   written = LIBC_NAMESPACE::strfromf(buff, 67, "%.3f", 1.0);
-  EXPECT_EQ(written, 5);
-  ASSERT_STREQ(buff, "1.000");
-
-  written = LIBC_NAMESPACE::strfromf(buff, 20, "%1f", 1234567890.0);
-  EXPECT_EQ(written, 3);
-  ASSERT_STREQ(buff, "%1f");
+  ASSERT_STREQ_LEN_STRFROMF(written, buff, "1.000");
 }
 
 TEST(LlvmLibcStrfromfTest, HexExpFloatFormat) {
@@ -54,12 +47,10 @@ TEST(LlvmLibcStrfromfTest, DecimalExpFloatFormat) {
   char buff[100];
   int written;
   written = LIBC_NAMESPACE::strfromf(buff, 20, "%.9e", 1234567890.0);
-  EXPECT_EQ(written, 15);
-  ASSERT_STREQ(buff, "1.234567936e+09");
+  ASSERT_STREQ_LEN_STRFROMF(written, buff, "1.234567936e+09");
 
   written = LIBC_NAMESPACE::strfromf(buff, 20, "%.9E", 1234567890.0);
-  EXPECT_EQ(written, 15);
-  ASSERT_STREQ(buff, "1.234567936E+09");
+  ASSERT_STREQ_LEN_STRFROMF(written, buff, "1.234567936E+09");
 }
 
 TEST(LlvmLibcStrfromfTest, AutoDecimalFloatFormat) {
@@ -67,41 +58,55 @@ TEST(LlvmLibcStrfromfTest, AutoDecimalFloatFormat) {
   int written;
 
   written = LIBC_NAMESPACE::strfromf(buff, 20, "%.9g", 1234567890.0);
-  EXPECT_EQ(written, 14);
-  ASSERT_STREQ(buff, "1.23456794e+09");
+  ASSERT_STREQ_LEN_STRFROMF(written, buff, "1.23456794e+09");
 
   written = LIBC_NAMESPACE::strfromf(buff, 20, "%.9G", 1234567890.0);
-  EXPECT_EQ(written, 14);
-  ASSERT_STREQ(buff, "1.23456794E+09");
-
-  written = LIBC_NAMESPACE::strfromf(buff, 0, "%G", 1.0);
-  EXPECT_EQ(written, 1);
+  ASSERT_STREQ_LEN_STRFROMF(written, buff, "1.23456794E+09");
 }
 
 TEST(LlvmLibcStrfromfTest, ImproperFormatString) {
-
   char buff[100];
-  int retval;
-  retval = LIBC_NAMESPACE::strfromf(
+  int written;
+  written = LIBC_NAMESPACE::strfromf(
       buff, 37, "A simple string with no conversions.", 1.0);
-  EXPECT_EQ(retval, 36);
-  ASSERT_STREQ(buff, "A simple string with no conversions.");
+  ASSERT_STREQ_LEN_STRFROMF(written, buff,
+                            "A simple string with no conversions.");
 
-  retval = LIBC_NAMESPACE::strfromf(
+  written = LIBC_NAMESPACE::strfromf(
       buff, 37, "%A simple string with one conversion, should overwrite.", 1.0);
-  EXPECT_EQ(retval, 6);
-  ASSERT_STREQ(buff, "0X1P+0");
-
-  retval = LIBC_NAMESPACE::strfromf(buff, 74,
-                                    "A simple string with one conversion in %A "
-                                    "between, writes string as it i...
[truncated]

@vinayakdsci vinayakdsci marked this pull request as draft March 21, 2024 18:05
@vinayakdsci vinayakdsci marked this pull request as ready for review March 22, 2024 16:30
Copy link
Contributor

@michaelrj-google michaelrj-google left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM with some comments

ASSERT_STREQ(buff, "1.05");

written = func(buff, 0, "%g", 1.0);
EXPECT_EQ(written, 1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should check that the buffer hasn't changed here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, will add a check for this too. Thanks!


printf_core::FormatSection section =
internal::parse_format_string(format, fp);
section.length_modifier = printf_core::LengthModifier::L;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Add a comment here to mark that this is different from the others.

@@ -961,7 +961,9 @@ def StdC : StandardSpec<"stdc"> {
FunctionSpec<"rand", RetValSpec<IntType>, [ArgSpec<VoidType>]>,
FunctionSpec<"srand", RetValSpec<VoidType>, [ArgSpec<UnsignedIntType>]>,

FunctionSpec<"strfromd", RetValSpec<IntType>, [ArgSpec<CharRestrictedPtr>, ArgSpec<SizeTType>, ArgSpec<ConstCharRestrictedPtr>, ArgSpec<DoubleType>]>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: While putting strfromd first makes these alphabetical, I would prefer to have them in order of size (strfromf, strfromd, strfroml). Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you are right here, putting them by size makes more sense. I will change this and re-push.

Copy link
Contributor

@michaelrj-google michaelrj-google left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@michaelrj-google
Copy link
Contributor

Just as a heads up: It's easier to review a PR if you add new commits instead of amending your previous one since github will let me filter to just the changes in the new commit. It will also handle squashing them all into one commit before merging, so you don't need to worry about that.

@vinayakdsci
Copy link
Contributor Author

vinayakdsci commented Mar 22, 2024

Just as a heads up: It's easier to review a PR if you add new commits instead of amending your previous one since github will let me filter to just the changes in the new commit. It will also handle squashing them all into one commit before merging, so you don't need to worry about

I have only contributed small patches in the past, so I was not sure about this. I will keep this in mind in the future. Thanks a lot for your review!

@michaelrj-google michaelrj-google merged commit 83e9697 into llvm:main Mar 22, 2024
4 checks passed
chencha3 pushed a commit to chencha3/llvm-project that referenced this pull request Mar 23, 2024
Follow up to llvm#85438.

Implements the functions `strfromd()` and `strfroml()` introduced in
C23, and unifies the testing framework for `strfrom*()` functions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants