dlj accepted this revision.
dlj added a comment.
I think this change is structurally fine, but I'll defer to others on whether
it's actually doing the right thing. (I'm pretty sure it is, but I'm far from
an expert on Solaris or Sparc.)
================
Comment at: lib/Driver/ToolChains/Gnu.cpp:2467
const Generic_GCC::GCCVersion &V = GCCInstallation.getVersion();
bool UseInitArrayDefault =
getTriple().getArch() == llvm::Triple::aarch64 ||
----------------
fedor.sergeev wrote:
> davide wrote:
> > this is really a mess.
> Any suggestions on how to improve? :)
From my perspective, this is really no worse than what's already here. I tried
to avoid changing logic in https://reviews.llvm.org/rL297250, but this function
is an example of something that still can use a lot of work.
Since this variable depends on all the parts of the triple, it probably won't
break down very cleanly among the various subclasses. That said, if you do want
to refactor this, you'll of course want to spend some time to think through
exactly how this should work, and spend some time gathering feedback from other
Clang devs... so it's probably out of the scope of this patch. I would be happy
to help review that change... but, more pragmatically, I'd say land this change
first before starting any surgery.
================
Comment at: test/Driver/constructors.c:83
+// RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1 \
+// RUN: -target i386-pc-solaris2.11 \
+// RUN: | FileCheck --check-prefix=CHECK-INIT-ARRAY %s
----------------
Thank you for adding these. :-D Most of the toolchain classes lack explanatory
comments about which platform combinations are expected to work for different
features, so test cases like this will be invaluable to anyone who wants to
refactor parts of the driver.
https://reviews.llvm.org/D35337
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
