[PATCH] D52296: [Clang] - Add '-gsplit-dwarf[=split, =single]' version for '-gsplit-dwarf' option.

[George Rimar via Phabricator via cfe-commits]

grimar added inline comments.


================
Comment at: lib/Driver/ToolChains/CommonArgs.cpp:813-830
+  if (Arg *A = Args.getLastArg(options::OPT_gsplit_dwarf_EQ))
+    if (StringRef(A->getValue()) == "single")
+      return Args.MakeArgString(Output.getFilename());
+
   Arg *FinalOutput = Args.getLastArg(options::OPT_o);
   if (FinalOutput && Args.hasArg(options::OPT_c)) {
     SmallString<128> T(FinalOutput->getValue());
----------------
grimar wrote:
> dblaikie wrote:
> > If this function now takes the output file name - could it be simplified to just always use that, rather than the conditional code that follows and tests whether -o is specified and builds up something like the output file name & uses the dwo suffix?
> I am investigating this.
So what I see now is that when the function becomes:

```
const char *tools::SplitDebugName(const ArgList &Args, const InputInfo &Input,
                                  const InputInfo &Output) {
  SmallString<128> T(Output.getFilename());
  llvm::sys::path::replace_extension(T, "dwo");
  return Args.MakeArgString(T);
}
```

Then no clang tests (check-clang) fail. This does not seem normal to me.
I would expect that such a change should break some `-fdebug-compilation-dir` relative 
logic at least and I suspect we just have no test(s) atm.

What about if I add test case(s) and post a follow-up refactor patch for this place?
(I started work on it, but it will take some time).


https://reviews.llvm.org/D52296