tra added inline comments.
================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3957
+ RenderDebugEnablingArgs(Args, CmdArgs, DebugInfoKind,
+ std::min(DWARFVersion, TC.getMaxDwarfVersion()),
DebuggerTuning);
----------------
dblaikie wrote:
> I think, ideally, the DWARFVersion calculation would happen up `if
> (EmitDwarf) {` block where all default and explicit dwarf version
> calculations are done.
>
> I guess it's not done that way because of the gembed-source error path?
That's part of it. `-gembed-source` does need to know both the specified DWARF
version and the clamped one. I could move the calculation of the effective
DWARF version back to where it was in the first version of the patch, and we'll
need both the specified and the clamped values.
That would bring back the need to explain which of the two DWARF versions to
use, where, and why. In the current revision the impact on DWARF version
clamping is localized to where it makes a difference. Neither is perfect, but
the current version is a bit easier to explain, I think.
As for moving it under `if( EmitDwarf)`, the problem is that the DWARF version
also affects cases when `EmitDwarf` is false, so the clamping needs to be done
regardless. E.g. the `-gmlt` option that was used in our build break that
uncovered the issue. We do not emit DWARF per se, but the `DefaultDWARFVersion`
does affect the generation of the lineinfo debugging and we do need to clamp it.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D92617/new/
https://reviews.llvm.org/D92617
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
