r225134 - Fix default image name to 'a.exe' on Windows, instead 'a.out'.

[Yaron Keren]

Rethinking, my suggestion is not good. getToolChain is called several times
so returning the target triple and setting DefaultImageName baed on it
duplicates the same operation several times.

Moving DefaultImageName to llvm::Triple or adding the helper function will
not help, as the the root of the problem is that the target triple is not
known when it's needed: It is computed in the const getToolChain but not
stored.
Storing in Driver::TargetTriple would make getToolChain non-const or or
Driver::TargetTriple mutuable just the same as DefaultImageName, which
isn't cleaner.

Also, look at the FIXME at line 339 which seems to workaround the same
const issue. Maybe we can solve both.

What do you think?



2015-01-08 23:59 GMT+02:00 Yaron Keren <yaron.keren at gmail.com>:

> The Driver needs DefaultImageName which is based on the target triple but
> the only location the target triple is computed using  computeTargetTriple() is
> in getToolChain() and the target triple is not kept otherwise.
>
> We could have getToolChain return the computed target triple to its
> caller BuildCompilation (non const mmeber function) and have
> BuildCompilation set DefaultImageName based on the OS. Does this make
> sense to you?
>
> I'll fix the test.
>
>
>
>
>
>
> 2015-01-08 23:16 GMT+02:00 Hans Wennborg <hans at chromium.org>:
>
>> On Sun, Jan 4, 2015 at 5:48 AM, Yaron Keren <yaron.keren at gmail.com>
>> wrote:
>> > Author: yrnkrn
>> > Date: Sun Jan  4 07:48:30 2015
>> > New Revision: 225134
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=225134&view=rev
>> > Log:
>> > Fix default image name to 'a.exe' on Windows, instead 'a.out'.
>> > This applies to mingw as clang-cl already has its own logic for the
>> filename.
>>
>> [..]
>>
>> > Modified: cfe/trunk/lib/Driver/Driver.cpp
>> > URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=225134&r1=225133&r2=225134&view=diff
>> >
>> ==============================================================================
>> > --- cfe/trunk/lib/Driver/Driver.cpp (original)
>> > +++ cfe/trunk/lib/Driver/Driver.cpp Sun Jan  4 07:48:30 2015
>> > @@ -1996,6 +1996,8 @@ const ToolChain &Driver::getToolChain(co
>> >                                        StringRef DarwinArchName) const {
>> >    llvm::Triple Target = computeTargetTriple(DefaultTargetTriple, Args,
>> >                                              DarwinArchName);
>> > +  if (Target.isOSWindows())
>> > +    DefaultImageName = "a.exe";
>>
>> This doesn't seem right. Why should getToolChain() be changing the
>> Driver's DefaultImageName? That this requires DefaultImageName to be
>> mutable makes it feel even more iffy.
>>
>> Maybe DefaultImageName should be a property of the Target? Or perhaps
>> there should be a "const char *Driver::DefaultImageName(llvm::Triple)"
>> method?
>>
>>
>> > Modified: cfe/trunk/test/Driver/lto.c
>>
>> It would be nice to test for the default image name explicitly by
>> setting -target and looking at the cc1 invocation.
>>
>>  - Hans
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150109/6a0bbb07/attachment.html>