[PATCH] D80242: [Clang] implement -fno-eliminate-unused-debug-types

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers marked an inline comment as done.
nickdesaulniers added inline comments.


================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:5565
+    if (CGDebugInfo *DI = getModuleDebugInfo())
+      if (getCodeGenOpts().DebugUnusedTypes) {
+        QualType QT = getContext().getTypedefType(cast<TypedefNameDecl>(D));
----------------
dblaikie wrote:
> Rather than testing DebugUnusedType for every call - might be best to add a "EmitUnusedType" function that tests the flag and does the emission? (same way EmitExplicitCastType already checks for a certain level of debug info before emitting)
It can be; what I don't like about that approach is that we have to determine the `QualType` here to pass in, at which point such function might just return without doing anything. (ie. we have to do slightly more work in the case where the debugging of unused types was *not* requested).  But that's a minor nit and we can live with it?


================
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:
> 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?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80242/new/

https://reviews.llvm.org/D80242