[Lldb-commits] [PATCH] D77376: [lldb][nfc] remove overriden funcs with default impl

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
kwk marked an inline comment as done.
Closed by commit rG9072df8ac143: [lldb][nfc] remove overriden funcs with 
default impl (authored by kwk).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77376/new/

https://reviews.llvm.org/D77376

Files:
  lldb/include/lldb/Core/SearchFilter.h
  lldb/source/Core/SearchFilter.cpp

Index: lldb/source/Core/SearchFilter.cpp
===
--- lldb/source/Core/SearchFilter.cpp
+++ lldb/source/Core/SearchFilter.cpp
@@ -417,12 +417,6 @@
   return true;
 }
 
-bool SearchFilterByModule::CompUnitPasses(FileSpec ) { return true; }
-
-bool SearchFilterByModule::CompUnitPasses(CompileUnit ) {
-  return true;
-}
-
 void SearchFilterByModule::Search(Searcher ) {
   if (!m_target_sp)
 return;
@@ -543,14 +537,6 @@
   return true;
 }
 
-bool SearchFilterByModuleList::CompUnitPasses(FileSpec ) {
-  return true;
-}
-
-bool SearchFilterByModuleList::CompUnitPasses(CompileUnit ) {
-  return true;
-}
-
 void SearchFilterByModuleList::Search(Searcher ) {
   if (!m_target_sp)
 return;
Index: lldb/include/lldb/Core/SearchFilter.h
===
--- lldb/include/lldb/Core/SearchFilter.h
+++ lldb/include/lldb/Core/SearchFilter.h
@@ -98,6 +98,8 @@
   ///The file spec to check against the filter.
   /// \return
   ///\b true if \a spec passes, and \b false otherwise.
+  ///
+  /// \note the default implementation always returns \c true.
   virtual bool ModulePasses(const FileSpec );
 
   /// Call this method with a Module to see if that module passes the filter.
@@ -107,6 +109,8 @@
   ///
   /// \return
   ///\b true if \a module passes, and \b false otherwise.
+  ///
+  /// \note the default implementation always returns \c true.
   virtual bool ModulePasses(const lldb::ModuleSP _sp);
 
   /// Call this method with a Address to see if \a address passes the filter.
@@ -116,6 +120,8 @@
   ///
   /// \return
   ///\b true if \a address passes, and \b false otherwise.
+  ///
+  /// \note the default implementation always returns \c true.
   virtual bool AddressPasses(Address );
 
   /// Call this method with a FileSpec to see if \a file spec passes the
@@ -126,6 +132,8 @@
   ///
   /// \return
   ///\b true if \a file spec passes, and \b false otherwise.
+  ///
+  /// \note the default implementation always returns \c true.
   virtual bool CompUnitPasses(FileSpec );
 
   /// Call this method with a CompileUnit to see if \a comp unit passes the
@@ -136,6 +144,8 @@
   ///
   /// \return
   ///\b true if \a Comp Unit passes, and \b false otherwise.
+  ///
+  /// \note the default implementation always returns \c true.
   virtual bool CompUnitPasses(CompileUnit );
 
   /// Call this method with a Function to see if \a function passes the
@@ -321,10 +331,6 @@
 
   bool AddressPasses(Address ) override;
 
-  bool CompUnitPasses(FileSpec ) override;
-
-  bool CompUnitPasses(CompileUnit ) override;
-
   void GetDescription(Stream *s) override;
 
   uint32_t GetFilterRequiredItems() override;
@@ -372,10 +378,6 @@
 
   bool AddressPasses(Address ) override;
 
-  bool CompUnitPasses(FileSpec ) override;
-
-  bool CompUnitPasses(CompileUnit ) override;
-
   void GetDescription(Stream *s) override;
 
   uint32_t GetFilterRequiredItems() override;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D77376: [lldb][nfc] remove overriden funcs with default impl

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 255300.
kwk added a comment.

- Simplify comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77376/new/

https://reviews.llvm.org/D77376

Files:
  lldb/include/lldb/Core/SearchFilter.h
  lldb/source/Core/SearchFilter.cpp

Index: lldb/source/Core/SearchFilter.cpp
===
--- lldb/source/Core/SearchFilter.cpp
+++ lldb/source/Core/SearchFilter.cpp
@@ -417,12 +417,6 @@
   return true;
 }
 
