NOTE: I'm checking this in just to have a record in source control of the idea.  We've decided for now it doesn't make sense to go forward with flag categories -- and the __VA_ARGS__ for macros has caused lots of problems with uncaught accidental commas, already -- so a future commit will back this out.  (Actually, it's all a series of commits, for annoying technical reasons.)

Add support for flag categories.

In this CL, all you can do is set categories in the DEFINE_*
macros and then retrieve them via GetCommandLineFlagInfo and
similar.

In future CLs, we will start to give some semantic meaning to
particular flag values, as described in the designdoc.  In
particular, we will start to use flag categories to revamp
--help output.

Implementation-wise: to keep categories an optional macro
argument, I had to use __VA_ARGS__, which means future gflags
releases will no longer work with MSVC 7.1.  We're at MSVC 10
now, so I'm pretty much ok with that.

The downside of __VA_ARGS__ is there is no error if you
specify more args after the ones we expect.  To get around
that, I only use __VA_ARGS_ in this idiom:
static const OptionalDefineArgs var = { __VA_ARGS__ };
The new OptionalDefineArgs struct defines all the args that
may be optionally specified in the DEFINE_* macros.  For now,
that's only the 'categories' arg, though in theory more could be
added later.

R=titus,ncalvin
DELTA=92  (54 added, 3 deleted, 35 changed)


Revision created by MOE tool push_codebase.
MOE_MIGRATION=3057


git-svn-id: http://gflags.googlecode.com/svn/trunk/src@61 6586e3c6-dcc4-952a-343f-ff74eb82781d
diff --git a/gflags.cc b/gflags.cc
index 63cee62..26955a8 100644
--- a/gflags.cc
+++ b/gflags.cc
@@ -112,6 +112,8 @@
 #include "mutex.h"
 #include "util.h"
 
+using fL::OptionalDefineArgs;
+
 #ifndef PATH_SEPARATOR
 #define PATH_SEPARATOR  '/'
 #endif
@@ -481,12 +483,14 @@
  public:
   // Note: we take over memory-ownership of current_val and default_val.
   CommandLineFlag(const char* name, const char* help, const char* filename,
+                  const char* categories,
                   FlagValue* current_val, FlagValue* default_val);
   ~CommandLineFlag();
 
   const char* name() const { return name_; }
   const char* help() const { return help_; }
   const char* filename() const { return file_; }
+  const char* categories() const { return categories_ ? categories_ : ""; }
   const char* CleanFileName() const;  // nixes irrelevant prefix such as homedir
   string current_value() const { return current_->ToString(); }
   string default_value() const { return defvalue_->ToString(); }
@@ -514,6 +518,7 @@
   const char* const name_;     // Flag name
   const char* const help_;     // Help message
   const char* const file_;     // Which file did this come from?
+  const char* categories_;     // Comma-separated list of flag's 'categories'
   bool modified_;              // Set after default assignment?
   FlagValue* defvalue_;        // Default value for flag
   FlagValue* current_;         // Current value for flag
@@ -528,10 +533,11 @@
 };
 
 CommandLineFlag::CommandLineFlag(const char* name, const char* help,
-                                 const char* filename,
+                                 const char* filename, const char* categories,
                                  FlagValue* current_val, FlagValue* default_val)
