grimar added inline comments.
================
Comment at: lib/Driver/ToolChains/Clang.cpp:2999-3001
+ StringRef Value = A->getOption().matches(options::OPT_fdwarf_fission_EQ)
+ ? A->getValue()
+ : "split";
----------------
dblaikie wrote:
> Rather than creating a string in the case where there's no need for string
> matching/parsing, perhaps that could be avoided?
>
> if (!A->getOption().matches(options::OPT_fdwarf_fission_EQ))
> return DwarfFissionKind::Split;
>
> StringRef Value = A->getValue();
> if (Value == "split")
> ...
>
>
Done.
================
Comment at: lib/Driver/ToolChains/Clang.cpp:3010-3011
+ }
+ if (ArgIndex)
+ *ArgIndex = 0;
+ return DwarfFissionKind::None;
----------------
dblaikie wrote:
> I'd probably either accept this as a precondition (assert(!ArgIndex ||
> ArgIndex == 0)) or do this bit at the start of the function rather than the
> end here, to make it simpler/clearer that all paths assign some value to
> ArgIndex before exit of the function (even though that does mean
> checking/assigning ArgIndex unnecessarily in the case where the argument is
> given, etc)
It turns out that `A->getIndex()` returned can be different from index in
`Args` array.
So it is not correct to use something like `size_t Index = A->getIndex(); ....;
Arg* A = Args.getArgs()[Index]`.
I rewrote this method.
================
Comment at: lib/Driver/ToolChains/Clang.cpp:5889
const llvm::Triple &T = getToolChain().getTriple();
- if (Args.hasArg(options::OPT_gsplit_dwarf) &&
+ if ((getDebugFissionKind(D, Args) == DwarfFissionKind::Split) &&
(T.isOSLinux() || T.isOSFuchsia())) {
----------------
dblaikie wrote:
> Could having more than one call to getDebugFissionKind lead to the error
> message (for -fdwarf-fission=garbage) being printed multiple times? Or is
> this call on a distinct/mutually exclusive codepath to the other?
I think it should not be possible. Currently, there are 2 inclusions of the
`getDebugFissionKind `. One is used for construction clang job,
and one is here, it is used to construct an assembler tool job. I do not see a
way how it can be printed multiple times.
For example, the following invocation will print it only once:
```
clang main.c -o out.s -S -fdwarf-fission=foo
clang-8: error: unsupported argument 'foo' to option 'fdwarf-fission='
```
https://reviews.llvm.org/D52296
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
