[PATCH] D13322: Add -f[no-]declspec to control recognition of __declspec as a keyword

[Richard Smith via cfe-commits]

rsmith added a subscriber: rsmith.

================
Comment at: include/clang/Basic/LangOptions.def:93
@@ -92,2 +92,3 @@
 LANGOPT(WChar             , 1, CPlusPlus, "wchar_t keyword")
+LANGOPT(DeclSpecKeyword   , 1, 0, "Microsoft __declspec keyword support")
 BENIGN_LANGOPT(DollarIdents   , 1, 1, "'$' in identifiers")
----------------
compnerd wrote:
> Im not sure I care for Microsoft here.  This is an extension that is supported on more than one compiler suite.  How about "Enable support for __declspec extension"?
These strings are used in a diagnostic of the form "support for %0 is not enabled", so `"__declspec"` or `"__declspec keyword"` would be appropriate.

================
Comment at: include/clang/Driver/Options.td:520
@@ -519,1 +519,3 @@
+def fdeclspec : Flag<["-"], "fdeclspec">, Group<f_Group>,
+  HelpText<"Allow __declspec as a keyword">, Flags<[CC1Option]>;
 def fdollars_in_identifiers : Flag<["-"], "fdollars-in-identifiers">, Group<f_Group>,
----------------
compnerd wrote:
> Shouldn't these be DriverOptions, and CC1Options?
This should be in `f_clang_Group`, not `f_Group`.

================
Comment at: lib/Driver/Tools.cpp:4663
@@ +4662,3 @@
+  else if (Args.hasArg(options::OPT_fno_declspec))
+    CmdArgs.push_back("-fno-declspec"); // Explicitly disabling __declspec.
+
----------------
compnerd wrote:
> The Args.hasFlag will correctly give you the value (-fdeclspec -fno-declspec will be treated as -fno-declspec).  In fact, doesn't your implementation make -fno-declspec take precedence?
> 
> Plus, you marked these as cc1options above, not driver options.  These aren't technically available here.
The options are in the driver's Options.td and marked as CC1Option, so they're available in both the driver and cc1.


http://reviews.llvm.org/D13322