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

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.


================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:5369
         if (ES->hasExternalDefinitions(D) == ExternalASTSource::EK_Never)
-          DebugInfo->completeUnusedClass(cast<CXXRecordDecl>(*D));
+          DebugInfo->completeUnusedClass(*CRD);
     }
----------------
nickdesaulniers wrote:
> The difference between using `DebugInfo` vs `getModuleDebugInfo` in this method is *killing* me.  Same with `return` vs `break`.
Feel free to send separate patches to clean these things up.


================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:5317
 /// EmitTopLevelDecl - Emit code for a single top level declaration.
 void CodeGenModule::EmitTopLevelDecl(Decl *D) {
   // Ignore dependent declarations.
----------------
Since this is only for top-level decls, have you checked this works for types inside namespaces, local types in functions (well, that one might not be supported by GCC - so probably good to test/compare GCC's behavior here too), etc?


================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:5365-5369
+      if (getCodeGenOpts().DebugUnusedTypes && CRD->hasDefinition())
+        DebugInfo->completeUnusedClass(*CRD);
+      else if (auto *ES = D->getASTContext().getExternalSource())
         if (ES->hasExternalDefinitions(D) == ExternalASTSource::EK_Never)
+          DebugInfo->completeUnusedClass(*CRD);
----------------
Perhaps this could be modified to use the same general purpose utility like the other type emissions (as speculated in another comment "EmitUnusedType" or the like)


================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:5565
+    if (CGDebugInfo *DI = getModuleDebugInfo())
+      if (getCodeGenOpts().DebugUnusedTypes) {
+        QualType QT = getContext().getTypedefType(cast<TypedefNameDecl>(D));
----------------
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)


================
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);
----------------
Could this be rolled into the debug-info-kind? (a kind beyond "standalone")


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80242