dblaikie added inline comments.
================ Comment at: clang/lib/CodeGen/CodeGenModule.cpp:5559 + case Decl::Typedef: + if (getCodeGenOpts().DebugUnusedTypes) + if (CGDebugInfo *DI = getModuleDebugInfo()) ---------------- nickdesaulniers wrote: > dblaikie wrote: > > Probably test this within the implementation of CGDebugInfo? & rename the > > EmitTypedef function to something that clarifies that it's for an otherwise > > unused type? > > > > But that function might need to be generalized further, rather than only > > having it for typedefs. (see general comment above) > I think it makes sense at this point to rename `EmitExplicitCastType` to > `RetainType` or `EmitType`. WDYT? Yep - now that I understand the confusingly named hasReducedDebugInfo "RetainType" sounds good to me, maybe "EmitAndRetainType" might be worthwhile for the extra clarity/symmetry with other "Emit" functions. ================ Comment at: clang/lib/Frontend/CompilerInvocation.cpp:768 Opts.DebugFwdTemplateParams = Args.hasArg(OPT_debug_forward_template_params); + Opts.DebugUnusedTypes = Args.hasArg(OPT_eliminate_unused_debug_types_fno); Opts.EmbedSource = Args.hasArg(OPT_gembed_source); ---------------- nickdesaulniers wrote: > nickdesaulniers wrote: > > dblaikie wrote: > > > Could this be rolled into the debug-info-kind? (a kind beyond > > > "standalone") > > That sounds like a good idea. Looking at the definition of `DebugInfoKind` > > (llvm/llvm-project/clang/include/clang/Basic/DebugInfoOptions.h), it seems > > that `DebugInfoKind` is an `enum` that defines a "level" of debug info to > > emit? Looking at the guard in `CGDebugInfo::EmitExplicitCastType` > > (llvm/llvm-project/clang/lib/CodeGen/CGDebugInfo.cpp), it calls > > `CodeGenOptions::hasReducedDebugInfo()` which does a comparison against a > > certain level. That seems to indicate the order of the enumerations is > > important. Do you have a suggestion what order I should add the new enum? > > > > I'm guessing after `LimitedDebugInfo` or `DebugInfoConstructor`, but before > > `FullDebugInfo`? (I find the name of the method `hasReducedDebugInfo` > > confusing in that regard). > Ok, so I have a diff that implements this approach. I feel like I should > maybe publish it as a child commit to this one, to be able to more easily > discuss it? > > Two problems I run into: > 1. as alluded to in my previous response in this thread, `DebugInfoKind` is > an enum that specifies a level. It tends to "drag" other debug flags in > based on the ordering. Looking at extending the `switch` in > `CGDebugInfo::CreateCompileUnit` (clang/lib/CodeGen/CGDebugInfo.cpp), it's > not at all clear to me which we existing case should we choose? > 2. we want the flag `-fno-eliminate-unused-debug-types` to match GCC for > compatibility. We can additionally add a new debug info kind like > `"standalone"` (clang/lib/Frontend/CompilerInvocation.cpp), but it's not > clear how the two flags together should interact. > > The suggestion for a new debug info kind feels like a recommendation to add a > new "level" of debug info, but `-fno-eliminate-unused-debug-types` feels like > it should be mutually exclusive of debug info kind? (I guess GCC does > *require* `-g` with `-fno-eliminate-unused-debug-types`) > > @dblaikie maybe you have more recommendations on this? This value would probably go "after" "full" (full isn't full enough, as you've found - you need a mode that's even "fullerer") Perhaps renaming "Full" to "Referenced" and then introducing this new kind as the new "Full" or maybe under a somewhat different name to avoid ambiguity. Any suggestions on a less confusing name for "hasReducedDebugInfo"? (I think it was basically "Has less than full debug info"... but, yep, it doesn't do that at all - it's "HasTypeAndVariableDebugInfo" by the looks of it/by the comment) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80242/new/ https://reviews.llvm.org/D80242 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
