teemperor created this revision.
teemperor added reviewers: shafik, labath, JDevlieghere, mib.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
teemperor added a comment.

I don't know if this needs a unit test where we call the constructor and 
explicitly check the name is the one we passed in. Let me know if you think 
this would make sense.


I often struggle to understand what exactly LLDB is doing by looking at our 
expression evaluation logging as our messages look like this:

  CompleteTagDecl[2] on (ASTContext*)0x7ff31f01d240 Completing 
(TagDecl*)0x7ff31f01d568 named DeclName1

>From the log messages it's unclear what this ASTContext is. Is it the scratch 
>context, the expression context, some decl vendor context or a context from a 
>module?
The pointer value isn't helpful for anyone unless I'm in a debugger where I 
could inspect the memory at the address. But even with a debugger it's not easy 
to
figure out what this ASTContext is without having deeper understanding about 
all the different ASTContext instances in LLDB (e.g., valid SourceLocation
from the file system usually means that this is the Objective-C decl vendor, a 
file name from multiple expressions is probably the scratch context, etc.).

This patch adds a name field to ClangASTContext instances that we can use to 
store a name which can be used for logging and debugging. With this
our log messages now look like this:

  CompleteTagDecl[2] on scratch ASTContext. Completing (TagDecl*)0x7ff31f01d568 
named Foo

We can now also just print a ClangASTContext from the debugger and see a useful 
name in the `m_display_name` field, e.g.

  m_display_name = "AST for /Users/user/test/main.o";


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D72391

Files:
  lldb/include/lldb/Symbol/ClangASTContext.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp
  lldb/source/Symbol/ClangASTContext.cpp
  lldb/unittests/Symbol/TestClangASTContext.cpp
  lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
  lldb/unittests/TestingSupport/Symbol/ClangTestUtils.h

