Do not pass flag type as a char literal when registering a new flag.
It is possible to create a type-safe version of FlagRegisterer
ctor (as well as some internal gflags classes), that will deduce
type of the new flag automatically.
This results in removing quite a few calls to strcmp() when new
flag is created.
No existing behavior change.
diff --git a/src/gflags.cc b/src/gflags.cc
index 1619e9c..56414ec 100644
--- a/src/gflags.cc
+++ b/src/gflags.cc
@@ -191,17 +191,21 @@
class CommandLineFlag;
class FlagValue {
public:
- FlagValue(void* valbuf, const char* type, bool transfer_ownership_of_value);
+ template <typename FlagType>
+ FlagValue(FlagType* valbuf, bool transfer_ownership_of_value);
~FlagValue();
bool ParseFrom(const char* spec);
string ToString() const;
+ template <typename FlagType>
+ bool OfType() const { return type_ == FlagValueTraits<FlagType>::kValueType; }
+
private:
friend class CommandLineFlag; // for many things, including Validate()
friend class GFLAGS_NAMESPACE::FlagSaverImpl; // calls New()
friend class FlagRegistry; // checks value_buffer_ for flags_by_ptr_ map
- template <typename T> friend T GetFromEnv(const char*, const char*, T);
+ template <typename T> friend T GetFromEnv(const char*, T);
friend bool TryParseLocked(const CommandLineFlag*, FlagValue*,
const char*, string*); // for New(), CopyFrom()
@@ -215,6 +219,10 @@
FV_STRING = 6,
FV_MAX_INDEX = 6,
};
+
+ template <typename FlagType>
+ struct FlagValueTraits;
+
const char* TypeName() const;
bool Equal(const FlagValue& x) const;
FlagValue* New() const; // creates a new one with default value
@@ -227,14 +235,33 @@
// (*validate_fn)(bool) for a bool flag).
bool Validate(const char* flagname, ValidateFnProto validate_fn_proto) const;
- void* value_buffer_; // points to the buffer holding our data
- int8 type_; // how to interpret value_
- bool owns_value_; // whether to free value on destruct
+ void* const value_buffer_; // points to the buffer holding our data
+ const int8 type_; // how to interpret value_
+ const bool owns_value_; // whether to free value on destruct
FlagValue(const FlagValue&); // no copying!
void operator=(const FlagValue&);
};
+// Map the given C++ type to a value of the ValueType enum at compile time.
+#define DEFINE_FLAG_TRAITS(type, value) \
+ template <> \
+ struct FlagValue::FlagValueTraits<type> { \
+ static const ValueType kValueType = value; \
+ }
+
+// Define full template specializations of the FlagValueTraits template
+// for all supported flag types.
+DEFINE_FLAG_TRAITS(bool, FV_BOOL);
+DEFINE_FLAG_TRAITS(int32, FV_INT32);
+DEFINE_FLAG_TRAITS(uint32, FV_UINT32);
+DEFINE_FLAG_TRAITS(int64, FV_INT64);
+DEFINE_FLAG_TRAITS(uint64, FV_UINT64);
+DEFINE_FLAG_TRAITS(double, FV_DOUBLE);
+DEFINE_FLAG_TRAITS(std::string, FV_STRING);
+
+#undef DEFINE_FLAG_TRAITS
+
// This could be a templated method of FlagValue, but doing so adds to the
// size of the .o. Since there's no type-safety here anyway, macro is ok.
@@ -242,15 +269,12 @@
#define OTHER_VALUE_AS(fv, type) *reinterpret_cast<type*>(fv.value_buffer_)
#define SET_VALUE_AS(type, value) VALUE_AS(type) = (value)
-FlagValue::FlagValue(void* valbuf, const char* type,
+template <typename FlagType>
+FlagValue::FlagValue(FlagType* valbuf,
bool transfer_ownership_of_value)
: value_buffer_(valbuf),
+ type_(FlagValueTraits<FlagType>::kValueType),
owns_value_(transfer_ownership_of_value) {
- for (type_ = 0; type_ <= FV_MAX_INDEX; ++type_) {
- if (!strcmp(type, TypeName())) {
- break;
- }
- }
assert(type_ <= FV_MAX_INDEX); // Unknown typename
}
@@ -438,15 +462,14 @@
}
FlagValue* FlagValue::New() const {
- const char *type = TypeName();
switch (type_) {
- case FV_BOOL: return new FlagValue(new bool(false), type, true);
- case FV_INT32: return new FlagValue(new int32(0), type, true);
- case FV_UINT32: return new FlagValue(new uint32(0), type, true);
- case FV_INT64: return new FlagValue(new int64(0), type, true);
- case FV_UINT64: return new FlagValue(new uint64(0), type, true);
- case FV_DOUBLE: return new FlagValue(new double(0.0), type, true);
- case FV_STRING: return new FlagValue(new string, type, true);
+ case FV_BOOL: return new FlagValue(new bool(false), true);
+ case FV_INT32: return new FlagValue(new int32(0), true);
+ case FV_UINT32: return new FlagValue(new uint32(0), true);
+ case FV_INT64: return new FlagValue(new int64(0), true);
+ case FV_UINT64: return new FlagValue(new uint64(0), true);
+ case FV_DOUBLE: return new FlagValue(new double(0.0), true);
+ case FV_STRING: return new FlagValue(new string, true);
default: assert(false); return NULL; // unknown type
}
}
@@ -510,6 +533,9 @@
ValidateFnProto validate_function() const { return validate_fn_proto_; }
const void* flag_ptr() const { return current_->value_buffer_; }
+ template <typename FlagType>
+ bool OfType() const { return defvalue_->OfType<FlagType>(); }
+
void FillCommandLineFlagInfo(struct CommandLineFlagInfo* result);
// If validate_fn_proto_ is non-NULL, calls it on value, returns result.
@@ -800,7 +826,7 @@
kError, key->c_str());
return NULL;
}
- if (strcmp(flag->type_name(), "bool") != 0) {
+ if (!flag->OfType<bool>()) {
// 'x' exists but is not boolean, so we're not in the exception case.
*error_message = StringPrintf(
"%sboolean value (%s) specified for %s command line flag\n",
@@ -814,7 +840,7 @@
}
// Assign a value if this is a boolean flag
- if (*v == NULL && strcmp(flag->type_name(), "bool") == 0) {
+ if (*v == NULL && flag->OfType<bool>()) {
*v = "1"; // the --nox case was already handled, so this is the --x case
}
@@ -1073,7 +1099,7 @@
if (value == NULL) {
// Boolean options are always assigned a value by SplitArgumentLocked()
- assert(strcmp(flag->type_name(), "bool") != 0);
+ assert(!flag->OfType<bool>());
if (i+1 >= first_nonopt) {
// This flag needs a value, but there is nothing available
error_flags_[key] = (string(kError) + "flag '" + (*argv)[i] + "'"
@@ -1098,7 +1124,7 @@
// "-lat -30.5" would trigger the warning. The common cases we
// want to solve talk about true and false as values.
if (value[0] == '-'
- && strcmp(flag->type_name(), "string") == 0
+ && flag->OfType<string>()
&& (strstr(flag->help(), "true")
|| strstr(flag->help(), "false"))) {
LOG(WARNING) << "Did you really mean to set flag '"
@@ -1163,8 +1189,8 @@
}
const string envname = string("FLAGS_") + string(flagname);
- string envval;
- if (!SafeGetEnv(envname.c_str(), envval)) {
+ string envval;
+ if (!SafeGetEnv(envname.c_str(), envval)) {
if (errors_are_fatal) {
error_flags_[flagname] = (string(kError) + envname +
" not found in environment\n");
@@ -1362,14 +1388,14 @@
// --------------------------------------------------------------------
template<typename T>
-T GetFromEnv(const char *varname, const char* type, T dflt) {
+T GetFromEnv(const char *varname, T dflt) {
std::string valstr;
if (SafeGetEnv(varname, valstr)) {
- FlagValue ifv(new T, type, true);
+ FlagValue ifv(new T, true);
if (!ifv.ParseFrom(valstr.c_str())) {
ReportError(DIE, "ERROR: error parsing env variable '%s' with value '%s'\n",
varname, valstr.c_str());
- }
+ }
return OTHER_VALUE_AS(ifv, T);
} else return dflt;
}
@@ -1416,23 +1442,37 @@
// values in a global destructor.
// --------------------------------------------------------------------
-FlagRegisterer::FlagRegisterer(const char* name, const char* type,
+template <typename FlagType>
+FlagRegisterer::FlagRegisterer(const char* name,
const char* help, const char* filename,
- void* current_storage, void* defvalue_storage) {
+ FlagType* current_storage, FlagType* defvalue_storage) {
if (help == NULL)
help = "";
- // FlagValue expects the type-name to not include any namespace
- // components, so we get rid of those, if any.
- if (strchr(type, ':'))
- type = strrchr(type, ':') + 1;
- FlagValue* current = new FlagValue(current_storage, type, false);
- FlagValue* defvalue = new FlagValue(defvalue_storage, type, false);
+ FlagValue* current = new FlagValue(current_storage, false);
+ FlagValue* defvalue = new FlagValue(defvalue_storage, false);
// Importantly, flag_ will never be deleted, so storage is always good.
CommandLineFlag* flag = new CommandLineFlag(name, help, filename,
current, defvalue);
FlagRegistry::GlobalRegistry()->RegisterFlag(flag); // default registry
}
+// Force compiler to generate code for the given template specialization.
+#define INSTANTIATE_FLAG_REGISTERER_CTOR(type) \
+ template FlagRegisterer::FlagRegisterer( \
+ const char* name, const char* help, const char* filename, \
+ type* current_storage, type* defvalue_storage)
+
+// Do this for all supported flag types.
+INSTANTIATE_FLAG_REGISTERER_CTOR(bool);
+INSTANTIATE_FLAG_REGISTERER_CTOR(int32);
+INSTANTIATE_FLAG_REGISTERER_CTOR(uint32);
+INSTANTIATE_FLAG_REGISTERER_CTOR(int64);
+INSTANTIATE_FLAG_REGISTERER_CTOR(uint64);
+INSTANTIATE_FLAG_REGISTERER_CTOR(double);
+INSTANTIATE_FLAG_REGISTERER_CTOR(std::string);
+
+#undef INSTANTIATE_FLAG_REGISTERER_CTOR
+
// --------------------------------------------------------------------
// GetAllFlags()
// The main way the FlagRegistry class exposes its data. This
@@ -1820,22 +1860,22 @@
// --------------------------------------------------------------------
bool BoolFromEnv(const char *v, bool dflt) {
- return GetFromEnv(v, "bool", dflt);
+ return GetFromEnv(v, dflt);
}
int32 Int32FromEnv(const char *v, int32 dflt) {
- return GetFromEnv(v, "int32", dflt);
+ return GetFromEnv(v, dflt);
}
uint32 Uint32FromEnv(const char *v, uint32 dflt) {
- return GetFromEnv(v, "uint32", dflt);
+ return GetFromEnv(v, dflt);
}
int64 Int64FromEnv(const char *v, int64 dflt) {
- return GetFromEnv(v, "int64", dflt);
+ return GetFromEnv(v, dflt);
}
uint64 Uint64FromEnv(const char *v, uint64 dflt) {
- return GetFromEnv(v, "uint64", dflt);
+ return GetFromEnv(v, dflt);
}
double DoubleFromEnv(const char *v, double dflt) {
- return GetFromEnv(v, "double", dflt);
+ return GetFromEnv(v, dflt);
}
#ifdef _MSC_VER
diff --git a/src/gflags.h.in b/src/gflags.h.in
index 03b7776..09daac3 100644
--- a/src/gflags.h.in
+++ b/src/gflags.h.in
@@ -431,9 +431,14 @@
class GFLAGS_DLL_DECL FlagRegisterer {
public:
- FlagRegisterer(const char* name, const char* type,
+ // We instantiate this template ctor for all supported types,
+ // so it is possible to place implementation of the FlagRegisterer ctor in
+ // .cc file.
+ // Calling this constructor with unsupported type will produce linker error.
+ template <typename FlagType>
+ FlagRegisterer(const char* name,
const char* help, const char* filename,
- void* current_storage, void* defvalue_storage);
+ FlagType* current_storage, FlagType* defvalue_storage);
};
// If your application #defines STRIP_FLAG_HELP to a non-zero value
@@ -475,7 +480,7 @@
GFLAGS_DLL_DEFINE_FLAG type FLAGS_##name = FLAGS_nono##name; \
type FLAGS_no##name = FLAGS_nono##name; \
static GFLAGS_NAMESPACE::FlagRegisterer o_##name( \
- #name, #type, MAYBE_STRIPPED_HELP(help), __FILE__, \
+ #name, MAYBE_STRIPPED_HELP(help), __FILE__, \
&FLAGS_##name, &FLAGS_no##name); \
} \
using fL##shorttype::FLAGS_##name
@@ -581,8 +586,8 @@
dont_pass0toDEFINE_string(s_##name[0].s, \
val); \
static GFLAGS_NAMESPACE::FlagRegisterer o_##name( \
- #name, "string", MAYBE_STRIPPED_HELP(txt), __FILE__, \
- s_##name[0].s, new (s_##name[1].s) clstring(*FLAGS_no##name)); \
+ #name, MAYBE_STRIPPED_HELP(txt), __FILE__, \
+ FLAGS_no##name, new (s_##name[1].s) clstring(*FLAGS_no##name)); \
static StringFlagDestructor d_##name(s_##name[0].s, s_##name[1].s); \
extern GFLAGS_DLL_DEFINE_FLAG clstring& FLAGS_##name; \
using fLS::FLAGS_##name; \
diff --git a/test/gflags_unittest.cc b/test/gflags_unittest.cc
index 4c079ba..47dfd3c 100755
--- a/test/gflags_unittest.cc
+++ b/test/gflags_unittest.cc
@@ -216,7 +216,7 @@
int32 FLAGS_tldflag1 = FLAGS_nonotldflag1;
int32 FLAGS_notldflag1 = FLAGS_nonotldflag1;
static FlagRegisterer o_tldflag1(
- "tldflag1", "int32",
+ "tldflag1",
"should show up in --helpshort", "gflags_unittest.cc",
&FLAGS_tldflag1, &FLAGS_notldflag1);
}
@@ -227,7 +227,7 @@
int32 FLAGS_tldflag2 = FLAGS_nonotldflag2;
int32 FLAGS_notldflag2 = FLAGS_nonotldflag2;
static FlagRegisterer o_tldflag2(
- "tldflag2", "int32",
+ "tldflag2",
"should show up in --helpshort", "gflags_unittest.",
&FLAGS_tldflag2, &FLAGS_notldflag2);
}
@@ -1355,7 +1355,7 @@
// addresses of these variables will be overwritten... Stack smash!
static bool current_storage;
static bool defvalue_storage;
- FlagRegisterer fr("flag_name", "bool", 0, "filename",
+ FlagRegisterer fr("flag_name", NULL, "filename",
¤t_storage, &defvalue_storage);
CommandLineFlagInfo fi;
EXPECT_TRUE(GetCommandLineFlagInfo("flag_name", &fi));
