Export of internal Abseil changes

-- 832be2d52d7695cf72fd70248909791fb8ad1003 by Gennadiy Rozental <[email protected]>: Migrate some FlagImpl fields to bit fields. To save padding space we are migrating some fields representing bool and enums into bit fields. Eventually we'll use remaining padding space for call_once control flag. No other semantic changes made in this CL. PiperOrigin-RevId: 293878165 -- 09162bba5fd8eddacfd732d46fcfeb33074a259f by Samuel Benzaquen <[email protected]>: Correctly initialize the `length_mod` member. Now that it is a raw enum, the default initialization is not enough. PiperOrigin-RevId: 293827817 -- 842b7b805d75c5ab670c52ccd7368cdeba11853d by Matthew Brown <[email protected]>: Move str_format_internal::LengthMod from extension.h to parser.h; change to enum PiperOrigin-RevId: 293697274 GitOrigin-RevId: 832be2d52d7695cf72fd70248909791fb8ad1003 Change-Id: I90899519e9480543e22638616fdf31a41e7f75c0
abseil · Feb 7, 2020 · d95d156 · d95d156
1 parent 24713a7
commit d95d156
Show file tree

Hide file tree

Showing 9 changed files with 116 additions and 128 deletions.
diff --git a/absl/flags/internal/flag.cc b/absl/flags/internal/flag.cc
@@ -120,7 +120,7 @@ void FlagImpl::Destroy() {
     if (value_.dynamic) Delete(op_, value_.dynamic);
 
     // Release the dynamically allocated default value if any.
-    if (def_kind_ == FlagDefaultSrcKind::kDynamicValue) {
+    if (DefaultKind() == FlagDefaultKind::kDynamicValue) {
       Delete(op_, default_src_.dynamic_value);
     }
 
@@ -135,7 +135,7 @@ void FlagImpl::Destroy() {
 
 std::unique_ptr<void, DynValueDeleter> FlagImpl::MakeInitValue() const {
   void* res = nullptr;
-  if (def_kind_ == FlagDefaultSrcKind::kDynamicValue) {
+  if (DefaultKind() == FlagDefaultKind::kDynamicValue) {
     res = Clone(op_, default_src_.dynamic_value);
   } else {
     res = (*default_src_.gen_func)();
@@ -150,8 +150,8 @@ std::string FlagImpl::Filename() const {
 }
 
 std::string FlagImpl::Help() const {
-  return help_source_kind_ == FlagHelpKind::kLiteral ? help_.literal
-                                                     : help_.gen_func();
+  return HelpSourceKind() == FlagHelpKind::kLiteral ? help_.literal
+                                                    : help_.gen_func();
 }
 
 bool FlagImpl::IsModified() const {
@@ -364,7 +364,7 @@ bool FlagImpl::SetFromString(absl::string_view value, FlagSettingMode set_mode,
       break;
     }
     case SET_FLAGS_DEFAULT: {
-      if (def_kind_ == FlagDefaultSrcKind::kDynamicValue) {
+      if (DefaultKind() == FlagDefaultKind::kDynamicValue) {
         if (!TryParse(&default_src_.dynamic_value, value, err)) {
           return false;
         }
@@ -375,7 +375,7 @@ bool FlagImpl::SetFromString(absl::string_view value, FlagSettingMode set_mode,
         }
 
         default_src_.dynamic_value = new_default_val;
-        def_kind_ = FlagDefaultSrcKind::kDynamicValue;
+        def_kind_ = static_cast<uint8_t>(FlagDefaultKind::kDynamicValue);
       }
 
       if (!modified_) {

diff --git a/absl/flags/internal/flag.h b/absl/flags/internal/flag.h
@@ -87,7 +87,7 @@ union FlagHelpMsg {
   HelpGenFunc gen_func;
 };
 
-enum class FlagHelpKind : int8_t { kLiteral, kGenFunc };
+enum class FlagHelpKind : uint8_t { kLiteral = 0, kGenFunc = 1 };
 
 struct FlagHelpArg {
   FlagHelpMsg source;
@@ -147,7 +147,7 @@ union FlagDefaultSrc {
   FlagDfltGenFunc gen_func;
 };
 
-enum class FlagDefaultSrcKind : int8_t { kDynamicValue, kGenFunc };
+enum class FlagDefaultKind : uint8_t { kDynamicValue = 0, kGenFunc = 1 };
 
 ///////////////////////////////////////////////////////////////////////////////
 // Flag current value auxiliary structs.
@@ -279,8 +279,14 @@ class FlagImpl {
         op_(op),
         marshalling_op_(marshalling_op),
         help_(help.source),
-        help_source_kind_(help.kind),
-        def_kind_(FlagDefaultSrcKind::kGenFunc),
+        help_source_kind_(static_cast<uint8_t>(help.kind)),
+        def_kind_(static_cast<uint8_t>(FlagDefaultKind::kGenFunc)),
+        is_data_guard_inited_(false),
+        modified_(false),
+        on_command_line_(false),
+        inited_(false),
+        counter_(0),
+        callback_(nullptr),
         default_src_(default_value_gen),
         data_guard_{} {}
 
@@ -370,6 +376,14 @@ class FlagImpl {
   // Lazy initialization of the Flag's data.
   void Init();
 
+  FlagHelpKind HelpSourceKind() const {
+    return static_cast<FlagHelpKind>(help_source_kind_);
+  }
+  FlagDefaultKind DefaultKind() const
+      ABSL_EXCLUSIVE_LOCKS_REQUIRED(*DataGuard()) {
+    return static_cast<FlagDefaultKind>(def_kind_);
+  }
+
   // Immutable flag's state.
 
   // Flags name passed to ABSL_FLAG as second arg.
@@ -383,35 +397,31 @@ class FlagImpl {
   // Help message literal or function to generate it.
   const FlagHelpMsg help_;
   // Indicates if help message was supplied as literal or generator func.
-  const FlagHelpKind help_source_kind_;
-
-  // Indicates that the Flag state is initialized.
-  std::atomic<bool> inited_{false};
+  const uint8_t help_source_kind_ : 1;
 
   // Mutable flag's state (guarded by `data_guard_`).
 
+  // If def_kind_ == kDynamicValue, default_src_ holds a dynamically allocated
+  // value.
+  uint8_t def_kind_ : 1 ABSL_GUARDED_BY(*DataGuard());
   // Protects against multiple concurrent constructions of `data_guard_`.
-  bool is_data_guard_inited_ = false;
+  bool is_data_guard_inited_ : 1;
   // Has this flag's value been modified?
-  bool modified_ ABSL_GUARDED_BY(*DataGuard()) = false;
+  bool modified_ : 1 ABSL_GUARDED_BY(*DataGuard());
   // Has this flag been specified on command line.
-  bool on_command_line_ ABSL_GUARDED_BY(*DataGuard()) = false;
+  bool on_command_line_ : 1 ABSL_GUARDED_BY(*DataGuard());
 
+  // Indicates that the flag state is initialized.
+  std::atomic<bool> inited_;
   // Mutation counter
-  int64_t counter_ ABSL_GUARDED_BY(*DataGuard()) = 0;
-
+  int64_t counter_ ABSL_GUARDED_BY(*DataGuard());
   // Optional flag's callback and absl::Mutex to guard the invocations.
-  FlagCallback* callback_ ABSL_GUARDED_BY(*DataGuard()) = nullptr;
-
-  // If def_kind_ == kDynamicValue, default_src_ holds a dynamically allocated
-  // value.
-  FlagDefaultSrcKind def_kind_ ABSL_GUARDED_BY(*DataGuard());
+  FlagCallback* callback_ ABSL_GUARDED_BY(*DataGuard());
   // Either a pointer to the function generating the default value based on the
   // value specified in ABSL_FLAG or pointer to the dynamically set default
   // value via SetCommandLineOptionWithMode. def_kind_ is used to distinguish
   // these two cases.
   FlagDefaultSrc default_src_ ABSL_GUARDED_BY(*DataGuard());
-
   // Current Flag Value
   FlagValue value_;
 

diff --git a/absl/strings/internal/str_format/bind.cc b/absl/strings/internal/str_format/bind.cc
@@ -77,7 +77,6 @@ inline bool ArgContext::Bind(const UnboundConversion* unbound,
     bound->set_precision(-1);
   }
 
-  bound->set_length_mod(unbound->length_mod);
   bound->set_conv(unbound->conv);
   bound->set_arg(arg);
   return true;
@@ -143,7 +142,7 @@ class SummarizingConverter {
     ss << "{" << Streamable(spec, {*bound.arg()}) << ":" << bound.flags();
     if (bound.width() >= 0) ss << bound.width();
     if (bound.precision() >= 0) ss << "." << bound.precision();
-    ss << bound.length_mod() << bound.conv() << "}";
+    ss << bound.conv() << "}";
     Append(ss.str());
     return true;
   }

diff --git a/absl/strings/internal/str_format/bind_test.cc b/absl/strings/internal/str_format/bind_test.cc
@@ -113,18 +113,18 @@ TEST_F(FormatBindTest, FormatPack) {
                                 FormatArgImpl(ia[2]), FormatArgImpl(ia[3]),
                                 FormatArgImpl(ia[4])};
   const Expectation kExpect[] = {
-    {__LINE__, "a%4db%dc", "a{10:4d}b{20:d}c"},
-    {__LINE__, "a%.4db%dc", "a{10:.4d}b{20:d}c"},
-    {__LINE__, "a%4.5db%dc", "a{10:4.5d}b{20:d}c"},
-    {__LINE__, "a%db%4.5dc", "a{10:d}b{20:4.5d}c"},
-    {__LINE__, "a%db%*.*dc", "a{10:d}b{40:20.30d}c"},
-    {__LINE__, "a%.*fb", "a{20:.10f}b"},
-    {__LINE__, "a%1$db%2$*3$.*4$dc", "a{10:d}b{20:30.40d}c"},
-    {__LINE__, "a%4$db%3$*2$.*1$dc", "a{40:d}b{30:20.10d}c"},
-    {__LINE__, "a%04ldb", "a{10:04ld}b"},
-    {__LINE__, "a%-#04lldb", "a{10:-#04lld}b"},
-    {__LINE__, "a%1$*5$db", "a{10:-10d}b"},
-    {__LINE__, "a%1$.*5$db", "a{10:d}b"},
+      {__LINE__, "a%4db%dc", "a{10:4d}b{20:d}c"},
+      {__LINE__, "a%.4db%dc", "a{10:.4d}b{20:d}c"},
+      {__LINE__, "a%4.5db%dc", "a{10:4.5d}b{20:d}c"},
+      {__LINE__, "a%db%4.5dc", "a{10:d}b{20:4.5d}c"},
+      {__LINE__, "a%db%*.*dc", "a{10:d}b{40:20.30d}c"},
+      {__LINE__, "a%.*fb", "a{20:.10f}b"},
+      {__LINE__, "a%1$db%2$*3$.*4$dc", "a{10:d}b{20:30.40d}c"},
+      {__LINE__, "a%4$db%3$*2$.*1$dc", "a{40:d}b{30:20.10d}c"},
+      {__LINE__, "a%04ldb", "a{10:04d}b"},
+      {__LINE__, "a%-#04lldb", "a{10:-#04d}b"},
+      {__LINE__, "a%1$*5$db", "a{10:-10d}b"},
+      {__LINE__, "a%1$.*5$db", "a{10:d}b"},
   };
   for (const Expectation &e : kExpect) {
     absl::string_view fmt = e.fmt;

diff --git a/absl/strings/internal/str_format/extension.cc b/absl/strings/internal/str_format/extension.cc
@@ -22,28 +22,6 @@
 namespace absl {
 ABSL_NAMESPACE_BEGIN
 namespace str_format_internal {
-namespace {
-// clang-format off
-#define ABSL_LENGTH_MODS_EXPAND_ \
-  X_VAL(h) X_SEP \
-  X_VAL(hh) X_SEP \
-  X_VAL(l) X_SEP \
-  X_VAL(ll) X_SEP \
-  X_VAL(L) X_SEP \
-  X_VAL(j) X_SEP \
-  X_VAL(z) X_SEP \
-  X_VAL(t) X_SEP \
-  X_VAL(q)
-// clang-format on
-}  // namespace
-
-const LengthMod::Spec LengthMod::kSpecs[] = {
-#define X_VAL(id) { LengthMod::id, #id, strlen(#id) }
-#define X_SEP ,
-    ABSL_LENGTH_MODS_EXPAND_, {LengthMod::none, "", 0}
-#undef X_VAL
-#undef X_SEP
-};
 
 const ConversionChar::Spec ConversionChar::kSpecs[] = {
 #define X_VAL(id) { ConversionChar::id, #id[0] }
@@ -64,8 +42,6 @@ std::string Flags::ToString() const {
   return s;
 }
 
-const size_t LengthMod::kNumValues;
-
 const size_t ConversionChar::kNumValues;
 
 bool FormatSinkImpl::PutPaddedString(string_view v, int w, int p, bool l) {

diff --git a/absl/strings/internal/str_format/extension.h b/absl/strings/internal/str_format/extension.h
@@ -136,54 +136,6 @@ struct Flags {
   }
 };
 
-struct ABSL_DLL LengthMod {
- public:
-  enum Id : uint8_t {
-    h, hh, l, ll, L, j, z, t, q, none
-  };
-  static const size_t kNumValues = none + 1;
-
-  LengthMod() : id_(none) {}
-
-  // Index into the opaque array of LengthMod enums.
-  // Requires: i < kNumValues
-  static LengthMod FromIndex(size_t i) {
-    return LengthMod(kSpecs[i].value);
-  }
-
-  static LengthMod FromId(Id id) { return LengthMod(id); }
-
-  // The length modifier std::string associated with a specified LengthMod.
-  string_view name() const {
-    const Spec& spec = kSpecs[id_];
-    return {spec.name, spec.name_length};
-  }
-
-  Id id() const { return id_; }
-
-  friend bool operator==(const LengthMod& a, const LengthMod& b) {
-    return a.id() == b.id();
-  }
-  friend bool operator!=(const LengthMod& a, const LengthMod& b) {
-    return !(a == b);
-  }
-  friend std::ostream& operator<<(std::ostream& os, const LengthMod& v) {
-    return os << v.name();
-  }
-
- private:
-  struct Spec {
-    Id value;
-    const char *name;
-    size_t name_length;
-  };
-  static const Spec kSpecs[];
-
-  explicit LengthMod(Id id) : id_(id) {}
-
-  Id id_;
-};
-
 // clang-format off
 #define ABSL_CONVERSION_CHARS_EXPAND_(X_VAL, X_SEP) \
   /* text */ \
@@ -306,7 +258,6 @@ struct ABSL_DLL ConversionChar {
 class ConversionSpec {
  public:
   Flags flags() const { return flags_; }
-  LengthMod length_mod() const { return length_mod_; }
   ConversionChar conv() const {
     // Keep this field first in the struct . It generates better code when
     // accessing it when ConversionSpec is passed by value in registers.
@@ -322,7 +273,6 @@ class ConversionSpec {
   int precision() const { return precision_; }
 
   void set_flags(Flags f) { flags_ = f; }
-  void set_length_mod(LengthMod lm) { length_mod_ = lm; }
   void set_conv(ConversionChar c) { conv_ = c; }
   void set_width(int w) { width_ = w; }
   void set_precision(int p) { precision_ = p; }
@@ -331,7 +281,6 @@ class ConversionSpec {
  private:
   ConversionChar conv_;
   Flags flags_;
-  LengthMod length_mod_;
   int width_;
   int precision_;
 };

diff --git a/absl/strings/internal/str_format/parser.cc b/absl/strings/internal/str_format/parser.cc
@@ -18,7 +18,8 @@ ABSL_NAMESPACE_BEGIN
 namespace str_format_internal {
 
 using CC = ConversionChar::Id;
-using LM = LengthMod::Id;
+using LM = LengthMod;
+
 ABSL_CONST_INIT const ConvTag kTags[256] = {
     {},    {},    {},    {},    {},    {},    {},    {},     // 00-07
     {},    {},    {},    {},    {},    {},    {},    {},     // 08-0f
@@ -205,11 +206,11 @@ const char *ConsumeConversion(const char *pos, const char *const end,
     using str_format_internal::LengthMod;
     LengthMod length_mod = tag.as_length();
     ABSL_FORMAT_PARSER_INTERNAL_GET_CHAR();
-    if (c == 'h' && length_mod.id() == LengthMod::h) {
-      conv->length_mod = LengthMod::FromId(LengthMod::hh);
+    if (c == 'h' && length_mod == LengthMod::h) {
+      conv->length_mod = LengthMod::hh;
       ABSL_FORMAT_PARSER_INTERNAL_GET_CHAR();
-    } else if (c == 'l' && length_mod.id() == LengthMod::l) {
-      conv->length_mod = LengthMod::FromId(LengthMod::ll);
+    } else if (c == 'l' && length_mod == LengthMod::l) {
+      conv->length_mod = LengthMod::ll;
       ABSL_FORMAT_PARSER_INTERNAL_GET_CHAR();
     } else {
       conv->length_mod = length_mod;
@@ -228,6 +229,32 @@ const char *ConsumeConversion(const char *pos, const char *const end,
 
 }  // namespace
 
+std::string LengthModToString(LengthMod v) {
+  switch (v) {
+    case LengthMod::h:
+      return "h";
+    case LengthMod::hh:
+      return "hh";
+    case LengthMod::l:
+      return "l";
+    case LengthMod::ll:
+      return "ll";
+    case LengthMod::L:
+      return "L";
+    case LengthMod::j:
+      return "j";
+    case LengthMod::z:
+      return "z";
+    case LengthMod::t:
+      return "t";
+    case LengthMod::q:
+      return "q";
+    case LengthMod::none:
+      return "";
+  }
+  return "";
+}
+
 const char *ConsumeUnboundConversion(const char *p, const char *end,
                                      UnboundConversion *conv, int *next_arg) {
   if (*next_arg < 0) return ConsumeConversion<true>(p, end, conv, next_arg);