On Thu, Oct 11, 2012 at 7:23 PM, Rafael Espíndola <[email protected]> wrote: > On 11 October 2012 10:43, Simon Atanasyan <[email protected]> wrote: >> Please review the patch. > > + StringRef MultiarchSuffix = ::getMultiarchSuffix(TargetArch, Args); > > Why the ::?
There is GCCInstallationDetector.getMultiarchSuffix() and I tried to
avoid an ambiguity. In the new attached patch I just rename static
getMultiarchSuffix to getTargetMultiarchSuffix.
> +// FIXME: There is the same routine in the Tools.cpp.
> +static bool hasMipsN32ABIArg(const ArgList &Args) {
> + Arg *A = Args.getLastArg(options::OPT_mabi_EQ);
> + return A && (A->getValue(Args) == StringRef("n32"));
> +}
>
> Would making it a method of Generic_GCC fix this?
I'm not sure it is better than keeping a tiny code duplication. It
definitely removes the code duplication. But it will be the first and
the only so specialized function in Generic_GCC. It is not clear why
we do not factor out other checking expressions like
"Args.hasArg(options::OPT_static)" to the separate functions in the
Generic_GCC class.
> Why have you moved getMultilibDir? It makes the patch harder to read.
I want to group all Mips related helper function in one place. But you
are right - that pollutes the patch and should be done in separate
commit.
Please review new patch.
--
Simon
mips-n32.patch
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
