wristow 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")
----------------
rsmith wrote:
> 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.
I see. Then I'll change it to "__declspec keyword", to both remove the
Microsoft reference and make it appropriate for the diagnostic formed from it.
================
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>,
----------------
rsmith wrote:
> compnerd wrote:
> > Shouldn't these be DriverOptions, and CC1Options?
> This should be in `f_clang_Group`, not `f_Group`.
I'll upload an updated patch shortly that takes care of this.
================
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.
+
----------------
rsmith wrote:
> 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.
OK, then I'll leave them marked as CC1Option.
================
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.
+
----------------
wristow wrote:
> rsmith wrote:
> > 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.
> OK, then I'll leave them marked as CC1Option.
Regarding the -fno-declspec taking precedence question, I think I've got it
right, but I could have misunderstood something. There is a complication with
this declspec-keyword recognition, in that in addition to it being enabled by
default for PS4, it's implicitly enabled by default with any of
-fms-compatibility, -fms-extensions and -fborland-extensions (and temporarily
also for CUDA). My goal of -fno-declspec is that it should over-ride any
implicit enabling of declspec. The implicit enabling was handled in cc1 itself
(by inspecting values that depend on -fms-compatibility, -fms-extensions,
-fborland-extensions, and CUDA). I left that being handled in cc1, although I
considered collecting that information here in the driver, and passing a more
elaborate expression as the 'Default' arg to hasFlag(), in which case I
wouldn't have needed to explicitly push_back("-fno-declspec"). Anyway, with
implicit enabling of declspec happening in cc1, it was easy to over-ride it by
explicitly passing -fno-declspec through, iff that was the last
-fdeclspec/-fno-declspec arg passed by the user. So that's what I'm doing on
the line with the "// Explicitly disabling __declspec." comment (unless I've
misunderstood, if -fno-declspec isn't the last one, we don't get to that line).
http://reviews.llvm.org/D13322
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
