Emit error if unknown switch is passed to mojo_shell

mojo/shell/switches.cc

Index: mojo/shell/switches.cc
diff --git a/mojo/shell/switches.cc b/mojo/shell/switches.cc
index bb159b5b8bd306b212d4e9edf2f3335318dfaef7..9b377e3d64c960ff5fb48a4953b410aa9e8dd976 100644
--- a/mojo/shell/switches.cc
+++ b/mojo/shell/switches.cc
@@ -4,6 +4,8 @@
 
 #include "mojo/shell/switches.h"
 
+#include "base/basictypes.h"
+
 namespace switches {
 
 // Specify configuration arguments for a Mojo application URL. For example:
@@ -49,4 +51,27 @@ const char kSpy[] = "spy";
 // first maps 'a' to 'b' and the second 'c' to 'd'.
 const char kURLMappings[] = "url-mappings";
 
+const char* switch_array[] = {
+  kArgsFor,

[Aaron Boodman, 2014/11/17 15:46:06]

This duplicates information. People are going to update the list of switches,
but forget to update this. In fact, it has already happened -- switch_array is
missing kContentHandlers.

I think if you want to make this change, you should find a way to avoid the
duplicate information. Somehow, defining a switch should add it to the list of
switches.

Sorry.

PS: naming-wise, it should be: kSwitchArray.

[DaveMoore, 2014/11/17 16:36:47]

Name changed. I didn't think you used kFoo for arrays.

It seems pretty unlikely that you'll add a switch and not add it to this array,
as then your switch can't work. kContentHandlers wasn't like that. It was simply
a failure of the cl (thanks).

[Aaron Boodman, 2014/11/17 16:44:45]

Sorry, I disagree. I think it is likely that people will add a switch and not
know or remember this all_switches variable. Especially if they aren't in the
same screenful of code.

Perhaps there is some way you can register the flag right below the line it is
defined... But I can't think of how to do that without significantly refactoring
how switches or adding a static vector.

+  kChildProcessType,
+  kDisableCache,
+  kEnableExternalApplications,
+  kEnableMultiprocess,
+  kHelp,
+  kOrigin,
+  kSpy,
+  kURLMappings
+};
+
+const std::set<std::string>& GetAllSwitches() {
+  static std::set<std::string> switch_set;

[hansmuller, 2014/11/17 15:33:19]

The switch_array and the cached set values aren't performance critical. It might
be better to just make switch_array[] a local in GetAllSwitches() and create the
set each time the function is called.

[Aaron Boodman, 2014/11/17 15:46:07]

It's against Chromium style to have static non-POD types anyway. We spent a lot
of time in Chromium trying to find all of them and kill them because they hurt
startup perf.

[DaveMoore, 2014/11/17 16:36:47]

I see now that you're right that even function statics can't be classes. That
surprised me because they don't invoke static initializers so they have no
impact on startup. Perhaps that's compiler specific.

I changed it to return a new map each time. It's only really going to be called
once at startup anyway.

+  static bool initialized = false;
+  if (!initialized) {
+    for (size_t i = 0; i < arraysize(switch_array); ++i)
+      switch_set.insert(switch_array[i]);
+    initialized = true;
+  }
+  return switch_set;
+}
+
 }  // namespace switches