Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[LLDB][NFC] Remove DWARFASTParserClang as friend from SymbolFileDWARF #70157

Merged
merged 2 commits into from Oct 25, 2023

Conversation

walter-erquinigo
Copy link
Member

This effectively moves a few functions from protected to public. In any case, for the sake of having a cleaner SymbolFileDWARF API, it's better if it's not a friend of a one of its consumers, DWARFASTParserClang.
Another effect of this change is that I can use SymbolFileDWARF for the out-of-tree mojo dwarf parser, which relies on pretty much the same functions that DWARFASTParserClang needs from SymbolFileDWARF.

This effectively moves a few functions from protected to public. In any case, for the sake of having a cleaner SymbolFileDWARF API, it's better if it's not a friend of a one of its consumers, DWARFASTParserClang.
Another effect of this change is that I can use SymbolFileDWARF for the out-of-tree mojo dwarf parser, which relies on pretty much the same functions that DWARFASTParserClang needs from SymbolFileDWARF.
@llvmbot
Copy link
Collaborator

llvmbot commented Oct 25, 2023

@llvm/pr-subscribers-lldb

Author: Walter Erquinigo (walter-erquinigo)

Changes

This effectively moves a few functions from protected to public. In any case, for the sake of having a cleaner SymbolFileDWARF API, it's better if it's not a friend of a one of its consumers, DWARFASTParserClang.
Another effect of this change is that I can use SymbolFileDWARF for the out-of-tree mojo dwarf parser, which relies on pretty much the same functions that DWARFASTParserClang needs from SymbolFileDWARF.


Full diff: https://github.com/llvm/llvm-project/pull/70157.diff

1 Files Affected:

  • (modified) lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h (+34-31)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
