[cfe-commits] r163605 - /cfe/trunk/tools/driver/driver.cpp

[Chandler Carruth]

On Tue, Sep 11, 2012 at 2:58 AM, David Chisnall <csdavec at swan.ac.uk> wrote:

> Author: theraven
> Date: Tue Sep 11 04:58:54 2012
> New Revision: 163605
>
> URL: http://llvm.org/viewvc/llvm-project?rev=163605&view=rev
> Log:
> Select the correct, or, failing that, compatible, dialect when invoked as
> cc,
> c89, c99, and so on.  No change to the default dialect when invoked as
> clang /
> clang++.
>

This seems like a significant new user-visible feature. I would like to
have seen at least a brief discussion on cfe-dev about adding support to
this to make sure there aren't problems. The POSIX spec'ed bits seem pretty
reasonable, however:

+    { "clang", false, false, 0 },
> +    { "clang++", true, false, 0 },
> +    { "clang-c++", true, false, 0 },
> +    { "clang-cc", false, false, "-std=c89" },
> +    { "clang-cpp", false, true, 0 },
> +    { "clang-g++", true, false, "-std=gnu++89" },
> +    { "clang-gcc", false, false, "-std=gnu89" },
>

This doesn't make much sense to me... We've never had a symlink name that
changes from strict standard mode to the GNU dialect, and there has been no
indication of that changing. You don't mention it in your commit log or in
the comments. What's the rationale here?

Again, maybe a better discussion to have on a broader list rather than just
here on the commit list...

+    { "cc", false, false, "-std=c89" },
> +    { "c89", false, false, "-std=c89" },
> +    { "c99", false, false, "-std=c99" },
> +    { "c11", false, false, "-std=c11" },
> +    { "cpp", false, true, 0 },
> +    { "++", true, false, 0 },
>    };
>    std::string ProgName(llvm::sys::path::stem(ArgVector[0]));
>    StringRef ProgNameRef(ProgName);
> @@ -304,10 +315,14 @@
>      for (i = 0; i < sizeof(suffixes) / sizeof(suffixes[0]); ++i) {
>        if (ProgNameRef.endswith(suffixes[i].Suffix)) {
>          FoundMatch = true;
> -        if (suffixes[i].IsCXX)
> +        if (suffixes[i].IsCXX) {
>            TheDriver.CCCIsCXX = true;
> +          fprintf(stderr, "ccc is c++\n");
>

Er, huh? This should be removed. Also, it broke the build, so please fix or
back out promptly.


> +        }
>          if (suffixes[i].IsCPP)
>            TheDriver.CCCIsCPP = true;
> +        if (suffixes[i].DefaultDialect)
> +          ArgVector.insert(ArgVector.begin()+1,
> suffixes[i].DefaultDialect);
>

I worry a lot about the design here. Passing in arbitrary strings seems
very fragile. Why not use the driver itself to set an option for this? That
would seem like a much cleaner design.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120911/3388d084/attachment.html>