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 <[email protected]>: > 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 <[email protected]>: > >> On Sun, Jan 4, 2015 at 5:48 AM, Yaron Keren <[email protected]> >> 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 >> > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
