<div dir="rtl"><div dir="ltr">Rethinking, my suggestion is not good. getToolChain is called several times so returning the target triple and setting <span style="font-size:12.7272720336914px">DefaultImageName</span><span style="font-size:12.7272720336914px"> baed on it duplicates the same operation several times.</span></div><div dir="ltr"><span style="font-size:12.7272720336914px"><br></span></div><div dir="ltr"><span style="font-size:12.7272720336914px">Moving </span><span style="font-size:12.7272720336914px">DefaultImageName</span><span style="font-size:12.7272720336914px"> to llvm::Triple or adding the helper function will not help, as the t</span><span style="font-size:12.7272720336914px">he root of the problem is that the target triple is not known when it's needed: </span><span style="font-size:12.7272720336914px">It is computed in </span><span style="font-size:12.7272720336914px">the const </span><span style="font-size:12.7272720336914px">getToolChain but not stored. </span></div><div dir="ltr"><span style="font-size:12.7272720336914px">Storing in Driver::TargetTriple </span><span style="font-size:12.7272720336914px">would make </span><span style="font-size:12.7272720336914px">getToolChain</span><span style="font-size:12.7272720336914px"> non-const or or </span><span style="font-size:12.7272720336914px">Driver::TargetTriple </span><span style="font-size:12.7272720336914px">mutuable just the same as </span><span style="font-size:12.7272720336914px">DefaultImageName, which isn't cleaner.</span></div><div dir="ltr"><span style="font-size:12.7272720336914px"><br></span></div><div dir="ltr"><span style="font-size:12.7272720336914px">Also, look at the FIXME at line 339 which seems to workaround the same const issue. Maybe we can solve both.</span></div><div dir="ltr"><span style="font-size:12.7272720336914px"><br></span></div><div dir="ltr"><span style="font-size:12.7272720336914px">What do you think?</span></div><div dir="ltr"><span style="font-size:12.7272720336914px"><br></span></div><div dir="ltr"><br></div></div><div class="gmail_extra"><div dir="ltr"><br><div class="gmail_quote">2015-01-08 23:59 GMT+02:00 Yaron Keren <span dir="ltr"><<a href="mailto:yaron.keren@gmail.com" target="_blank">yaron.keren@gmail.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 .8ex;border-left:1px #ccc solid;border-right:1px #ccc solid;padding-left:1ex;padding-right:1ex"><div dir="rtl"><div dir="ltr"><div><span style="font-size:12.7272720336914px">The Driver needs </span><span style="font-size:12.7272720336914px">DefaultImageName</span><span style="font-size:12.7272720336914px"> which is based on the target triple but the only location the target triple is computed using </span><span style="font-size:12.7272720336914px"> </span>computeTargetTriple() is in <span style="font-size:12.7272720336914px">getToolChain() and the target triple is not kept otherwise. </span></div><div><span style="font-size:12.7272720336914px"><br></span></div><div><span style="font-size:12.7272720336914px">We could have </span>getToolChain return the computed target triple to its caller BuildCompilation (non const mmeber function) and have BuildCompilation set <span style="font-size:12.7272720336914px">DefaultImageName based on the OS. </span><span style="font-size:12.7272720336914px">Does this make sense to you?</span></div><div><span style="font-size:12.7272720336914px"><br></span></div><div>I'll fix the test.</div></div><div dir="ltr"><br></div><div dir="ltr"><span style="font-size:12.7272720336914px"><br></span></div><div dir="ltr"><br></div><div dir="ltr"><br></div><div dir="ltr"><br></div></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote"><div dir="ltr">2015-01-08 23:16 GMT+02:00 Hans Wennborg <span dir="ltr"><<a href="mailto:hans@chromium.org" target="_blank">hans@chromium.org</a>></span>:</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>On Sun, Jan 4, 2015 at 5:48 AM, Yaron Keren <<a href="mailto:yaron.keren@gmail.com" target="_blank">yaron.keren@gmail.com</a>> wrote:<br>
> Author: yrnkrn<br>
> Date: Sun Jan 4 07:48:30 2015<br>
> New Revision: 225134<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=225134&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=225134&view=rev</a><br>
> Log:<br>
> Fix default image name to 'a.exe' on Windows, instead 'a.out'.<br>
> This applies to mingw as clang-cl already has its own logic for the filename.<br>
<br>
</span>[..]<br>
<span><br>
> Modified: cfe/trunk/lib/Driver/Driver.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=225134&r1=225133&r2=225134&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=225134&r1=225133&r2=225134&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/Driver/Driver.cpp (original)<br>
> +++ cfe/trunk/lib/Driver/Driver.cpp Sun Jan 4 07:48:30 2015<br>
> @@ -1996,6 +1996,8 @@ const ToolChain &Driver::getToolChain(co<br>
> StringRef DarwinArchName) const {<br>
> llvm::Triple Target = computeTargetTriple(DefaultTargetTriple, Args,<br>
> DarwinArchName);<br>
> + if (Target.isOSWindows())<br>
> + DefaultImageName = "a.exe";<br>
<br>
</span>This doesn't seem right. Why should getToolChain() be changing the<br>
Driver's DefaultImageName? That this requires DefaultImageName to be<br>
mutable makes it feel even more iffy.<br>
<br>
Maybe DefaultImageName should be a property of the Target? Or perhaps<br>
there should be a "const char *Driver::DefaultImageName(llvm::Triple)"<br>
method?<br>
<br>
<br>
> Modified: cfe/trunk/test/Driver/lto.c<br>
<br>
It would be nice to test for the default image name explicitly by<br>
setting -target and looking at the cc1 invocation.<br>
<span><font color="#888888"><br>
- Hans<br>
</font></span></blockquote></div><br></div>
</div></div></blockquote></div></div></div>