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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to