-    : name_(name), help_(help), file_(filename), modified_(false),
-      defvalue_(default_val), current_(current_val), validate_fn_proto_(NULL) {
+    : name_(name), help_(help), file_(filename), categories_(categories),
+      modified_(false), defvalue_(default_val), current_(current_val),
+      validate_fn_proto_(NULL) {
 }
 
 CommandLineFlag::~CommandLineFlag() {
@@ -567,6 +573,7 @@
   result->name = name();
   result->type = type_name();
   result->description = help();
+  result->categories = categories();
   result->current_value = current_value();
   result->default_value = default_value();
   result->filename = CleanFileName();
@@ -1397,7 +1404,8 @@
 
 FlagRegisterer::FlagRegisterer(const char* name, const char* type,
                                const char* help, const char* filename,
-                               void* current_storage, void* defvalue_storage) {
+                               void* current_storage, void* defvalue_storage,
+                               const OptionalDefineArgs& optional_args) {
   if (help == NULL)
     help = "";
   // FlagValue expects the type-name to not include any namespace
@@ -1408,6 +1416,7 @@
   FlagValue* defvalue = new FlagValue(defvalue_storage, type, false);
   // Importantly, flag_ will never be deleted, so storage is always good.
   CommandLineFlag* flag = new CommandLineFlag(name, help, filename,
+                                              optional_args.categories,
                                               current, defvalue);
   FlagRegistry::GlobalRegistry()->RegisterFlag(flag);   // default registry
 }
@@ -1652,7 +1661,7 @@
       const CommandLineFlag* main = it->second;
       // Sets up all the const variables in backup correctly
       CommandLineFlag* backup = new CommandLineFlag(
-          main->name(), main->help(), main->filename(),
+          main->name(), main->help(), main->filename(), main->categories(),
           main->current_->New(), main->defvalue_->New());
       // Sets up all the non-const variables in backup correctly
       backup->CopyFrom(*main);
diff --git a/gflags/gflags.h.in b/gflags/gflags.h.in
index 3b1aef7..2c495d4 100644
--- a/gflags/gflags.h.in
+++ b/gflags/gflags.h.in
@@ -40,7 +40,7 @@
 //
 //    DEFINE_int32(end, 1000, "The last record to read");
 //
-//    DEFINE_string(filename, "my_file.txt", "The file to read");
+//    DEFINE_string(filename, "my_file.txt", "The file to read", "important");
 //    // Crash if the specified file does not exist.
 //    static bool dummy = RegisterFlagValidator(&FLAGS_filename,
 //                                              &ValidateIsFile);
@@ -81,7 +81,6 @@
 #include <string>
 #include <vector>
 #include <gflags/gflags_declare.h>  // IWYU pragma: export
-@ac_google_start_namespace@
 
 //
 // NOTE: all functions below MUST have an explicit 'extern' before
@@ -91,6 +90,10 @@
 #define GFLAGS_DLL_DECL  /* rewritten to be non-empty in windows dir */
 #define GFLAGS_DLL_DEFINE_FLAG  /* rewritten to be non-empty in windows dir */
 
+namespace fL { struct OptionalDefineArgs; }   // defined below
+
+
+@ac_google_start_namespace@
 
 // --------------------------------------------------------------------
 // To actually define a flag in a file, use DEFINE_bool,
@@ -151,6 +154,7 @@
   std::string name;            // the name of the flag
   std::string type;            // the type of the flag: int32, etc
   std::string description;     // the "help text" associated with the flag
+  std::string categories;      // the value of the "categories" arg to DEFINE_*()
   std::string current_value;   // the current value, as a string
   std::string default_value;   // the default value, as a string
   std::string filename;        // 'cleaned' version of filename holding the flag
@@ -426,7 +430,8 @@
  public:
   FlagRegisterer(const char* name, const char* type,
                  const char* help, const char* filename,
-                 void* current_storage, void* defvalue_storage);
+                 void* current_storage, void* defvalue_storage,
+                 const fL::OptionalDefineArgs& optional_args);
 };
 
 // If your application #defines STRIP_FLAG_HELP to a non-zero value
@@ -448,6 +453,19 @@
 #define MAYBE_STRIPPED_HELP(txt) txt
 #endif
 
+// This holds all the optional macro argument fields that *may* be
+// present in DEFINE_* after the helpstring, but are not required to
+// be.  We use static initialization, so they must all be POD types,
+// and if not specified they default to 0.
+namespace fL {
+struct OptionalDefineArgs {
+  // A comma-separated list of "categories" this flag falls into.
+  // For details on categories, see gflags_categories.h.
+  const char* categories;
+};
+typedef OptionalDefineArgs ODA;   // used in macros to save on code size
+}
+
 // Each command-line flag has two variables associated with it: one
 // with the current value, and one with the default value.  However,
 // we have a third variable, which is where value is assigned; it's a
@@ -459,15 +477,18 @@
 // FLAGS_no<name>.  This serves the second purpose of assuring a
 // compile error if someone tries to define a flag named no<name>
 // which is illegal (--foo and --nofoo both affect the "foo" flag).
-#define DEFINE_VARIABLE(type, shorttype, name, value, help)             \
+// The ... maps to the fields in OptionalDefineArgs, above.  It may
+// be omitted entirely if no optional args need to be specified.
+#define DEFINE_VARIABLE(type, shorttype, name, value, help, ...)        \
   namespace fL##shorttype {                                             \