Index: lldb/unittests/TestingSupport/Symbol/ClangTestUtils.h
===================================================================
--- lldb/unittests/TestingSupport/Symbol/ClangTestUtils.h
+++ lldb/unittests/TestingSupport/Symbol/ClangTestUtils.h
@@ -22,7 +22,8 @@
 }
 
 inline std::unique_ptr<ClangASTContext> createAST() {
-  return std::make_unique<ClangASTContext>(HostInfo::GetTargetTriple());
+  return std::make_unique<ClangASTContext>("test AST",
+                                           HostInfo::GetTargetTriple());
 }
 
 inline CompilerType createRecord(ClangASTContext &ast, llvm::StringRef name) {
Index: lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
===================================================================
--- lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
+++ lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
@@ -39,7 +39,7 @@
 // defining here, causing this test to fail, feel free to delete it.
 TEST_F(DWARFASTParserClangTests,
        EnsureAllDIEsInDeclContextHaveBeenParsedParsesOnlyMatchingEntries) {
-  ClangASTContext ast_ctx(HostInfoBase::GetTargetTriple());
+  ClangASTContext ast_ctx("dummy", HostInfoBase::GetTargetTriple());
   DWARFASTParserClangStub ast_parser(ast_ctx);
 
   DWARFUnit *unit = nullptr;
Index: lldb/unittests/Symbol/TestClangASTContext.cpp
===================================================================
--- lldb/unittests/Symbol/TestClangASTContext.cpp
+++ lldb/unittests/Symbol/TestClangASTContext.cpp
@@ -26,7 +26,7 @@
   SubsystemRAII<FileSystem, HostInfo> subsystems;
 
   void SetUp() override {
-    m_ast.reset(new ClangASTContext(HostInfo::GetTargetTriple()));
+    m_ast.reset(new ClangASTContext("test AST", HostInfo::GetTargetTriple()));
   }
 
   void TearDown() override { m_ast.reset(); }
Index: lldb/source/Symbol/ClangASTContext.cpp
===================================================================
--- lldb/source/Symbol/ClangASTContext.cpp
+++ lldb/source/Symbol/ClangASTContext.cpp
@@ -499,7 +499,9 @@
   Opts.NoInlineDefine = !Opt;
 }
 
-ClangASTContext::ClangASTContext(llvm::Triple target_triple) {
+ClangASTContext::ClangASTContext(llvm::StringRef name,
+                                 llvm::Triple target_triple) {
+  m_display_name = name.str();
   if (!target_triple.str().empty())
     SetTargetTriple(target_triple.str());
   // The caller didn't pass an ASTContext so create a new one for this
@@ -507,7 +509,9 @@
   CreateASTContext();
 }
 
-ClangASTContext::ClangASTContext(ASTContext &existing_ctxt) {
+ClangASTContext::ClangASTContext(llvm::StringRef name,
+                                 ASTContext &existing_ctxt) {
+  m_display_name = name.str();
   SetTargetTriple(existing_ctxt.getTargetInfo().getTriple().str());
 
   m_ast_up.reset(&existing_ctxt);
@@ -556,9 +560,10 @@
     }
   }
 
-  if (module)
-    return std::make_shared<ClangASTContext>(triple);
-  else if (target && target->IsValid())
+  if (module) {
+    std::string ast_name = "AST for '" + module->GetFileSpec().GetPath() + "'";
+    return std::make_shared<ClangASTContext>(ast_name, triple);
+  } else if (target && target->IsValid())
     return std::make_shared<ClangASTContextForExpressions>(*target, triple);
   return lldb::TypeSystemSP();
 }
@@ -9246,7 +9251,8 @@
 
 ClangASTContextForExpressions::ClangASTContextForExpressions(
     Target &target, llvm::Triple triple)
-    : ClangASTContext(triple), m_target_wp(target.shared_from_this()),
+    : ClangASTContext("scratch ASTContext", triple),
+      m_target_wp(target.shared_from_this()),
       m_persistent_variables(new ClangPersistentVariables) {
   m_scratch_ast_source_up.reset(new ClangASTSource(
       target.shared_from_this(), target.GetClangASTImporter()));
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp
===================================================================
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp
@@ -23,10 +23,9 @@
     ObjCLanguageRuntime &runtime)
     : ObjCLanguageRuntime::EncodingToType(), m_runtime(runtime) {
   if (!m_scratch_ast_ctx_up)
-    m_scratch_ast_ctx_up.reset(new ClangASTContext(runtime.GetProcess()
-                                                       ->GetTarget()
-                                                       .GetArchitecture()
-                                                       .GetTriple()));
+    m_scratch_ast_ctx_up.reset(new ClangASTContext(
+        "AppleObjCTypeEncodingParser AST",
+        runtime.GetProcess()->GetTarget().GetArchitecture().GetTriple()));
 }
 
 std::string AppleObjCTypeEncodingParser::ReadStructName(StringLexer &type) {
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
===================================================================
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
@@ -143,10 +143,9 @@
 
 AppleObjCDeclVendor::AppleObjCDeclVendor(ObjCLanguageRuntime &runtime)
     : ClangDeclVendor(eAppleObjCDeclVendor), m_runtime(runtime),
-      m_ast_ctx(runtime.GetProcess()
-                    ->GetTarget()
-                    .GetArchitecture()
-                    .GetTriple()),
+      m_ast_ctx(
+          "AppleObjCDeclVendor AST",
+          runtime.GetProcess()->GetTarget().GetArchitecture().GetTriple()),
       m_type_realizer_sp(m_runtime.GetEncodingToType()) {
   m_external_source = new AppleObjCExternalASTSource(*this);
   llvm::IntrusiveRefCntPtr<clang::ExternalASTSource> external_source_owning_ptr(
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
@@ -160,7 +160,8 @@
       m_parser(std::move(parser)) {
 
   // Initialize our ClangASTContext.
-  m_ast_context.reset(new ClangASTContext(m_compiler_instance->getASTContext()));
+  m_ast_context.reset(new ClangASTContext(
+      "ClangModulesDeclVendor AST", m_compiler_instance->getASTContext()));
 }
 
 void ClangModulesDeclVendorImpl::ReportModuleExportsHelper(
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -605,7 +605,8 @@
   m_compiler->createASTContext();
   clang::ASTContext &ast_context = m_compiler->getASTContext();
 
-  m_ast_context.reset(new ClangASTContext(ast_context));
+  m_ast_context.reset(new ClangASTContext(
+      "Expression AST for '" + m_filename + "'", ast_context));
 
   std::string module_name("$__lldb_module");
 
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
@@ -201,9 +201,9 @@
 
   if (log) {
     LLDB_LOGF(log,
-              "    CompleteTagDecl[%u] on (ASTContext*)%p Completing "
+              "    CompleteTagDecl[%u] on %s. Completing "
               "(TagDecl*)%p named %s",
-              current_id, static_cast<void *>(m_ast_context),
+              current_id, m_clang_ast_context->getDisplayName().c_str(),
               static_cast<void *>(tag_decl), tag_decl->getName().str().c_str());
 
     LLDB_LOG(log, "      CTD[%u] Before:\n{0}", current_id,
@@ -336,9 +336,9 @@
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
 
   LLDB_LOGF(log,
-            "    [CompleteObjCInterfaceDecl] on (ASTContext*)%p Completing "
+            "    [CompleteObjCInterfaceDecl] on %s. Completing "
             "an ObjCInterfaceDecl named %s",
-            static_cast<void *>(m_ast_context),
+            m_clang_ast_context->getDisplayName().c_str(),
             interface_decl->getName().str().c_str());
   LLDB_LOG(log, "      [COID] Before:\n{0}",
            ClangUtil::DumpDecl(interface_decl));
@@ -441,24 +441,19 @@
 
   if (log) {
     if (const NamedDecl *context_named_decl = dyn_cast<NamedDecl>(context_decl))
-      LLDB_LOGF(
-          log,
-          "FindExternalLexicalDecls[%u] on (ASTContext*)%p in '%s' (%sDecl*)%p",
-          current_id, static_cast<void *>(m_ast_context),
-          context_named_decl->getNameAsString().c_str(),
-          context_decl->getDeclKindName(),
-          static_cast<const void *>(context_decl));
+      LLDB_LOGF(log, "FindExternalLexicalDecls[%u] on %s in '%s' (%sDecl*)%p",
+                current_id, m_clang_ast_context->getDisplayName().c_str(),
+                context_named_decl->getNameAsString().c_str(),
+                context_decl->getDeclKindName(),
+                static_cast<const void *>(context_decl));
     else if (context_decl)
-      LLDB_LOGF(
-          log, "FindExternalLexicalDecls[%u] on (ASTContext*)%p in (%sDecl*)%p",
-          current_id, static_cast<void *>(m_ast_context),
-          context_decl->getDeclKindName(),
-          static_cast<const void *>(context_decl));
+      LLDB_LOGF(log, "FindExternalLexicalDecls[%u] on %s in (%sDecl*)%p",
+                current_id, m_clang_ast_context->getDisplayName().c_str(),
+                context_decl->getDeclKindName(),
+                static_cast<const void *>(context_decl));
     else
-      LLDB_LOGF(
-          log,
-          "FindExternalLexicalDecls[%u] on (ASTContext*)%p in a NULL context",
-          current_id, static_cast<const void *>(m_ast_context));
+      LLDB_LOGF(log, "FindExternalLexicalDecls[%u] on %s in a NULL context",
+                current_id, m_clang_ast_context->getDisplayName().c_str());
   }
 
   ClangASTImporter::DeclOrigin original = m_ast_importer_sp->GetDeclOrigin(context_decl);
@@ -466,10 +461,10 @@
   if (!original.Valid())
     return;
 
-  LLDB_LOG(
-      log, "  FELD[{0}] Original decl (ASTContext*){1:x} (Decl*){2:x}:\n{3}",
-      current_id, static_cast<void *>(original.ctx),
-      static_cast<void *>(original.decl), ClangUtil::DumpDecl(original.decl));
+  LLDB_LOG(log, "  FELD[{0}] Original decl {1} (Decl*){2:x}:\n{3}", current_id,
+           static_cast<void *>(original.ctx),
+           static_cast<void *>(original.decl),
+           ClangUtil::DumpDecl(original.decl));
 
   if (ObjCInterfaceDecl *original_iface_decl =
           dyn_cast<ObjCInterfaceDecl>(original.decl)) {
@@ -577,22 +572,22 @@
     if (!context.m_decl_context)
       LLDB_LOGF(log,
                 "ClangASTSource::FindExternalVisibleDecls[%u] on "
-                "(ASTContext*)%p for '%s' in a NULL DeclContext",
-                current_id, static_cast<void *>(m_ast_context),
+                "%s for '%s' in a NULL DeclContext",
+                current_id, m_clang_ast_context->getDisplayName().c_str(),
                 name.GetCString());
     else if (const NamedDecl *context_named_decl =
                  dyn_cast<NamedDecl>(context.m_decl_context))
       LLDB_LOGF(log,
                 "ClangASTSource::FindExternalVisibleDecls[%u] on "
-                "(ASTContext*)%p for '%s' in '%s'",
-                current_id, static_cast<void *>(m_ast_context),
+                "%s for '%s' in '%s'",
+                current_id, m_clang_ast_context->getDisplayName().c_str(),
                 name.GetCString(),
                 context_named_decl->getNameAsString().c_str());
     else
       LLDB_LOGF(log,
                 "ClangASTSource::FindExternalVisibleDecls[%u] on "
-                "(ASTContext*)%p for '%s' in a '%s'",
-                current_id, static_cast<void *>(m_ast_context),
+                "%s for '%s' in a '%s'",
+                current_id, m_clang_ast_context->getDisplayName().c_str(),
                 name.GetCString(), context.m_decl_context->getDeclKindName());
   }
 
@@ -1061,9 +1056,9 @@
   ConstString selector_name(ss.GetString());
 
   LLDB_LOGF(log,
-            "ClangASTSource::FindObjCMethodDecls[%d] on (ASTContext*)%p "
+            "ClangASTSource::FindObjCMethodDecls[%d] on %s "
             "for selector [%s %s]",
-            current_id, static_cast<void *>(m_ast_context),
+            current_id, m_clang_ast_context->getDisplayName().c_str(),
             interface_decl->getNameAsString().c_str(),
             selector_name.AsCString());
   SymbolContextList sc_list;
@@ -1358,8 +1353,8 @@
 
   LLDB_LOGF(log,
             "ClangASTSource::FindObjCPropertyAndIvarDecls[%d] on "
-            "(ASTContext*)%p for '%s.%s'",
-            current_id, static_cast<void *>(m_ast_context),
+            "%s for '%s.%s'",
+            current_id, m_clang_ast_context->getDisplayName().c_str(),
             parser_iface_decl->getNameAsString().c_str(),
             context.m_decl_name.getAsString().c_str());
 
@@ -1577,9 +1572,9 @@
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
 
   LLDB_LOGF(log,
-            "LayoutRecordType[%u] on (ASTContext*)%p for (RecordDecl*)%p "
+            "LayoutRecordType[%u] on %s for (RecordDecl*)%p "
             "[name = '%s']",
-            current_id, static_cast<void *>(m_ast_context),
+            current_id, m_clang_ast_context->getDisplayName().c_str(),
             static_cast<const void *>(record),
             record->getNameAsString().c_str());
 
@@ -1706,16 +1701,16 @@
   if (log) {
     if (parent_map && parent_map->size())
       LLDB_LOGF(log,
-                "CompleteNamespaceMap[%u] on (ASTContext*)%p Searching for "
+                "CompleteNamespaceMap[%u] on %s Searching for "
                 "namespace %s in namespace %s",
-                current_id, static_cast<void *>(m_ast_context),
+                current_id, m_clang_ast_context->getDisplayName().c_str(),
                 name.GetCString(),
                 parent_map->begin()->second.GetName().AsCString());
     else
       LLDB_LOGF(log,
-                "CompleteNamespaceMap[%u] on (ASTContext*)%p Searching for "
+                "CompleteNamespaceMap[%u] on %s Searching for "
                 "namespace %s",
-                current_id, static_cast<void *>(m_ast_context),
+                current_id, m_clang_ast_context->getDisplayName().c_str(),
                 name.GetCString());
   }
 
Index: lldb/include/lldb/Symbol/ClangASTContext.h
===================================================================
--- lldb/include/lldb/Symbol/ClangASTContext.h
+++ lldb/include/lldb/Symbol/ClangASTContext.h
@@ -57,17 +57,20 @@
 
   /// Constructs a ClangASTContext with an ASTContext using the given triple.
   ///
+  /// \param name The name for the ClangASTContext (for logging purposes)
   /// \param triple The llvm::Triple used for the ASTContext. The triple defines
   ///               certain characteristics of the ASTContext and its types
   ///               (e.g., whether certain primitive types exist or what their
   ///               signedness is).
-  explicit ClangASTContext(llvm::Triple triple);
+  explicit ClangASTContext(llvm::StringRef name, llvm::Triple triple);
 
   /// Constructs a ClangASTContext that uses an existing ASTContext internally.
   /// Useful when having an existing ASTContext created by Clang.
   ///
+  /// \param name The name for the ClangASTContext (for logging purposes)
   /// \param existing_ctxt An existing ASTContext.
-  explicit ClangASTContext(clang::ASTContext &existing_ctxt);
+  explicit ClangASTContext(llvm::StringRef name,
+                           clang::ASTContext &existing_ctxt);
 
   ~ClangASTContext() override;
 
@@ -104,6 +107,10 @@
     return llvm::dyn_cast<ClangASTContext>(&type_system_or_err.get());
   }
 
+  /// Returns the display name of this ClangASTContext that indicates what
+  /// purpose it serves in LLDB. Used for example in logs.
+  const std::string &getDisplayName() const { return m_display_name; }
+
   clang::ASTContext &getASTContext();
 
   clang::MangleContext *getMangleContext();
@@ -949,6 +956,10 @@
   std::unique_ptr<clang::MangleContext> m_mangle_ctx_up;
   uint32_t m_pointer_byte_size = 0;
   bool m_ast_owned = false;
+  /// A string describing what this ClangASTContext represents (e.g.,
+  /// AST for debug information, an expression, some other utility ClangAST).
+  /// Useful for logging and debugging.
+  std::string m_display_name;
 
   typedef llvm::DenseMap<const clang::Decl *, ClangASTMetadata> DeclMetadataMap;
   /// Maps Decls to their associated ClangASTMetadata.
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to