[PATCH] D13322: Add -f[no-]declspec to control recognition of __declspec as a keyword
Saleem Abdulrasool via cfe-commits
cfe-commits at lists.llvm.org
Wed Sep 30 18:55:22 PDT 2015
compnerd added inline comments.
================
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")
----------------
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"?
================
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>,
----------------
Shouldn't these be DriverOptions, and CC1Options?
================
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.
+
----------------
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.
http://reviews.llvm.org/D13322
More information about the cfe-commits
mailing list