-    static const type FLAGS_nono##name = value;                         \
+    static const type v_##name = value;                                 \
+    static const ::fL::ODA e_##name = { __VA_ARGS__ };                  \
     /* We always want to export defined variables, dll or no */         \
-    GFLAGS_DLL_DEFINE_FLAG type FLAGS_##name = FLAGS_nono##name;        \
-    type FLAGS_no##name = FLAGS_nono##name;                             \
+    GFLAGS_DLL_DEFINE_FLAG type FLAGS_##name = v_##name;                \
+    type FLAGS_no##name = v_##name;                                     \
     static @ac_google_namespace@::FlagRegisterer o_##name( \
       #name, #type, MAYBE_STRIPPED_HELP(help), __FILE__,                \
-      &FLAGS_##name, &FLAGS_no##name);                                  \
+      &FLAGS_##name, &FLAGS_no##name, e_##name);                        \
   }                                                                     \
   using fL##shorttype::FLAGS_##name
 
@@ -490,27 +511,28 @@
 // Here are the actual DEFINE_*-macros. The respective DECLARE_*-macros
 // are in a separate include, gflags_declare.h, for reducing
 // the physical transitive size for DECLARE use.
-#define DEFINE_bool(name, val, txt)                                     \
+// As always, the ... maps to the fields in OptionalDefineArgs, above.
+#define DEFINE_bool(name, val, txt, ...)                                \
   namespace fLB {                                                       \
     typedef ::fLB::CompileAssert FLAG_##name##_value_is_not_a_bool[     \
             (sizeof(::fLB::IsBoolFlag(val)) != sizeof(double)) ? 1 : -1]; \
   }                                                                     \
-  DEFINE_VARIABLE(bool, B, name, val, txt)
+  DEFINE_VARIABLE(bool, B, name, val, txt, __VA_ARGS__)
 
-#define DEFINE_int32(name, val, txt) \
+#define DEFINE_int32(name, val, txt, ...)                           \
    DEFINE_VARIABLE(@ac_google_namespace@::int32, I, \
-                   name, val, txt)
+                   name, val, txt, __VA_ARGS__)
 
-#define DEFINE_int64(name, val, txt) \
+#define DEFINE_int64(name, val, txt, ...)                             \
    DEFINE_VARIABLE(@ac_google_namespace@::int64, I64, \
-                   name, val, txt)
+                   name, val, txt, __VA_ARGS__)
 
-#define DEFINE_uint64(name,val, txt) \
+#define DEFINE_uint64(name,val, txt, ...)                              \
    DEFINE_VARIABLE(@ac_google_namespace@::uint64, U64, \
-                   name, val, txt)
+                   name, val, txt, __VA_ARGS__)
 
-#define DEFINE_double(name, val, txt) \
-   DEFINE_VARIABLE(double, D, name, val, txt)
+#define DEFINE_double(name, val, txt, ...)                      \
+   DEFINE_VARIABLE(double, D, name, val, txt, __VA_ARGS__)
 
 // Strings are trickier, because they're not a POD, so we can't
 // construct them at static-initialization time (instead they get
@@ -540,16 +562,19 @@
 // The weird 'using' + 'extern' inside the fLS namespace is to work around
 // an unknown compiler bug/issue with the gcc 4.2.1 on SUSE 10.  See
 //    http://code.google.com/p/google-gflags/issues/detail?id=20
