jgorbe created this revision.
jgorbe added reviewers: labath, JDevlieghere.
Herald added a project: All.
jgorbe requested review of this revision.
Herald added a project: LLDB.

As far as I can tell, the only thing those `friend` classes access is the
`ValueSP` typedef.

Given that this is a map-ish class, with "Map" in its name, it doesn't seem like
a stretch to make `KeyType`, `ValueType` and `ValueSP` public. More so when the
public methods of the class have `KeyType` and `ValueSP` arguments and clearly
`ValueSP` needs to be accessed from the outside.

`friend` complicates local reasoning about who can access private members, which
is valuable in a class like this that has every method locking a mutex to
prevent concurrent access.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126103

Files:
  lldb/include/lldb/DataFormatters/TypeCategoryMap.h


Index: lldb/include/lldb/DataFormatters/TypeCategoryMap.h
===================================================================
--- lldb/include/lldb/DataFormatters/TypeCategoryMap.h
+++ lldb/include/lldb/DataFormatters/TypeCategoryMap.h
@@ -23,13 +23,13 @@
 namespace lldb_private {
 class TypeCategoryMap {
 private:
-  typedef ConstString KeyType;
-  typedef TypeCategoryImpl ValueType;
-  typedef ValueType::SharedPointer ValueSP;
   typedef std::list<lldb::TypeCategoryImplSP> ActiveCategoriesList;
   typedef ActiveCategoriesList::iterator ActiveCategoriesIterator;
 
 public:
+  typedef ConstString KeyType;
+  typedef TypeCategoryImpl ValueType;
+  typedef ValueType::SharedPointer ValueSP;
   typedef std::map<KeyType, ValueSP> MapType;
   typedef MapType::iterator MapIterator;
   typedef std::function<bool(const ValueSP &)> ForEachCallback;
@@ -103,9 +103,6 @@
   ActiveCategoriesList &active_list() { return m_active_categories; }
 
   std::recursive_mutex &mutex() { return m_map_mutex; }
-
-  friend class FormattersContainer<ValueType>;
-  friend class FormatManager;
 };
 } // namespace lldb_private
 


Index: lldb/include/lldb/DataFormatters/TypeCategoryMap.h
===================================================================
--- lldb/include/lldb/DataFormatters/TypeCategoryMap.h
+++ lldb/include/lldb/DataFormatters/TypeCategoryMap.h
@@ -23,13 +23,13 @@
 namespace lldb_private {
 class TypeCategoryMap {
 private:
-  typedef ConstString KeyType;
-  typedef TypeCategoryImpl ValueType;
-  typedef ValueType::SharedPointer ValueSP;
   typedef std::list<lldb::TypeCategoryImplSP> ActiveCategoriesList;
   typedef ActiveCategoriesList::iterator ActiveCategoriesIterator;
 
 public:
+  typedef ConstString KeyType;
+  typedef TypeCategoryImpl ValueType;
+  typedef ValueType::SharedPointer ValueSP;
   typedef std::map<KeyType, ValueSP> MapType;
   typedef MapType::iterator MapIterator;
   typedef std::function<bool(const ValueSP &)> ForEachCallback;
@@ -103,9 +103,6 @@
   ActiveCategoriesList &active_list() { return m_active_categories; }
 
   std::recursive_mutex &mutex() { return m_map_mutex; }
-
-  friend class FormattersContainer<ValueType>;
-  friend class FormatManager;
 };
 } // namespace lldb_private
 
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
  • [Lldb-commits] [PATCH] ... Jorge Gorbe Moya via Phabricator via lldb-commits

Reply via email to