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

[Hans Wennborg]

Hmm, the whole thing seems a bit of a mess :/

However, I don't think we need to base DefaultImageName on the full
logic of computeTargetTriple(), which is mostly fine-tuning the triple
based on various flags. I think it's enough to make DefaultImageName
match DefaultTargetTriple, like in the attached patch. What do you
think?

On Thu, Jan 8, 2015 at 2:38 PM, Yaron Keren <yaron.keren at gmail.com> wrote:
> 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 --------------
A non-text attachment was scrubbed...
Name: DefaultImageName.patch
Type: text/x-patch
Size: 3377 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150108/6872b7fe/attachment.bin>