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 <[email protected]> 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 <[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
>>
>>
>
diff --git a/include/clang/Driver/Driver.h b/include/clang/Driver/Driver.h
index 20ca5f0..67d67c3 100644
--- a/include/clang/Driver/Driver.h
+++ b/include/clang/Driver/Driver.h
@@ -104,9 +104,6 @@ public:
/// Default target triple.
std::string DefaultTargetTriple;
- /// Default name for linked images (e.g., "a.out").
- mutable std::string DefaultImageName;
-
/// Driver title to use with help.
std::string DriverTitle;
@@ -365,6 +362,9 @@ public:
const char *LinkingOutput,
InputInfo &Result) const;
+ /// Returns the default name for linked images (e.g., "a.out").
+ const char *getDefaultImageName() const;
+
/// GetNamedOutputPath - Return the name to use for the output of
/// the action \p JA. The result is appended to the compilation's
/// list of temporary or result files, as appropriate.
diff --git a/lib/Driver/Driver.cpp b/lib/Driver/Driver.cpp
index 38765f0..b742fec 100644
--- a/lib/Driver/Driver.cpp
+++ b/lib/Driver/Driver.cpp
@@ -50,7 +50,6 @@ Driver::Driver(StringRef ClangExecutable,
: Opts(createDriverOptTable()), Diags(Diags), Mode(GCCMode),
ClangExecutable(ClangExecutable), SysRoot(DEFAULT_SYSROOT),
UseStdLib(true), DefaultTargetTriple(DefaultTargetTriple),
- DefaultImageName("a.out"),
DriverTitle("clang LLVM compiler"),
CCPrintOptionsFilename(nullptr), CCPrintHeadersFilename(nullptr),
CCLogDiagnosticsFilename(nullptr),
@@ -1411,7 +1410,7 @@ void Driver::BuildJobs(Compilation &C) const {
if (FinalOutput)
LinkingOutput = FinalOutput->getValue();
else
- LinkingOutput = DefaultImageName.c_str();
+ LinkingOutput = getDefaultImageName();
}
InputInfo II;
@@ -1624,6 +1623,11 @@ void Driver::BuildJobsForAction(Compilation &C,
}
}
+const char *Driver::getDefaultImageName() const {
+ llvm::Triple Target(DefaultTargetTriple);
+ return Target.isOSWindows() ? "a.exe" : "a.out";
+}
+
/// \brief Create output filename based on ArgValue, which could either be a
/// full filename, filename without extension, or a directory. If ArgValue
/// does not provide a filename, then use BaseName, and use the extension
@@ -1742,12 +1746,12 @@ const char *Driver::GetNamedOutputPath(Compilation &C,
NamedOutput = MakeCLOutputFilename(C.getArgs(), "", BaseName,
types::TY_Image);
} else if (MultipleArchs && BoundArch) {
- SmallString<128> Output(DefaultImageName.c_str());
+ SmallString<128> Output(getDefaultImageName());
Output += "-";
Output.append(BoundArch);
NamedOutput = C.getArgs().MakeArgString(Output.c_str());
} else
- NamedOutput = DefaultImageName.c_str();
+ NamedOutput = getDefaultImageName();
} else {
const char *Suffix = types::getTypeTempSuffix(JA.getType(), IsCLMode());
assert(Suffix && "All types used for output should have a suffix.");
@@ -1996,8 +2000,6 @@ const ToolChain &Driver::getToolChain(const ArgList &Args,
StringRef DarwinArchName) const {
llvm::Triple Target = computeTargetTriple(DefaultTargetTriple, Args,
DarwinArchName);
- if (Target.isOSWindows())
- DefaultImageName = "a.exe";
ToolChain *&TC = ToolChains[Target.str()];
if (!TC) {
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
