Reintroduce accidentally deleted "protected" keyword in SymbolFileDWARF #69990
Conversation
The "protected" was accidentally removed during refactoring of SymbolFileDWARF. Reintroduce it and also make DWARFASTParser a friend class since 040c4f4 was merged and won't build without it, as it dependeds on a method which was made public by accident.
|
@llvm/pr-subscribers-lldb Author: Augusto Noronha (augusto2112) ChangesThe "protected" was accidentally removed during refactoring of SymbolFileDWARF. Reintroduce it and also make DWARFASTParser a friend class since 040c4f4 was merged and won't build without it, as it dependeds on a method which was made public by accident. Full diff: https://github.com/llvm/llvm-project/pull/69990.diff 1 Files Affected:
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
index 646d5d9a471c41c..0818ee206fe06f6 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -82,6 +82,7 @@ class SymbolFileDWARF : public SymbolFileCommon {
friend class DebugMapModule;
friend class DWARFCompileUnit;
friend class DWARFDIE;
+ friend class DWARFASTParser;
friend class ::DWARFASTParserClang;
// Static Functions
@@ -321,6 +322,7 @@ 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;
|
| @@ -321,6 +322,7 @@ class SymbolFileDWARF : public SymbolFileCommon { | |||
| m_file_index = file_index; | |||
| } | |||
|
|
|||
| protected: | |||
There was a problem hiding this comment.
My bad for merging this, but I beg you not to reintroduce it. I originally intended to have an isolated patch just for this change, but I forgot about it after being able to build my project. In fact, I'm working on an out-of-tree dwarf parser plugin for the new language Mojo, and it requires accessing the methods that were originally marked as protected.
Logically, DWARFASTParserClang is able to exist thanks to being a friend class to this one, and my plugin needs the same visibility, but for obvious reasons it cannot be declared a friend of this class.
There was a problem hiding this comment.
Which things are you using specifically? If you want to use them out of tree, we should think more carefully about which symbols actually make sense to expose.
There was a problem hiding this comment.
Can't we merge it and then you can open your isolated patch with the methods you want to expose? Right now everything is public, including ivars.
There was a problem hiding this comment.
The ivars thing make sense. Could you at least just move the ivars into a protected or private section for now? If you strongly disagree, just go ahead with this patch and then I can solve it in a new patch.
There was a problem hiding this comment.
I think the big issue on keeping everything public is that will will use those methods without realizing they were supposed to be private (like I did), and then it'll be a bigger problem to disentangle that later on.
There was a problem hiding this comment.
Sure, after you merge this patch, I'll send another one exposing only what's needed. I'll also remove DWARFASTParserClang as a friend class, because pretty much what it uses is also what I need. I'll include @bulbazord as reviewer as well.
The "protected" was accidentally removed during refactoring of SymbolFileDWARF. Reintroduce it and also make DWARFASTParser a friend class since 040c4f4 was merged and won't build without it, as it dependeds on a method which was made public by accident.