[libc-commits] [PATCH] D144679: [libc] Clarify printf percent conversion behavior.

[Michael Jones via Phabricator via libc-commits]

michaelrj updated this revision to Diff 500248.
michaelrj marked an inline comment as done.
michaelrj added a comment.

fix comment phrasing before landing


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144679/new/

https://reviews.llvm.org/D144679

Files:
  libc/src/stdio/printf_core/parser.cpp
  libc/test/src/stdio/printf_core/parser_test.cpp


Index: libc/test/src/stdio/printf_core/parser_test.cpp
===================================================================
--- libc/test/src/stdio/printf_core/parser_test.cpp
+++ libc/test/src/stdio/printf_core/parser_test.cpp
@@ -523,4 +523,29 @@
   EXPECT_PFORMAT_EQ(expected1, format_arr[1]);
 }
 
+TEST(LlvmLibcPrintfParserTest, DoublePercentIsAllowedInvalidIndex) {
+  __llvm_libc::printf_core::FormatSection format_arr[20];
+
+  // Normally this conversion specifier would be raw (due to having a width
+  // defined as an invalid argument) but since it's a % conversion it's allowed
+  // by this specific parser. Any % conversion that is not just "%%" is
+  // undefined, so this is implementation-specific behavior.
+  // The goal is to be consistent. A conversion specifier of "%L%" is also
+  // undefined, but is traditionally displayed as just "%". "%2%" is also
+  // displayed as "%", even though if the width was respected it should be " %".
+  // Finally, "%*%" traditionally is displayed as "%" but also traditionally
+  // consumes an argument, since the * consumes an integer. Therefore, having
+  // "%*2$%" display as "%" is consistent with other printf behavior.
+  const char *str = "%*2$%";
+
+  evaluate(format_arr, str, 1, 2);
+
+  __llvm_libc::printf_core::FormatSection expected0;
+  expected0.has_conv = true;
+
+  expected0.raw_string = str;
+  expected0.conv_name = '%';
+  EXPECT_PFORMAT_EQ(expected0, format_arr[0]);
+}
+
 #endif // LIBC_COPT_PRINTF_DISABLE_INDEX_MODE
Index: libc/src/stdio/printf_core/parser.cpp
===================================================================
--- libc/src/stdio/printf_core/parser.cpp
+++ libc/src/stdio/printf_core/parser.cpp
@@ -106,6 +106,13 @@
     section.conv_name = str[cur_pos];
     switch (str[cur_pos]) {
     case ('%'):
+      // Regardless of options, a % conversion is always safe. The standard says
+      // that "The complete conversion specification shall be %%" but it also
+      // says that "If a conversion specification is invalid, the behavior is
+      // undefined." Based on that we define that any conversion specification
+      // ending in '%' shall display as '%' regardless of any valid or invalid
+      // options.
+      section.has_conv = true;
       break;
     case ('c'):
       WRITE_ARG_VAL_SIMPLEST(section.conv_val_raw, int, conv_index);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D144679.500248.patch
Type: text/x-patch
Size: 2358 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libc-commits/attachments/20230224/82403ca2/attachment.bin>