-#define DEFINE_string(name, val, txt)                                       \
+// As always, the ... maps to the fields in OptionalDefineArgs, above.
+#define DEFINE_string(name, val, txt, ...)                                  \
   namespace fLS {                                                           \
     using ::fLS::clstring;                                                  \
     static union { void* align; char s[sizeof(clstring)]; } s_##name[2];    \
     clstring* const FLAGS_no##name = ::fLS::                                \
                                    dont_pass0toDEFINE_string(s_##name[0].s, \
                                                              val);          \
+    static const ::fL::ODA e_##name = { __VA_ARGS__ };                      \
     static @ac_google_namespace@::FlagRegisterer o_##name(  \
         #name, "string", MAYBE_STRIPPED_HELP(txt), __FILE__,                \
-        s_##name[0].s, new (s_##name[1].s) clstring(*FLAGS_no##name));      \
+        s_##name[0].s, new (s_##name[1].s) clstring(*FLAGS_no##name),       \
+        e_##name);                                                          \
     extern GFLAGS_DLL_DEFINE_FLAG clstring& FLAGS_##name;                   \
     using fLS::FLAGS_##name;                                                \
     clstring& FLAGS_##name = *FLAGS_no##name;                               \
diff --git a/gflags_unittest.cc b/gflags_unittest.cc
index 890e157..c7cb30d 100644
--- a/gflags_unittest.cc
+++ b/gflags_unittest.cc
@@ -77,7 +77,8 @@
 
 DECLARE_string(tryfromenv);   // in gflags.cc
 
-DEFINE_bool(test_bool, false, "tests bool-ness");
+// This 4th arg specifies the 'categories' the flag belongs to.
+DEFINE_bool(test_bool, false, "tests bool-ness", "important,has_category");
 DEFINE_int32(test_int32, -1, "");
 DEFINE_int64(test_int64, -2, "");
 DEFINE_uint64(test_uint64, 2, "");
@@ -95,6 +96,7 @@
 DEFINE_string(test_str1, "initial", "");
 DEFINE_string(test_str2, "initial", "");
 DEFINE_string(test_str3, "initial", "");
+DEFINE_string(test_str_with_category, "", "", "required,filename");
 
 // This is used to test setting tryfromenv manually
 DEFINE_string(test_tryfromenv, "initial", "");
@@ -216,6 +218,8 @@
 #undef MAKEFLAG10
 #undef MAKEFLAG
 
+static fL::OptionalDefineArgs no_optional_args = { };
+
 // This is a pseudo-flag -- we want to register a flag with a filename
 // at the top level, but there is no way to do this except by faking
 // the filename.
@@ -226,7 +230,7 @@
   static FlagRegisterer o_tldflag1(
     "tldflag1", "int32",
     "should show up in --helpshort", "gflags_unittest.cc",
-    &FLAGS_tldflag1, &FLAGS_notldflag1);
+    &FLAGS_tldflag1, &FLAGS_notldflag1, no_optional_args);
 }
 using fLI::FLAGS_tldflag1;
 
@@ -237,7 +241,7 @@
   static FlagRegisterer o_tldflag2(
     "tldflag2", "int32",
     "should show up in --helpshort", "gflags_unittest.",
-    &FLAGS_tldflag2, &FLAGS_notldflag2);
+    &FLAGS_tldflag2, &FLAGS_notldflag2, no_optional_args);
 }
 using fLI::FLAGS_tldflag2;
 
@@ -986,17 +990,30 @@
   EXPECT_EQ("test_int32", info.name);
   EXPECT_EQ("int32", info.type);
   EXPECT_EQ("", info.description);
+  EXPECT_EQ("", info.categories);
   EXPECT_EQ("-1", info.current_value);
   EXPECT_EQ("-1", info.default_value);
   EXPECT_TRUE(info.is_default);
   EXPECT_FALSE(info.has_validator_fn);
 
+  r = GetCommandLineFlagInfo("test_str_with_category", &info);
+  EXPECT_TRUE(r);
+  EXPECT_EQ("test_str_with_category", info.name);
+  EXPECT_EQ("string", info.type);
+  EXPECT_EQ("", info.description);
+  EXPECT_EQ("required,filename", info.categories);
+  EXPECT_EQ("", info.current_value);
+  EXPECT_EQ("", info.default_value);
+  EXPECT_TRUE(info.is_default);
+  EXPECT_FALSE(info.has_validator_fn);
+
   FLAGS_test_bool = true;
   r = GetCommandLineFlagInfo("test_bool", &info);
   EXPECT_TRUE(r);
   EXPECT_EQ("test_bool", info.name);
   EXPECT_EQ("bool", info.type);
   EXPECT_EQ("tests bool-ness", info.description);
+  EXPECT_EQ("important,has_category", info.categories);
   EXPECT_EQ("true", info.current_value);
   EXPECT_EQ("false", info.default_value);
   EXPECT_FALSE(info.is_default);
@@ -1008,6 +1025,7 @@
   EXPECT_EQ("test_bool", info.name);
   EXPECT_EQ("bool", info.type);
   EXPECT_EQ("tests bool-ness", info.description);