-bool SearchFilterByModule::CompUnitPasses(FileSpec ) { return true; }
-
-bool SearchFilterByModule::CompUnitPasses(CompileUnit ) {
-  return true;
-}
-
 void SearchFilterByModule::Search(Searcher ) {
   if (!m_target_sp)
 return;
@@ -543,14 +537,6 @@
   return true;
 }
 
-bool SearchFilterByModuleList::CompUnitPasses(FileSpec ) {
-  return true;
-}
-
-bool SearchFilterByModuleList::CompUnitPasses(CompileUnit ) {
-  return true;
-}
-
 void SearchFilterByModuleList::Search(Searcher ) {
   if (!m_target_sp)
 return;
Index: lldb/include/lldb/Core/SearchFilter.h
===
--- lldb/include/lldb/Core/SearchFilter.h
+++ lldb/include/lldb/Core/SearchFilter.h
@@ -98,6 +98,8 @@
   ///The file spec to check against the filter.
   /// \return
   ///\b true if \a spec passes, and \b false otherwise.
+  ///
+  /// \note the default implementation always returns \c true.
   virtual bool ModulePasses(const FileSpec );
 
   /// Call this method with a Module to see if that module passes the filter.
@@ -107,6 +109,8 @@
   ///
   /// \return
   ///\b true if \a module passes, and \b false otherwise.
+  ///
+  /// \note the default implementation always returns \c true.
   virtual bool ModulePasses(const lldb::ModuleSP _sp);
 
   /// Call this method with a Address to see if \a address passes the filter.
@@ -116,6 +120,8 @@
   ///
   /// \return
   ///\b true if \a address passes, and \b false otherwise.
+  ///
+  /// \note the default implementation always returns \c true.
   virtual bool AddressPasses(Address );
 
   /// Call this method with a FileSpec to see if \a file spec passes the
@@ -126,6 +132,8 @@
   ///
   /// \return
   ///\b true if \a file spec passes, and \b false otherwise.
+  ///
+  /// \note the default implementation always returns \c true.
   virtual bool CompUnitPasses(FileSpec );
 
   /// Call this method with a CompileUnit to see if \a comp unit passes the
@@ -136,6 +144,8 @@
   ///
   /// \return
   ///\b true if \a Comp Unit passes, and \b false otherwise.
+  ///
+  /// \note the default implementation always returns \c true.
   virtual bool CompUnitPasses(CompileUnit );
 
   /// Call this method with a Function to see if \a function passes the
@@ -321,10 +331,6 @@
 
   bool AddressPasses(Address ) override;
 
-  bool CompUnitPasses(FileSpec ) override;
-
-  bool CompUnitPasses(CompileUnit ) override;
-
   void GetDescription(Stream *s) override;
 
   uint32_t GetFilterRequiredItems() override;
@@ -372,10 +378,6 @@
 
   bool AddressPasses(Address ) override;
 
-  bool CompUnitPasses(FileSpec ) override;
-
-  bool CompUnitPasses(CompileUnit ) override;
-
   void GetDescription(Stream *s) override;
 
   uint32_t GetFilterRequiredItems() override;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D77376: [lldb][nfc] remove overriden funcs with default impl

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk marked 2 inline comments as done.
kwk added a comment.

@jankratochvil thanks for the tip.
@labath, addressed your comments and will now push it.




Comment at: lldb/include/lldb/Core/SearchFilter.h:102
+  ///
+  /// \note if not overriden, default implementation always \c true.
   virtual bool ModulePasses(const FileSpec );

labath wrote:
> kwk wrote:
> > jankratochvil wrote:
> > > I am not against it but FYI in a paragraph above there is already 
> > > written: `The default implementation is "Everything Passes."`
> > > English: IMO missing "returns".
> > Thank you for the missing "returns". I'd like to keep the comment.
> You can keep _a_ comment if you want, but this comment is even internally 
> redundant. I don't see a need to mention overriding, saying "The default 
> (base?) implementation returns true" should be clear enough.
@labath. That is fine. I've changed the comment.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77376/new/

https://reviews.llvm.org/D77376



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D77376: [lldb][nfc] remove overriden funcs with default impl

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added inline comments.



Comment at: lldb/include/lldb/Core/SearchFilter.h:102
+  ///
+  /// \note if not overriden, default implementation always \c true.
   virtual bool ModulePasses(const FileSpec );

kwk wrote:
> jankratochvil wrote:
> > I am not against it but FYI in a paragraph above there is already written: 
> > `The default implementation is "Everything Passes."`
> > English: IMO missing "returns".
> Thank you for the missing "returns". I'd like to keep the comment.
You can keep _a_ comment if you want, but this comment is even internally 
redundant. I don't see a need to mention overriding, saying "The default 
(base?) implementation returns true" should be clear enough.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77376/new/

https://reviews.llvm.org/D77376



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D77376: [lldb][nfc] remove overriden funcs with default impl

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil accepted this revision.
jankratochvil added a comment.
This revision is now accepted and ready to land.

LGTM, thanks for the cleanup.

In D77376#1961410 , @kwk wrote:

> so I had to update the diff by hand with a bit of trouble.


I was told here already it is better to use `git diff -U9` as otherwise 
there is now `Context not available.` during this review. The line 416 comments 
are even not accessible in the diff now.

>   or should be put into `SearchFilterByModule::CompUnitPasses` as well?

`CompUnitPasses` is using `SymbolContext::comp_unit` which from my DWZ 
experience is many times not available. So for filtering by `Module`s the 
`AddressPasses` should be a more safe solution IMO.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77376/new/

https://reviews.llvm.org/D77376



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D77376: [lldb][nfc] remove overriden funcs with default impl

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a comment.

@jankratochvil for now I've re-added the `AddressPasses` functions and the 
`FIXME` comment. Can you approve?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77376/new/

https://reviews.llvm.org/D77376



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D77376: [lldb][nfc] remove overriden funcs with default impl

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 255026.
kwk edited the summary of this revision.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77376/new/

https://reviews.llvm.org/D77376

Files:
  lldb/include/lldb/Core/SearchFilter.h
  lldb/source/Core/SearchFilter.cpp

Index: lldb/source/Core/SearchFilter.cpp
===
--- lldb/source/Core/SearchFilter.cpp
+++ lldb/source/Core/SearchFilter.cpp
@@ -417,12 +417,6 @@
   return true;
 }
 