index 1ce62e6a6bb9e44..5b651a910e6deca 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -83,7 +83,6 @@ class SymbolFileDWARF : public SymbolFileCommon {
   friend class DWARFCompileUnit;
   friend class DWARFDIE;
   friend class DWARFASTParser;
-  friend class ::DWARFASTParserClang;
 
   // Static Functions
   static void Initialize();
@@ -138,7 +137,6 @@ class SymbolFileDWARF : public SymbolFileCommon {
 
   size_t ParseVariablesForContext(const SymbolContext &sc) override;
 
-  Type *ResolveTypeUID(lldb::user_id_t type_uid) override;
   std::optional<ArrayInfo>
   GetDynamicArrayInfoForUID(lldb::user_id_t type_uid,
                             const ExecutionContext *exe_ctx) override;
@@ -325,15 +323,46 @@ class SymbolFileDWARF : public SymbolFileCommon {
     m_file_index = file_index;
   }
 
-protected:
   typedef llvm::DenseMap<const DWARFDebugInfoEntry *, Type *> DIEToTypePtr;
-  typedef llvm::DenseMap<const DWARFDebugInfoEntry *, lldb::VariableSP>
-      DIEToVariableSP;
+
+  virtual DIEToTypePtr &GetDIEToType() { return m_die_to_type; }
+
   typedef llvm::DenseMap<const DWARFDebugInfoEntry *,
                          lldb::opaque_compiler_type_t>
       DIEToClangType;
+
+  virtual DIEToClangType &GetForwardDeclDieToClangType() {
+    return m_forward_decl_die_to_clang_type;
+  }
+
   typedef llvm::DenseMap<lldb::opaque_compiler_type_t, DIERef> ClangTypeToDIE;
 
+  virtual ClangTypeToDIE &GetForwardDeclClangTypeToDie() {
+    return m_forward_decl_clang_type_to_die;
+  }
+
+  virtual UniqueDWARFASTTypeMap &GetUniqueDWARFASTTypeMap();
+
+  bool ClassOrStructIsVirtual(const DWARFDIE &die);
+
+  SymbolFileDWARFDebugMap *GetDebugMapSymfile();
+
+  virtual lldb::TypeSP
+  FindDefinitionTypeForDWARFDeclContext(const DWARFDIE &die);
+
+  virtual lldb::TypeSP FindCompleteObjCDefinitionTypeForDIE(
+      const DWARFDIE &die, ConstString type_name, bool must_be_implementation);
+
+  Type *ResolveTypeUID(lldb::user_id_t type_uid) override;
+
+  Type *ResolveTypeUID(const DWARFDIE &die, bool assert_not_being_parsed);
+
+  Type *ResolveTypeUID(const DIERef &die_ref);
+
+protected:
+  typedef llvm::DenseMap<const DWARFDebugInfoEntry *, lldb::VariableSP>
+      DIEToVariableSP;
+
   SymbolFileDWARF(const SymbolFileDWARF &) = delete;
   const SymbolFileDWARF &operator=(const SymbolFileDWARF &) = delete;
 
@@ -371,10 +400,6 @@ class SymbolFileDWARF : public SymbolFileCommon {
   bool ParseSupportFiles(DWARFUnit &dwarf_cu, const lldb::ModuleSP &module,
                          FileSpecList &support_files);
 
-  Type *ResolveTypeUID(const DWARFDIE &die, bool assert_not_being_parsed);
-
-  Type *ResolveTypeUID(const DIERef &die_ref);
-
   lldb::VariableSP ParseVariableDIE(const SymbolContext &sc,
                                     const DWARFDIE &die,
                                     const lldb::addr_t func_low_pc);
@@ -402,8 +427,6 @@ class SymbolFileDWARF : public SymbolFileCommon {
   DIEArray MergeBlockAbstractParameters(const DWARFDIE &block_die,
                                         DIEArray &&variable_dies);
 
-  bool ClassOrStructIsVirtual(const DWARFDIE &die);
-
   // Given a die_offset, figure out the symbol context representing that die.
   bool ResolveFunction(const DWARFDIE &die, bool include_inlines,
                        SymbolContextList &sc_list);
@@ -415,12 +438,6 @@ class SymbolFileDWARF : public SymbolFileCommon {
   void ResolveFunctionAndBlock(lldb::addr_t file_vm_addr, bool lookup_block,
                                SymbolContext &sc);
 
-  virtual lldb::TypeSP
-  FindDefinitionTypeForDWARFDeclContext(const DWARFDIE &die);
-
-  virtual lldb::TypeSP FindCompleteObjCDefinitionTypeForDIE(
-      const DWARFDIE &die, ConstString type_name, bool must_be_implementation);
-
   Symbol *GetObjCClassSymbol(ConstString objc_class_name);
 
   lldb::TypeSP GetTypeForDIE(const DWARFDIE &die,
@@ -430,8 +447,6 @@ class SymbolFileDWARF : public SymbolFileCommon {
     m_debug_map_module_wp = module_sp;
   }
 
-  SymbolFileDWARFDebugMap *GetDebugMapSymfile();
-
   DWARFDIE
   FindBlockContainingSpecification(const DIERef &func_die_ref,
                                    dw_offset_t spec_block_die_offset);
@@ -440,8 +455,6 @@ class SymbolFileDWARF : public SymbolFileCommon {
   FindBlockContainingSpecification(const DWARFDIE &die,
                                    dw_offset_t spec_block_die_offset);
 
-  virtual UniqueDWARFASTTypeMap &GetUniqueDWARFASTTypeMap();
-
   bool DIEDeclContextsMatch(const DWARFDIE &die1, const DWARFDIE &die2);
 
   bool ClassContainsSelector(const DWARFDIE &class_die, ConstString selector);
@@ -473,18 +486,8 @@ class SymbolFileDWARF : public SymbolFileCommon {
 
   void UpdateExternalModuleListIfNeeded();
 
-  virtual DIEToTypePtr &GetDIEToType() { return m_die_to_type; }
-
   virtual DIEToVariableSP &GetDIEToVariable() { return m_die_to_variable_sp; }
 
-  virtual DIEToClangType &GetForwardDeclDieToClangType() {
-    return m_forward_decl_die_to_clang_type;
-  }
-
-  virtual ClangTypeToDIE &GetForwardDeclClangTypeToDie() {
-    return m_forward_decl_clang_type_to_die;
-  }
-
   void BuildCuTranslationTable();
   std::optional<uint32_t> GetDWARFUnitIndex(uint32_t cu_idx);
 

Copy link
Collaborator

@clayborg clayborg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might want to rename anything that says "ClangType*" to be "CompilerType*". This is still left over from when we didn't think we would ever have the TypeSystem stuff. See some inline comments for examples. Since we are refactoring a bit and making things public, it is a good idea to clean this up.

typedef llvm::DenseMap<const DWARFDebugInfoEntry *,
lldb::opaque_compiler_type_t>
DIEToClangType;

virtual DIEToClangType &GetForwardDeclDieToClangType() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DIEToClangType could be renamed to DieToCompilerType
ClangTypeToDIE could be renamed to ComilerTypeToDIE
Anything that is a map of something to lldb::opaque_compiler_type_t, or lldb::opaque_compiler_type_t to something can be renamed to be "CompilerType" instead of "ClangType"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! I'll do that

@walter-erquinigo walter-erquinigo merged commit 10508b6 into llvm:main Oct 25, 2023
3 checks passed
@walter-erquinigo walter-erquinigo deleted the walter/dwarf-parser branch October 25, 2023 22:04
zahiraam pushed a commit to zahiraam/llvm-project that referenced this pull request Oct 26, 2023
…llvm#70157)

This effectively moves a few functions from protected to public. In any
case, for the sake of having a cleaner SymbolFileDWARF API, it's better
if it's not a friend of a one of its consumers, DWARFASTParserClang.
Another effect of this change is that I can use SymbolFileDWARF for the
out-of-tree mojo dwarf parser, which relies on pretty much the same
functions that DWARFASTParserClang needs from SymbolFileDWARF.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants