Thanks for the information. I will make those changes.
On Tue, Jul 15, 2014 at 3:33 PM, Reid Kleckner <[email protected]> wrote: > ================ > Comment at: include/clang/Basic/DiagnosticDriverKinds.td:122 > @@ -121,1 +121,3 @@ > > +def warn_linker_args_take_Wl : Warning<"linker argument -z should be > -Wl,-z">, > + InGroup<InvalidCommandLineArgument>; > ---------------- > Maybe "linker argument %0 should be escaped with -Wl, and commas"? > Specifically, I think many users won't realize that they need to escape > the second space after -z. > > Alternatively, I would be happy if we removed this diagnostic, but I know > Joerg won't. My preferred resolution is, again, that we ask GCC to > document -z as a linker option and then we support it without equivocation. > > ================ > Comment at: include/clang/Driver/Options.td:312 > @@ -311,1 +311,3 @@ > Flags<[DriverOption, CoreOption]>; > +def z_Flag : Separate<["-"], "z">, Flags<[LinkerInput, RenderAsInput]>, > + HelpText<"Pass -z <arg> to the linker">, MetaVarName<"<arg>">; > ---------------- > This can be `def z :`. For -Z we use Z_Flag and Z_Joined because I think > they have different semantics. > > ================ > Comment at: lib/Driver/Tools.cpp:207 > @@ +206,3 @@ > + // Pass -z prefix for gcc linker compatibility. > + if (A.getOption().getName().equals(z_Flag)) { > + D.Diag(diag::warn_linker_args_take_Wl); > ---------------- > Why do this check this way instead of .matches(options::OPT_z)? > > ================ > Comment at: lib/Driver/Tools.cpp:209 > @@ +208,3 @@ > + D.Diag(diag::warn_linker_args_take_Wl); > + CmdArgs.push_back("-z"); > + } > ---------------- > Why not `A.claim(); A.render(Args, CmdArgs)`? Then you can split this > whole clause into it's own 'else if' bucket. > > ================ > Comment at: lib/Driver/Tools.cpp:7588 > @@ +7587,3 @@ > + // Pass -z prefix for gcc linker compatibility. > + if (Input.getInputArg().getOption().getName().equals(z_Flag)) { > + D.Diag(diag::warn_linker_args_take_Wl); > ---------------- > ditto, why not .matches? > > http://reviews.llvm.org/D4393 > > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