-bool SearchFilterByModule::CompUnitPasses(FileSpec ) { return true; }
-
-bool SearchFilterByModule::CompUnitPasses(CompileUnit ) {
-  return true;
-}
-
 void SearchFilterByModule::Search(Searcher ) {
   if (!m_target_sp)
 return;
@@ -543,14 +537,6 @@
   return true;
 }
 
-bool SearchFilterByModuleList::CompUnitPasses(FileSpec ) {
-  return true;
-}
-
-bool SearchFilterByModuleList::CompUnitPasses(CompileUnit ) {
-  return true;
-}
-
 void SearchFilterByModuleList::Search(Searcher ) {
   if (!m_target_sp)
 return;
Index: lldb/include/lldb/Core/SearchFilter.h
===
--- lldb/include/lldb/Core/SearchFilter.h
+++ lldb/include/lldb/Core/SearchFilter.h
@@ -98,6 +98,8 @@
   ///The file spec to check against the filter.
   /// \return
   ///\b true if \a spec passes, and \b false otherwise.
+  ///
+  /// \note if not overriden, the default implementation always returns \c true.
   virtual bool ModulePasses(const FileSpec );
 
   /// Call this method with a Module to see if that module passes the filter.
@@ -107,6 +109,8 @@
   ///
   /// \return
   ///\b true if \a module passes, and \b false otherwise.
+  ///
+  /// \note if not overriden, the default implementation always returns \c true.
   virtual bool ModulePasses(const lldb::ModuleSP _sp);
 
   /// Call this method with a Address to see if \a address passes the filter.
@@ -116,6 +120,8 @@
   ///
   /// \return
   ///\b true if \a address passes, and \b false otherwise.
+  ///
+  /// \note if not overriden, the default implementation always returns \c true.
   virtual bool AddressPasses(Address );
 
   /// Call this method with a FileSpec to see if \a file spec passes the
@@ -126,6 +132,8 @@
   ///
   /// \return
   ///\b true if \a file spec passes, and \b false otherwise.
+  ///
+  /// \note if not overriden, the default implementation always returns \c true.
   virtual bool CompUnitPasses(FileSpec );
 
   /// Call this method with a CompileUnit to see if \a comp unit passes the
@@ -136,6 +144,8 @@
   ///
   /// \return
   ///\b true if \a Comp Unit passes, and \b false otherwise.
+  ///
+  /// \note if not overriden, the default implementation always returns \c true.
   virtual bool CompUnitPasses(CompileUnit );
 
   /// Call this method with a Function to see if \a function passes the
@@ -321,10 +331,6 @@
 
   bool AddressPasses(Address ) override;
 
-  bool CompUnitPasses(FileSpec ) override;
-
-  bool CompUnitPasses(CompileUnit ) override;
-
   void GetDescription(Stream *s) override;
 
   uint32_t GetFilterRequiredItems() override;
@@ -372,10 +378,6 @@
 
   bool AddressPasses(Address ) override;
 
-  bool CompUnitPasses(FileSpec ) override;
-
-  bool CompUnitPasses(CompileUnit ) override;
-
   void GetDescription(Stream *s) override;
 
   uint32_t GetFilterRequiredItems() override;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D77376: [lldb][nfc] remove overriden funcs with default impl

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a subscriber: lattner.
kwk added a comment.

Had some problem with my `arc diff` reporting that another git was running 
which it wasn't so I had to update the diff by hand with a bit of trouble.




Comment at: lldb/include/lldb/Core/SearchFilter.h:102
+  ///
+  /// \note if not overriden, default implementation always \c true.
   virtual bool ModulePasses(const FileSpec );

jankratochvil wrote:
> I am not against it but FYI in a paragraph above there is already written: 
> `The default implementation is "Everything Passes."`
> English: IMO missing "returns".
Thank you for the missing "returns". I'd like to keep the comment.



Comment at: lldb/source/Core/SearchFilter.cpp:416
-bool SearchFilterByModule::AddressPasses(Address ) {
-  // FIXME: Not yet implemented
-  return true;

jankratochvil wrote:
> This comment will get lost. Maybe you could keep this override just due to 
> the FIXME comment. Or keep the FIXME comment some other way there. Or file a 
> Bugzilla tracking Bug instead.
> It is there since: [[ 
> https://github.com/llvm/llvm-project/commit/30fdc8d841c9d24ac5f3d452b6ece84ee0ac991c
>  | Initial checkin of lldb code from internal Apple repo. ]]
I've checked the code myself to find out where it was coming from and found out 
that the FIXME is there for almost 10 years. I wonder if @lattner still 
remembers why it was there or if it can be removed.  By the nature of the other 
default implementations below, I wonder if this was accidentally a left-over or 
should be put into `SearchFilterByModule::CompUnitPasses` as well? 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77376/new/

https://reviews.llvm.org/D77376



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D77376: [lldb][nfc] remove overriden funcs with default impl

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 255024.
kwk marked 3 inline comments as done.
kwk added a comment.

Re-added `AddressPasses` with FIXME comment


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77376/new/

https://reviews.llvm.org/D77376

Files:
  lldb/source/Core/SearchFilter.cpp


Index: lldb/source/Core/SearchFilter.cpp
===
--- lldb/source/Core/SearchFilter.cpp
+++ lldb/source/Core/SearchFilter.cpp
@@ -412,6 +412,11 @@
   return FileSpec::Match(m_module_spec, spec);
 }
 
+bool SearchFilterByModule::AddressPasses(Address ) {
+  // FIXME: Not yet implemented
+  return true;
+}
+
 void SearchFilterByModule::Search(Searcher ) {
   if (!m_target_sp)
 return;
@@ -527,6 +532,11 @@
   return m_module_spec_list.FindFileIndex(0, spec, true) != UINT32_MAX;
 }
 
+bool SearchFilterByModuleList::AddressPasses(Address ) {
+  // FIXME: Not yet implemented
+  return true;
+}
+
 void SearchFilterByModuleList::Search(Searcher ) {
   if (!m_target_sp)
 return;


Index: lldb/source/Core/SearchFilter.cpp
===
--- lldb/source/Core/SearchFilter.cpp
+++ lldb/source/Core/SearchFilter.cpp
@@ -412,6 +412,11 @@
   return FileSpec::Match(m_module_spec, spec);
 }
 
+bool SearchFilterByModule::AddressPasses(Address ) {
+  // FIXME: Not yet implemented
+  return true;
+}
+
 void SearchFilterByModule::Search(Searcher ) {
   if (!m_target_sp)
 return;
@@ -527,6 +532,11 @@
   return m_module_spec_list.FindFileIndex(0, spec, true) != UINT32_MAX;
 }
 
+bool SearchFilterByModuleList::AddressPasses(Address ) {
+  // FIXME: Not yet implemented
+  return true;
+}
+
 void SearchFilterByModuleList::Search(Searcher ) {
   if (!m_target_sp)
 return;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D77376: [lldb][nfc] remove overriden funcs with default impl

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 255022.
kwk added a comment.

- fix comment


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77376/new/

https://reviews.llvm.org/D77376

Files:
  lldb/include/lldb/Core/SearchFilter.h
  lldb/source/Core/SearchFilter.cpp

Index: lldb/source/Core/SearchFilter.cpp
===
--- lldb/source/Core/SearchFilter.cpp
+++ lldb/source/Core/SearchFilter.cpp
@@ -412,17 +412,6 @@
   return FileSpec::Match(m_module_spec, spec);
 }
 
-bool SearchFilterByModule::AddressPasses(Address ) {
-  // FIXME: Not yet implemented
-  return true;
-}
-
-bool SearchFilterByModule::CompUnitPasses(FileSpec ) { return true; }
-
-bool SearchFilterByModule::CompUnitPasses(CompileUnit ) {
-  return true;
-}
-
 void SearchFilterByModule::Search(Searcher ) {
   if (!m_target_sp)
 return;
@@ -538,19 +527,6 @@
   return m_module_spec_list.FindFileIndex(0, spec, true) != UINT32_MAX;
 }
 
-bool SearchFilterByModuleList::AddressPasses(Address ) {
-  // FIXME: Not yet implemented
-  return true;
-}
-
-bool SearchFilterByModuleList::CompUnitPasses(FileSpec ) {
-  return true;
-}
-
-bool SearchFilterByModuleList::CompUnitPasses(CompileUnit ) {
-  return true;
-}
-
 void SearchFilterByModuleList::Search(Searcher ) {
   if (!m_target_sp)
 return;
Index: lldb/include/lldb/Core/SearchFilter.h
===
--- lldb/include/lldb/Core/SearchFilter.h
+++ lldb/include/lldb/Core/SearchFilter.h
@@ -98,6 +98,8 @@
   ///The file spec to check against the filter.
   /// \return
   ///\b true if \a spec passes, and \b false otherwise.
+  ///
+  /// \note if not overriden, the default implementation always returns \c true.
   virtual bool ModulePasses(const FileSpec );
 
   /// Call this method with a Module to see if that module passes the filter.
@@ -107,6 +109,8 @@
   ///
   /// \return
   ///\b true if \a module passes, and \b false otherwise.
+  ///
+  /// \note if not overriden, the default implementation always returns \c true.
   virtual bool ModulePasses(const lldb::ModuleSP _sp);
 
   /// Call this method with a Address to see if \a address passes the filter.
@@ -116,6 +120,8 @@
   ///
   /// \return
   ///\b true if \a address passes, and \b false otherwise.
+  ///
+  /// \note if not overriden, the default implementation always returns \c true.
   virtual bool AddressPasses(Address );
 
   /// Call this method with a FileSpec to see if \a file spec passes the
@@ -126,6 +132,8 @@
   ///
   /// \return
   ///\b true if \a file spec passes, and \b false otherwise.
+  ///
+  /// \note if not overriden, the default implementation always returns \c true.
   virtual bool CompUnitPasses(FileSpec );
 
   /// Call this method with a CompileUnit to see if \a comp unit passes the
@@ -136,6 +144,8 @@
   ///
   /// \return
   ///\b true if \a Comp Unit passes, and \b false otherwise.
+  ///
+  /// \note if not overriden, the default implementation always returns \c true.
   virtual bool CompUnitPasses(CompileUnit );
 
   /// Call this method with a Function to see if \a function passes the
@@ -319,12 +329,6 @@
 
   bool ModulePasses(const FileSpec ) override;
 
-  bool AddressPasses(Address ) override;
-
-  bool CompUnitPasses(FileSpec ) override;
-
-  bool CompUnitPasses(CompileUnit ) override;
-
   void GetDescription(Stream *s) override;
 
   uint32_t GetFilterRequiredItems() override;
@@ -370,12 +374,6 @@
 
   bool ModulePasses(const FileSpec ) override;
 
-  bool AddressPasses(Address ) override;
-
-  bool CompUnitPasses(FileSpec ) override;
-
-  bool CompUnitPasses(CompileUnit ) override;
-
   void GetDescription(Stream *s) override;
 
   uint32_t GetFilterRequiredItems() override;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D77376: [lldb][nfc] remove overriden funcs with default impl

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added inline comments.



Comment at: lldb/include/lldb/Core/SearchFilter.h:102
+  ///
+  /// \note if not overriden, default implementation always \c true.
   virtual bool ModulePasses(const FileSpec );

I am not against it but FYI in a paragraph above there is already written: `The 
default implementation is "Everything Passes."`
English: IMO missing "returns".



Comment at: lldb/source/Core/SearchFilter.cpp:416
-bool SearchFilterByModule::AddressPasses(Address ) {
-  // FIXME: Not yet implemented
-  return true;

This comment will get lost. Maybe you could keep this override just due to the 
FIXME comment. Or keep the FIXME comment some other way there. Or file a 
Bugzilla tracking Bug instead.
It is there since: [[ 
https://github.com/llvm/llvm-project/commit/30fdc8d841c9d24ac5f3d452b6ece84ee0ac991c
 | Initial checkin of lldb code from internal Apple repo. ]]



Comment at: lldb/source/Core/SearchFilter.cpp:542
-bool SearchFilterByModuleList::AddressPasses(Address ) {
-  // FIXME: Not yet implemented
-  return true;

Likewise.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77376/new/

https://reviews.llvm.org/D77376



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D77376: [lldb][nfc] remove overriden funcs with default impl

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk created this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

These three `SearchFilter` methods all return `true` by their default
implementation:

  virtual bool ModulePasses(const FileSpec );
  virtual bool ModulePasses(const lldb::ModuleSP _sp);
  virtual bool AddressPasses(Address );
  virtual bool CompUnitPasses(FileSpec );
  virtual bool CompUnitPasses(CompileUnit );

That's why I've documented the default behavior and remove the overrides
in these `SearchFilter`-subclasses which all just repeated the default
implementation: `SearchFilterByModule`, `SearchFilterByModuleList`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77376

Files:
  lldb/include/lldb/Core/SearchFilter.h
  lldb/source/Core/SearchFilter.cpp

Index: lldb/source/Core/SearchFilter.cpp
===
--- lldb/source/Core/SearchFilter.cpp
+++ lldb/source/Core/SearchFilter.cpp
@@ -412,17 +412,6 @@
   return FileSpec::Match(m_module_spec, spec);
 }
 
-bool SearchFilterByModule::AddressPasses(Address ) {
-  // FIXME: Not yet implemented
-  return true;
-}
-
-bool SearchFilterByModule::CompUnitPasses(FileSpec ) { return true; }
-
-bool SearchFilterByModule::CompUnitPasses(CompileUnit ) {
-  return true;
-}
-
 void SearchFilterByModule::Search(Searcher ) {
   if (!m_target_sp)
 return;
@@ -538,19 +527,6 @@
   return m_module_spec_list.FindFileIndex(0, spec, true) != UINT32_MAX;
 }
 
-bool SearchFilterByModuleList::AddressPasses(Address ) {
-  // FIXME: Not yet implemented
-  return true;
-}
-
-bool SearchFilterByModuleList::CompUnitPasses(FileSpec ) {
-  return true;
-}
-
-bool SearchFilterByModuleList::CompUnitPasses(CompileUnit ) {
-  return true;
-}
-
 void SearchFilterByModuleList::Search(Searcher ) {
   if (!m_target_sp)
 return;
Index: lldb/include/lldb/Core/SearchFilter.h
===
--- lldb/include/lldb/Core/SearchFilter.h
+++ lldb/include/lldb/Core/SearchFilter.h
@@ -98,6 +98,8 @@
   ///The file spec to check against the filter.
   /// \return
   ///\b true if \a spec passes, and \b false otherwise.
+  ///
+  /// \note if not overriden, default implementation always \c true.
   virtual bool ModulePasses(const FileSpec );
 
   /// Call this method with a Module to see if that module passes the filter.
@@ -107,6 +109,8 @@
   ///
   /// \return
   ///\b true if \a module passes, and \b false otherwise.
+  ///
+  /// \note if not overriden, default implementation always \c true.
   virtual bool ModulePasses(const lldb::ModuleSP _sp);
 
   /// Call this method with a Address to see if \a address passes the filter.
@@ -116,6 +120,8 @@
   ///
   /// \return
   ///\b true if \a address passes, and \b false otherwise.
+  ///
+  /// \note if not overriden, default implementation always \c true.
   virtual bool AddressPasses(Address );
 
   /// Call this method with a FileSpec to see if \a file spec passes the
@@ -126,6 +132,8 @@
   ///
   /// \return
   ///\b true if \a file spec passes, and \b false otherwise.
+  ///
+  /// \note if not overriden, default implementation always \c true.
   virtual bool CompUnitPasses(FileSpec );
 
   /// Call this method with a CompileUnit to see if \a comp unit passes the
@@ -136,6 +144,8 @@
   ///
   /// \return
   ///\b true if \a Comp Unit passes, and \b false otherwise.
+  ///
+  /// \note if not overriden, default implementation always \c true.
   virtual bool CompUnitPasses(CompileUnit );
 
   /// Call this method with a Function to see if \a function passes the
@@ -319,12 +329,6 @@
 
   bool ModulePasses(const FileSpec ) override;
 
-  bool AddressPasses(Address ) override;
-
-  bool CompUnitPasses(FileSpec ) override;
-
-  bool CompUnitPasses(CompileUnit ) override;
-
   void GetDescription(Stream *s) override;
 
   uint32_t GetFilterRequiredItems() override;
@@ -370,12 +374,6 @@
 
   bool ModulePasses(const FileSpec ) override;
 
-  bool AddressPasses(Address ) override;
-
-  bool CompUnitPasses(FileSpec ) override;
-
-  bool CompUnitPasses(CompileUnit ) override;
-
   void GetDescription(Stream *s) override;
 
   uint32_t GetFilterRequiredItems() override;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits