about summary refs log tree commit diff
path: root/absl/strings
diff options
context:
space:
mode:
Diffstat (limited to 'absl/strings')
-rw-r--r--absl/strings/internal/str_format/parser.cc75
-rw-r--r--absl/strings/internal/str_format/parser_test.cc16
-rw-r--r--absl/strings/string_view.h29
3 files changed, 70 insertions, 50 deletions
diff --git a/absl/strings/internal/str_format/parser.cc b/absl/strings/internal/str_format/parser.cc
index c3e16fceb15e..5e3d0d0547d1 100644
--- a/absl/strings/internal/str_format/parser.cc
+++ b/absl/strings/internal/str_format/parser.cc
@@ -84,16 +84,24 @@ bool ConsumeConversion(string_view *src, UnboundConversion *conv,
   const char *pos = src->data();
   const char *const end = pos + src->size();
   char c;
-  // Read the next char into `c` and update `pos`. Reads '\0' if at end.
-  const auto get_char = [&] { c = pos == end ? '\0' : *pos++; };
+  // Read the next char into `c` and update `pos`. Returns false if there are
+  // no more chars to read.
+#define ABSL_FORMAT_PARSER_INTERNAL_GET_CHAR()        \
+  do {                                                \
+    if (ABSL_PREDICT_FALSE(pos == end)) return false; \
+    c = *pos++;                                       \
+  } while (0)
 
   const auto parse_digits = [&] {
     int digits = c - '0';
-    // We do not want to overflow `digits` so we consume at most digits10-1
+    // We do not want to overflow `digits` so we consume at most digits10
     // digits. If there are more digits the parsing will fail later on when the
     // digit doesn't match the expected characters.
-    int num_digits = std::numeric_limits<int>::digits10 - 2;
-    for (get_char(); num_digits && std::isdigit(c); get_char()) {
+    int num_digits = std::numeric_limits<int>::digits10;
+    for (;;) {
+      if (ABSL_PREDICT_FALSE(pos == end || !num_digits)) break;
+      c = *pos++;
+      if (!std::isdigit(c)) break;
       --num_digits;
       digits = 10 * digits + c - '0';
     }
@@ -101,14 +109,14 @@ bool ConsumeConversion(string_view *src, UnboundConversion *conv,
   };
 
   if (is_positional) {
-    get_char();
-    if (c < '1' || c > '9') return false;
+    ABSL_FORMAT_PARSER_INTERNAL_GET_CHAR();
+    if (ABSL_PREDICT_FALSE(c < '1' || c > '9')) return false;
     conv->arg_position = parse_digits();
     assert(conv->arg_position > 0);
-    if (c != '$') return false;
+    if (ABSL_PREDICT_FALSE(c != '$')) return false;
   }
 
-  get_char();
+  ABSL_FORMAT_PARSER_INTERNAL_GET_CHAR();
 
   // We should start with the basic flag on.
   assert(conv->flags.basic);
@@ -119,32 +127,39 @@ bool ConsumeConversion(string_view *src, UnboundConversion *conv,
   if (c < 'A') {
     conv->flags.basic = false;
 
-    for (; c <= '0'; get_char()) {
+    for (; c <= '0';) {
+      // FIXME: We might be able to speed this up reusing the kIds lookup table
+      // from above.
+      // It might require changing Flags to be a plain integer where we can |= a
+      // value.
       switch (c) {
         case '-':
           conv->flags.left = true;
-          continue;
+          break;
         case '+':
           conv->flags.show_pos = true;
-          continue;
+          break;
         case ' ':
           conv->flags.sign_col = true;
-          continue;
+          break;
         case '#':
           conv->flags.alt = true;
-          continue;
+          break;
         case '0':
           conv->flags.zero = true;
-          continue;
+          break;
+        default:
+          goto flags_done;
       }
-      break;
+      ABSL_FORMAT_PARSER_INTERNAL_GET_CHAR();
     }
+flags_done:
 
     if (c <= '9') {
       if (c >= '0') {
         int maybe_width = parse_digits();
         if (!is_positional && c == '$') {
-          if (*next_arg != 0) return false;
+          if (ABSL_PREDICT_FALSE(*next_arg != 0)) return false;
           // Positional conversion.
           *next_arg = -1;
           conv->flags = Flags();
@@ -153,12 +168,12 @@ bool ConsumeConversion(string_view *src, UnboundConversion *conv,
         }
         conv->width.set_value(maybe_width);
       } else if (c == '*') {
-        get_char();
+        ABSL_FORMAT_PARSER_INTERNAL_GET_CHAR();
         if (is_positional) {
-          if (c < '1' || c > '9') return false;
+          if (ABSL_PREDICT_FALSE(c < '1' || c > '9')) return false;
           conv->width.set_from_arg(parse_digits());
-          if (c != '$') return false;
-          get_char();
+          if (ABSL_PREDICT_FALSE(c != '$')) return false;
+          ABSL_FORMAT_PARSER_INTERNAL_GET_CHAR();
         } else {
           conv->width.set_from_arg(++*next_arg);
         }
@@ -166,16 +181,16 @@ bool ConsumeConversion(string_view *src, UnboundConversion *conv,
     }
 
     if (c == '.') {
-      get_char();
+      ABSL_FORMAT_PARSER_INTERNAL_GET_CHAR();
       if (std::isdigit(c)) {
         conv->precision.set_value(parse_digits());
       } else if (c == '*') {
-        get_char();
+        ABSL_FORMAT_PARSER_INTERNAL_GET_CHAR();
         if (is_positional) {
-          if (c < '1' || c > '9') return false;
+          if (ABSL_PREDICT_FALSE(c < '1' || c > '9')) return false;
           conv->precision.set_from_arg(parse_digits());
           if (c != '$') return false;
-          get_char();
+          ABSL_FORMAT_PARSER_INTERNAL_GET_CHAR();
         } else {
           conv->precision.set_from_arg(++*next_arg);
         }
@@ -188,23 +203,23 @@ bool ConsumeConversion(string_view *src, UnboundConversion *conv,
   std::int8_t id = kIds[static_cast<unsigned char>(c)];
 
   if (id < 0) {
-    if (id == none) return false;
+    if (ABSL_PREDICT_FALSE(id == none)) return false;
 
     // It is a length modifier.
     using str_format_internal::LengthMod;
     LengthMod length_mod = LengthMod::FromId(static_cast<LM>(~id));
-    get_char();
+    ABSL_FORMAT_PARSER_INTERNAL_GET_CHAR();
     if (c == 'h' && length_mod.id() == LengthMod::h) {
       conv->length_mod = LengthMod::FromId(LengthMod::hh);
-      get_char();
+      ABSL_FORMAT_PARSER_INTERNAL_GET_CHAR();
     } else if (c == 'l' && length_mod.id() == LengthMod::l) {
       conv->length_mod = LengthMod::FromId(LengthMod::ll);
-      get_char();
+      ABSL_FORMAT_PARSER_INTERNAL_GET_CHAR();
     } else {
       conv->length_mod = length_mod;
     }
     id = kIds[static_cast<unsigned char>(c)];
-    if (id < 0) return false;
+    if (ABSL_PREDICT_FALSE(id < 0)) return false;
   }
 
   assert(CheckFastPathSetting(*conv));
diff --git a/absl/strings/internal/str_format/parser_test.cc b/absl/strings/internal/str_format/parser_test.cc
index ac6886d04d93..ae40203191b9 100644
--- a/absl/strings/internal/str_format/parser_test.cc
+++ b/absl/strings/internal/str_format/parser_test.cc
@@ -84,9 +84,9 @@ class ConsumeUnboundConversionTest : public ::testing::Test {
 TEST_F(ConsumeUnboundConversionTest, ConsumeSpecification) {
   struct Expectation {
     int line;
-    const char *src;
-    const char *out;
-    const char *src_post;
+    string_view src;
+    string_view out;
+    string_view src_post;
   };
   const Expectation kExpect[] = {
     {__LINE__, "",     "",     ""  },
@@ -236,6 +236,16 @@ TEST_F(ConsumeUnboundConversionTest, WidthAndPrecision) {
   EXPECT_EQ(9, o.precision.get_from_arg());
 
   EXPECT_FALSE(Run(".*0$d")) << "no arg 0";
+
+  // Large values
+  EXPECT_TRUE(Run("999999999.999999999d"));
+  EXPECT_FALSE(o.width.is_from_arg());
+  EXPECT_EQ(999999999, o.width.value());
+  EXPECT_FALSE(o.precision.is_from_arg());
+  EXPECT_EQ(999999999, o.precision.value());
+
+  EXPECT_FALSE(Run("1000000000.999999999d"));
+  EXPECT_FALSE(Run("999999999.1000000000d"));
 }
 
 TEST_F(ConsumeUnboundConversionTest, Flags) {
diff --git a/absl/strings/string_view.h b/absl/strings/string_view.h
index 9c03108b4439..6bcd3c4e341f 100644
--- a/absl/strings/string_view.h
+++ b/absl/strings/string_view.h
@@ -173,8 +173,19 @@ class string_view {
   // Implicit constructor of a `string_view` from nul-terminated `str`. When
   // accepting possibly null strings, use `absl::NullSafeStringView(str)`
   // instead (see below).
+#if ABSL_HAVE_BUILTIN(__builtin_strlen) || \
+    (defined(__GNUC__) && !defined(__clang__))
+  // GCC has __builtin_strlen according to
+  // https://gcc.gnu.org/onlinedocs/gcc-4.7.0/gcc/Other-Builtins.html, but
+  // ABSL_HAVE_BUILTIN doesn't detect that, so we use the extra checks above.
+  // __builtin_strlen is constexpr.
+  constexpr string_view(const char* str)  // NOLINT(runtime/explicit)
+      : ptr_(str),
+        length_(CheckLengthInternal(str ? __builtin_strlen(str) : 0)) {}
+#else
   constexpr string_view(const char* str)  // NOLINT(runtime/explicit)
-      : ptr_(str), length_(CheckLengthInternal(StrLenInternal(str))) {}
+      : ptr_(str), length_(CheckLengthInternal(str ? strlen(str) : 0)) {}
+#endif
 
   // Implicit constructor of a `string_view` from a `const char*` and length.
   constexpr string_view(const char* data, size_type len)
@@ -481,22 +492,6 @@ class string_view {
   static constexpr size_type kMaxSize =
       std::numeric_limits<difference_type>::max();
 
-  // check whether __builtin_strlen is provided by the compiler.
-  // GCC doesn't have __has_builtin()
-  // (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66970),
-  // but has __builtin_strlen according to
-  // https://gcc.gnu.org/onlinedocs/gcc-4.7.0/gcc/Other-Builtins.html.
-#if ABSL_HAVE_BUILTIN(__builtin_strlen) || \
-    (defined(__GNUC__) && !defined(__clang__))
-  static constexpr size_type StrLenInternal(const char* str) {
-    return str ? __builtin_strlen(str) : 0;
-  }
-#else
-  static constexpr size_type StrLenInternal(const char* str) {
-    return str ? strlen(str) : 0;
-  }
-#endif
-
   static constexpr size_type CheckLengthInternal(size_type len) {
     return ABSL_ASSERT(len <= kMaxSize), len;
   }
generated by cgit-pink 1.4.1 (git 2.44.2) at 2024-11-10T20ยท50+0000