+  EXPECT_EQ("important,has_category", info.categories);
   EXPECT_EQ("false", info.current_value);
   EXPECT_EQ("false", info.default_value);
   EXPECT_FALSE(info.is_default);  // value is same, but flag *was* modified
@@ -1335,7 +1353,7 @@
   static bool current_storage;
   static bool defvalue_storage;
   FlagRegisterer fr("flag_name", "bool", 0, "filename",
-                    &current_storage, &defvalue_storage);
+                    &current_storage, &defvalue_storage, no_optional_args);
   CommandLineFlagInfo fi;
   EXPECT_TRUE(GetCommandLineFlagInfo("flag_name", &fi));
   EXPECT_EQ("", fi.description);
diff --git a/windows/gflags/gflags.h b/windows/gflags/gflags.h
index c2827d8..bfaab47 100644
--- a/windows/gflags/gflags.h
+++ b/windows/gflags/gflags.h
@@ -40,7 +40,7 @@
 //
 //    DEFINE_int32(end, 1000, "The last record to read");
 //
-//    DEFINE_string(filename, "my_file.txt", "The file to read");
+//    DEFINE_string(filename, "my_file.txt", "The file to read", "important");
 //    // Crash if the specified file does not exist.
 //    static bool dummy = RegisterFlagValidator(&FLAGS_filename,
 //                                              &ValidateIsFile);
@@ -81,7 +81,6 @@
 #include <string>
 #include <vector>
 #include <gflags/gflags_declare.h>  // IWYU pragma: export
-namespace google {
 
 //
 // NOTE: all functions below MUST have an explicit 'extern' before
@@ -95,6 +94,10 @@
 # define GFLAGS_DLL_DEFINE_FLAG  __declspec(dllexport)
 #endif
 
+namespace fL { struct OptionalDefineArgs; }   // defined below
+
+
+namespace google {
 
 // --------------------------------------------------------------------
 // To actually define a flag in a file, use DEFINE_bool,
@@ -155,6 +158,7 @@
   std::string name;            // the name of the flag
   std::string type;            // the type of the flag: int32, etc
   std::string description;     // the "help text" associated with the flag
+  std::string categories;      // the value of the "categories" arg to DEFINE_*()
   std::string current_value;   // the current value, as a string
   std::string default_value;   // the default value, as a string
   std::string filename;        // 'cleaned' version of filename holding the flag
@@ -430,7 +434,8 @@
  public:
   FlagRegisterer(const char* name, const char* type,
                  const char* help, const char* filename,
-                 void* current_storage, void* defvalue_storage);
+                 void* current_storage, void* defvalue_storage,
+                 const fL::OptionalDefineArgs& optional_args);
 };
 
 // If your application #defines STRIP_FLAG_HELP to a non-zero value
@@ -452,6 +457,19 @@
 #define MAYBE_STRIPPED_HELP(txt) txt
 #endif
 
+// This holds all the optional macro argument fields that *may* be
+// present in DEFINE_* after the helpstring, but are not required to
+// be.  We use static initialization, so they must all be POD types,
+// and if not specified they default to 0.
+namespace fL {
+struct OptionalDefineArgs {
+  // A comma-separated list of "categories" this flag falls into.
+  // For details on categories, see gflags_categories.h.
+  const char* categories;
+};
+typedef OptionalDefineArgs ODA;   // used in macros to save on code size
+}
+
 // Each command-line flag has two variables associated with it: one
 // with the current value, and one with the default value.  However,
 // we have a third variable, which is where value is assigned; it's a
@@ -463,15 +481,18 @@
 // FLAGS_no<name>.  This serves the second purpose of assuring a
 // compile error if someone tries to define a flag named no<name>
 // which is illegal (--foo and --nofoo both affect the "foo" flag).
-#define DEFINE_VARIABLE(type, shorttype, name, value, help)             \
+// The ... maps to the fields in OptionalDefineArgs, above.  It may
+// be omitted entirely if no optional args need to be specified.
+#define DEFINE_VARIABLE(type, shorttype, name, value, help, ...)        \
   namespace fL##shorttype {                                             \
-    static const type FLAGS_nono##name = value;                         \
+    static const type v_##name = value;                                 \
+    static const ::fL::ODA e_##name = { __VA_ARGS__ };                  \
     /* We always want to export defined variables, dll or no */         \
