[cfe-commits] [patch] Remove most of the --with-cxx-* logic

[Chandler Carruth]

On Tue, Jan 31, 2012 at 9:34 AM, Rafael Espíndola <
rafael.espindola at gmail.com> wrote:

> I noticed a small regression when using --with-cxx-include-dir (/32
> was not being added to the lib search dir).


Ouch!


> The good news is that the
> new logic for detecting gcc installations progressed so much that we
> should be able to remove most of the special cases.
>

And yay! ;]


> The attached patches replace the existing options with a single
> --with-cxx-install-dir and the only thing the new option does is point
> the existing detection logic to the directory it is given.
>
> Are the patches OK?


 I have a few issues with the patches as-is, and a higher-level request
which may make sense as part of this patch, or as a follow up patch.

My issue with the current patch is not at all to do with the
*functionality* but with the naming and documentation of the functionality.
Especially as this is getting away from a pile of FIXMEs and toward a
really sustainable pattern.

'with-cxx-install-root' is the flag, and the docs then talk about
'libstdc++'. This made me want to rename the flag
'with-libstdcxx-install-root' at first, but then I looked at how you used
it. You're just providing a root to look for the GCC installation in, not a
root for libstdc++. This root will impact what linker is invoke, what
assembler (if one is needed), etc. That makes me think it should be
'with-gcc-install-root'.

However, what the user should place in this is also a bit confusing,
because it's not really the root of the GCC installation, its the "prefix"
(in configure terminology) of the GCC installation, such as "/usr",
"/usr/local", etc. I'm not sure what the best way to clarify this is...
'with-gcc-install-prefix' is close, and maybe with some examples in the
docs for the configure flag?

Finally, can you update CMake as well? If not, I can, I'm just being a bit
lazy. ;]


Now for the high-level issue: I really like this logic, but I note that no
tests get updated, and no tests broke when I refactored things. I think we
need to shift the way this is implemented to something we can test going
forward. In the process I think we can also make the entire thing more
flexible: let's make it a run-time flag, not (just) a configure-time flag.
Then the configure-time flag can setup a default for the runtime flag, and
we can test the actual behavior of the runtime flag in the regression test
suite. Thoughts?

-Chandler
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120131/3e81d07f/attachment.html>