DebugInfo: Emit any enum with a referenced enum constant.

[dblaikie]

Summary

This is incomplete/too verbose. Even if the enum constant is used in a
header (to declare the size of an array member of a type that's never
used, or in an inline function never called).

This could be fixed by keeping a chain of references "this enum is
referenced from this type which is referenced from this type, used in
this inline function, etc..." which is a substantial undertaking, though
it would help power several other debug info correctness/optimizations.
(eg: struct foo { ... }; .... void *f; ((foo*)f)->x currently doesn't
emit the definition of 'foo', but we could keep track of the fact that
foo was referenced from that expression, etc)

[dblaikie]

Just wrapping my head around Arcanist - bumping this to the mailing list.

(apologies to 'reviewers' for popping this into your inbox instead of your mailing list filters... )

This is just a throwaway patch - something I've written up, but I think it'll be too much work to implement well now.

(oh, and imported entities have the same problem as enums... it'd be nice to have their use tracked precisely too so we could reduce some debug info size)

I haven't done the size analysis to see just how bad this is, perhaps it's acceptable as-is, even without the use tracking, but I rather doubt it.

[rsmith]

Implementation LGTM, if you decide that you want to go in this direction.

[dblaikie]

In D3635#44720, @rsmith wrote:

Implementation LGTM, if you decide that you want to go in this direction.

Just looking over some old patches/git branches I had lying around.

It seems what I might need to make this closer to GCC's size/fidelity tradeoff, is the ability to detect if the use of the enumerator was an ODR use. So that this:

enum X { Y };
int i = Y;

was a use, but this:

int i[Y];

was not.

Is that reasnable? Can I practically detect that property in MarkAnyDeclReferenced?

(is "int i = Y;" an ODR use? Perhaps I'm getting the terminology wrong... maybe there is no nice linguistic property that distinguishes these two cases?)

& just for the record, the patch as it stands grows the .dwo size for the dwos build for the clang binary by a little over 18% :/

& using the MightBeOdrUse flag to MarkAnyDeclReferenced didn't seem to change the array case behavior above - I didn't get aggregate numbers to see if it had any impact.

include/clang/AST/ASTConsumer.h

namespace clang {
   class ASTContext;
   class CXXRecordDecl;
   class Decl;
   class DeclGroupRef;
   class ASTMutationListener;
   class ASTDeserializationListener; // layering violation because void* is ugly
   class SemaConsumer; // layering violation required for safe SemaConsumer
   class TagDecl;
+  class EnumConstantDecl;
   class VarDecl;
   class FunctionDecl;
   class ImportDecl;
 
 /// ASTConsumer - This is an abstract interface that should be implemented by
 /// clients that read ASTs.  This abstraction layer allows the client to be
 /// independent of the AST producer (e.g. parser vs AST dump file reader, etc).
 class ASTConsumer {
public:
   /// hack on the type, which can occur at any point in the file (because these
   /// can be defined in declspecs).
   virtual void HandleTagDeclDefinition(TagDecl *D) {}
 
   /// \brief This callback is invoked the first time each TagDecl is required to
   /// be complete.
   virtual void HandleTagDeclRequiredDefinition(const TagDecl *D) {}
 
+  virtual void HandleEnumDeclReferenced(const EnumConstantDecl *ED) {}
+
   /// \brief Invoked when a function is implicitly instantiated.
   /// Note that at this point point it does not have a body, its body is
   /// instantiated at the end of the translation unit and passed to
   /// HandleTopLevelDecl.
   virtual void HandleCXXImplicitFunctionInstantiation(FunctionDecl *D) {}
 
   /// \brief Handle the specified top-level declaration that occurred inside
   /// and ObjC container.

lib/CodeGen/CGDebugInfo.h

public:
   /// getOrCreateInterfaceType - Emit an objective c interface type standalone
   /// debug info.
   llvm::DIType getOrCreateInterfaceType(QualType Ty,
                                         SourceLocation Loc);
 
   void completeType(const EnumDecl *ED);
   void completeType(const RecordDecl *RD);
   void completeRequiredType(const RecordDecl *RD);
+  void completeRequiredType(const EnumConstantDecl *ED);
   void completeClassData(const RecordDecl *RD);
 
   void completeTemplateDefinition(const ClassTemplateSpecializationDecl &SD);
 
 private:
   /// EmitDeclare - Emit call to llvm.dbg.declare for a variable declaration.
   void EmitDeclare(const VarDecl *decl, unsigned Tag, llvm::Value *AI,
                    unsigned ArgNo, CGBuilderTy &Builder);

lib/CodeGen/CGDebugInfo.cpp

T = CGM.getContext().getConstantArrayType(ET, ConstVal,
                                            ArrayType::Normal, 0);
   }
 
   DBuilder.createGlobalVariable(Name, Unit, LineNo,
                                 getOrCreateType(T, Unit),
                                 Var->hasInternalLinkage(), Var);
 }
 
+void CGDebugInfo::completeRequiredType(const EnumConstantDecl *ECD) {
+  const EnumDecl *ED = cast<EnumDecl>(ECD->getDeclContext());
+  const EnumType *Ty = cast<EnumType>(ED->getTypeForDecl());
+  if (Ty->isDependentType())
+    return;
+  if (!ED->getDefinition())
+    return;
+  llvm::DIFile Unit = getOrCreateFile(ED->getLocation());
+  getOrCreateType(QualType(Ty, 0), Unit);
+}
+
 /// EmitGlobalVariable - Emit global variable's debug info.
 void CGDebugInfo::EmitGlobalVariable(const ValueDecl *VD,
                                      llvm::Constant *Init) {
   assert(DebugKind >= CodeGenOptions::LimitedDebugInfo);
+  assert(!isa<EnumConstantDecl>(VD));
   // Create the descriptor for the variable.
   llvm::DIFile Unit = getOrCreateFile(VD->getLocation());
   StringRef Name = VD->getName();
   llvm::DIType Ty = getOrCreateType(VD->getType(), Unit);
-  if (const EnumConstantDecl *ECD = dyn_cast<EnumConstantDecl>(VD)) {
-    const EnumDecl *ED = cast<EnumDecl>(ECD->getDeclContext());
-    assert(isa<EnumType>(ED->getTypeForDecl()) && "Enum without EnumType?");
-    Ty = getOrCreateType(QualType(ED->getTypeForDecl(), 0), Unit);
-  }
-  // Do not use DIGlobalVariable for enums.
-  if (Ty.getTag() == llvm::dwarf::DW_TAG_enumeration_type)
-    return;
+  assert(Ty.getTag() != llvm::dwarf::DW_TAG_enumeration_type);
   // Do not emit separate definitions for function local const/statics.
   if (isa<FunctionDecl>(VD->getDeclContext()))
     return;
   VD = cast<ValueDecl>(VD->getCanonicalDecl());
   auto pair = DeclCache.insert(std::make_pair(VD, llvm::WeakVH()));
   if (!pair.second)
     return;
   llvm::DIDescriptor DContext =

lib/CodeGen/CGExpr.cpp

CodeGenFunction::tryEmitAsConstant(DeclRefExpr *refExpr) {
   if (result.HasSideEffects)
     return ConstantEmission();
 
   // Emit as a constant.
   llvm::Constant *C = CGM.EmitConstantValue(result.Val, resultType, this);
 
   // Make sure we emit a debug reference to the global variable.
   // This should probably fire even for
-  if (isa<VarDecl>(value)) {
-    if (!getContext().DeclMustBeEmitted(cast<VarDecl>(value)))
-      EmitDeclRefExprDbgValue(refExpr, C);
-  } else {
-    assert(isa<EnumConstantDecl>(value));
-    EmitDeclRefExprDbgValue(refExpr, C);
-  }
-
+  if (isa<VarDecl>(value) && !getContext().DeclMustBeEmitted(cast<VarDecl>(value)))
   // If we emitted a reference constant, we need to dereference that.
   if (resultIsReference)
     return ConstantEmission::forReference(C);
 
   return ConstantEmission::forValue(C);
 }
 
 llvm::Value *CodeGenFunction::EmitLoadOfScalar(LValue lvalue,

lib/CodeGen/CodeGenAction.cpp

public:
 
     void HandleTagDeclDefinition(TagDecl *D) override {
       PrettyStackTraceDecl CrashInfo(D, SourceLocation(),
                                      Context->getSourceManager(),
                                      "LLVM IR generation of declaration");
       Gen->HandleTagDeclDefinition(D);
     }
 
+    void HandleEnumDeclReferenced(const EnumConstantDecl *ED) override {
+      Gen->HandleEnumDeclReferenced(ED);
+    }
+
     void HandleTagDeclRequiredDefinition(const TagDecl *D) override {
       Gen->HandleTagDeclRequiredDefinition(D);
     }
 
     void CompleteTentativeDefinition(VarDecl *D) override {
       Gen->CompleteTentativeDefinition(D);
     }
 

lib/CodeGen/ModuleBuilder.cpp

void HandleTagDeclDefinition(TagDecl *D) override {
           if (auto *Method = dyn_cast<CXXMethodDecl>(M))
             if (Method->doesThisDeclarationHaveABody() &&
                 (Method->hasAttr<UsedAttr>() || 
                  Method->hasAttr<ConstructorAttr>()))
               Builder->EmitTopLevelDecl(Method);
       }
     }
 
+    void HandleEnumDeclReferenced(const EnumConstantDecl *ED) override {
+      if (Diags.hasErrorOccurred())
+        return;
+
+      if (CodeGen::CGDebugInfo *DI = Builder->getModuleDebugInfo())
+        DI->completeRequiredType(ED);
+    }
+
     void HandleTagDeclRequiredDefinition(const TagDecl *D) override {
       if (Diags.hasErrorOccurred())
         return;
 
       if (CodeGen::CGDebugInfo *DI = Builder->getModuleDebugInfo())
         if (const RecordDecl *RD = dyn_cast<RecordDecl>(D))
           DI->completeRequiredType(RD);
     }

lib/Sema/SemaExpr.cpp

if (VarDecl *VD = dyn_cast<VarDecl>(D)) {
       MarkVariableReferenced(Loc, VD);
       return;
     }
     if (FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
       MarkFunctionReferenced(Loc, FD);
       return;
     }
   }
+
+  if (const EnumConstantDecl *ED = dyn_cast<EnumConstantDecl>(D))
+    Consumer.HandleEnumDeclReferenced(ED);
+
   D->setReferenced();
 }
 
 namespace {
   // Mark all of the declarations referenced
   // FIXME: Not fully implemented yet! We need to have a better understanding
   // of when we're entering
   class MarkReferencedDecls : public RecursiveASTVisitor<MarkReferencedDecls> {

test/CodeGenCXX/debug-info-enum.cpp

 // RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm -g %s -o - | FileCheck %s
 
 // CHECK: [[ENUMS:![0-9]*]], {{[^,]*}}, {{[^,]*}}, {{[^,]*}}, {{[^,]*}}, {{[^,]*}}} ; [ DW_TAG_compile_unit ]
-// CHECK: [[ENUMS]] = metadata !{metadata [[E1:![0-9]*]], metadata [[E2:![0-9]*]], metadata [[E3:![0-9]*]]}
+// CHECK: [[ENUMS]] = metadata !{metadata [[E1:![0-9]*]], metadata [[E2:![0-9]*]], metadata [[E3:![0-9]*]], metadata [[E5:![0-9]*]]}
 
 namespace test1 {
 // CHECK: [[E1]] = metadata !{i32 {{[^,]*}}, {{[^,]*}}, metadata [[TEST1:![0-9]*]], {{.*}}, metadata [[TEST1_ENUMS:![0-9]*]], {{[^,]*}}, null, null, metadata !"_ZTSN5test11eE"} ; [ DW_TAG_enumeration_type ] [e]
 // CHECK: [[TEST1]] = {{.*}} ; [ DW_TAG_namespace ] [test1]
 // CHECK: [[TEST1_ENUMS]] = metadata !{metadata [[TEST1_E:![0-9]*]]}
 // CHECK: [[TEST1_E]] = {{.*}}, metadata !"E", i64 0} ; [ DW_TAG_enumerator ] [E :: 0]
 enum e { E };
 void foo() {

 namespace test4 {
 // Don't try to build debug info for a dependent enum.
 // CHECK-NOT: test4
 template <typename T>
 struct S {
   enum e { E = T::v };
 };
 }
+
+namespace test5 {
+// CHECK: [[E5]] = metadata !{i32 {{[^,]*}}, {{[^,]*}}, metadata [[TEST5:![0-9]*]], {{.*}}, metadata [[TEST1_ENUMS]], {{[^,]*}}, null, null, metadata !"_ZTSN5test51eE"} ; [ DW_TAG_enumeration_type ] [e]
+// CHECK: [[TEST5]] = {{.*}} ; [ DW_TAG_namespace ] [test5]
+enum e { E };
+int i = E;
+}