-    GFLAGS_DLL_DEFINE_FLAG type FLAGS_##name = FLAGS_nono##name;        \
-    type FLAGS_no##name = FLAGS_nono##name;                             \
+    GFLAGS_DLL_DEFINE_FLAG type FLAGS_##name = v_##name;                \
+    type FLAGS_no##name = v_##name;                                     \
     static ::google::FlagRegisterer o_##name( \
       #name, #type, MAYBE_STRIPPED_HELP(help), __FILE__,                \
-      &FLAGS_##name, &FLAGS_no##name);                                  \
+      &FLAGS_##name, &FLAGS_no##name, e_##name);                        \
   }                                                                     \
   using fL##shorttype::FLAGS_##name
 
@@ -494,27 +515,28 @@
 // Here are the actual DEFINE_*-macros. The respective DECLARE_*-macros
 // are in a separate include, gflags_declare.h, for reducing
 // the physical transitive size for DECLARE use.
-#define DEFINE_bool(name, val, txt)                                     \
+// As always, the ... maps to the fields in OptionalDefineArgs, above.
+#define DEFINE_bool(name, val, txt, ...)                                \
   namespace fLB {                                                       \
     typedef ::fLB::CompileAssert FLAG_##name##_value_is_not_a_bool[     \
             (sizeof(::fLB::IsBoolFlag(val)) != sizeof(double)) ? 1 : -1]; \
   }                                                                     \
-  DEFINE_VARIABLE(bool, B, name, val, txt)
+  DEFINE_VARIABLE(bool, B, name, val, txt, __VA_ARGS__)
 
-#define DEFINE_int32(name, val, txt) \
+#define DEFINE_int32(name, val, txt, ...)                           \
    DEFINE_VARIABLE(::google::int32, I, \
-                   name, val, txt)
+                   name, val, txt, __VA_ARGS__)
 
-#define DEFINE_int64(name, val, txt) \
+#define DEFINE_int64(name, val, txt, ...)                             \
    DEFINE_VARIABLE(::google::int64, I64, \
-                   name, val, txt)
+                   name, val, txt, __VA_ARGS__)
 
-#define DEFINE_uint64(name,val, txt) \
+#define DEFINE_uint64(name,val, txt, ...)                              \
    DEFINE_VARIABLE(::google::uint64, U64, \
-                   name, val, txt)
+                   name, val, txt, __VA_ARGS__)
 
-#define DEFINE_double(name, val, txt) \
-   DEFINE_VARIABLE(double, D, name, val, txt)
+#define DEFINE_double(name, val, txt, ...)                      \
+   DEFINE_VARIABLE(double, D, name, val, txt, __VA_ARGS__)
 
 // Strings are trickier, because they're not a POD, so we can't
 // construct them at static-initialization time (instead they get
@@ -544,16 +566,19 @@
 // The weird 'using' + 'extern' inside the fLS namespace is to work around
 // an unknown compiler bug/issue with the gcc 4.2.1 on SUSE 10.  See
 //    http://code.google.com/p/google-gflags/issues/detail?id=20
-#define DEFINE_string(name, val, txt)                                       \
+// As always, the ... maps to the fields in OptionalDefineArgs, above.
+#define DEFINE_string(name, val, txt, ...)                                  \
   namespace fLS {                                                           \
     using ::fLS::clstring;                                                  \
     static union { void* align; char s[sizeof(clstring)]; } s_##name[2];    \
     clstring* const FLAGS_no##name = ::fLS::                                \
                                    dont_pass0toDEFINE_string(s_##name[0].s, \
                                                              val);          \
+    static const ::fL::ODA e_##name = { __VA_ARGS__ };                      \
     static ::google::FlagRegisterer o_##name(  \
         #name, "string", MAYBE_STRIPPED_HELP(txt), __FILE__,                \
-        s_##name[0].s, new (s_##name[1].s) clstring(*FLAGS_no##name));      \
+        s_##name[0].s, new (s_##name[1].s) clstring(*FLAGS_no##name),       \
+        e_##name);                                                          \
     extern GFLAGS_DLL_DEFINE_FLAG clstring& FLAGS_##name;                   \
     using fLS::FLAGS_##name;                                                \
     clstring& FLAGS_##name = *FLAGS_no##name;                               \