dblaikie added inline comments.
================ Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3250 // -gline-directives-only supported only for the DWARF debug info. if (DWARFVersion == 0 && DebugInfoKind == codegenoptions::DebugDirectivesOnly) DebugInfoKind = codegenoptions::NoDebugInfo; ---------------- probinson wrote: > After setting DWARFVersion above, when nothing was requested, this looks > peculiar. Is `DWARFVersion == 0` a proxy for `-gcodeview` ? Yep, DWARFVersion shouldn't be set when none was requested - see discussion above between myself & @cmtice - that should leave this code fine/correct. ================ Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6174 + DwarfVersion = ParseDebugDefaultVersion(getToolChain(), Args); + if (DwarfVersion == 0) ---------------- probinson wrote: > I have to say, this sequence is a whole lot easier to follow than what's up > in RenderDebugOptions. It would be nice if they were both so easy to > understand. > > Although it makes me wonder, does `-fdebug-default-version` go unclaimed here > if there's a `-gdwarf-2` on the command line? Once the "else if (DefaultDWARFVersion != 0)" clause is removed, and the ParseDebugDefaultVersion call is sunk down into the "if (EmitDwarf)" case - hopefully that'll simplify the RenderDebugOptions code enough to be reasonable? I think the complication there is that "-g" can imply CodeView if nothing explicitly requests DWARF & the target is windows, whereas there's no support for that kind of CV fallback here? So some of that might be inherent. & agreed - worth testing that -fdebug-default-version doesn't get a -Wunused warning if someone says "-fdebug-default-version 4 -gdwarf-2". @cmtice is that tested? You might be able to add -Werror to the existing test case's RUN lines to demonstrate that no such warnings are produced? ================ Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:1151 + || Value > 5 + || Value < 1) + TC.getDriver().Diag(diag::err_drv_invalid_int_value) ---------------- probinson wrote: > Clang doesn't support DWARF v1. I expect nobody does at this point (DWARF 2 > came out in 1993). You'd recommend/request moving this bound up to 2 (to be consistent with the fact that clang supports -gdwarf-2 technically (even though we probably don't produce fully conformant DWARFv2 these days, I would guess))? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69822/new/ https://reviews.llvm.org/D69822 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits