[PATCH] D92617: [DWARF] Allow toolchain to adjust specified DWARF version.

[Artem Belevich via Phabricator via cfe-commits]

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