[lldb][nfc] remove overriden funcs with default impl

[kwk]

Summary

These SearchFilter methods all return true by their default
implementation:

virtual bool ModulePasses(const FileSpec &spec);
virtual bool ModulePasses(const lldb::ModuleSP &module_sp);
virtual bool AddressPasses(Address &addr);
virtual bool CompUnitPasses(FileSpec &fileSpec);
virtual bool CompUnitPasses(CompileUnit &compUnit);

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

[jankratochvil]

lldb/include/lldb/Core/SearchFilter.h

102

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".

lldb/source/Core/SearchFilter.cpp

416

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: Initial checkin of lldb code from internal Apple repo.

542

Likewise.

[kwk]

• fix comment

[kwk]

Re-added AddressPasses with FIXME comment

[kwk]

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.

lldb/include/lldb/Core/SearchFilter.h

102

Thank you for the missing "returns". I'd like to keep the comment.

lldb/source/Core/SearchFilter.cpp

416

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?

[kwk]

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

[jankratochvil]

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 -U999999999 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 Modules the AddressPasses should be a more safe solution IMO.

[labath]

lldb/include/lldb/Core/SearchFilter.h

102

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.

[kwk]

• Simplify comments

[kwk]

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

lldb/include/lldb/Core/SearchFilter.h

102

@labath. That is fine. I've changed the comment.

lldb/include/lldb/Core/SearchFilter.h

public:
   virtual ~SearchFilter();
 
   /// Call this method with a file spec to see if that spec passes the filter.
   ///
   /// \param[in] spec
   ///    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 &spec);
 
   /// Call this method with a Module to see if that module passes the filter.
   ///
   /// \param[in] module_sp
   ///    The Module to check against the filter.
   ///
   /// \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 &module_sp);
 
   /// Call this method with a Address to see if \a address passes the filter.
   ///
   /// \param[in] addr
   ///    The address to check against the filter.
   ///
   /// \return
   ///    \b true if \a address passes, and \b false otherwise.
+  ///
+  /// \note the default implementation always returns \c true.
   virtual bool AddressPasses(Address &addr);
 
   /// Call this method with a FileSpec to see if \a file spec passes the
   /// filter as the name of a compilation unit.
   ///
   /// \param[in] fileSpec
   ///    The file spec to check against the filter.
   ///
   /// \return
   ///    \b true if \a file spec passes, and \b false otherwise.
+  ///
+  /// \note the default implementation always returns \c true.
   virtual bool CompUnitPasses(FileSpec &fileSpec);
 
   /// Call this method with a CompileUnit to see if \a comp unit passes the
   /// filter.
   ///
   /// \param[in] compUnit
   ///    The CompileUnit to check against the filter.
   ///
   /// \return
   ///    \b true if \a Comp Unit passes, and \b false otherwise.
+  ///
+  /// \note the default implementation always returns \c true.
   virtual bool CompUnitPasses(CompileUnit &compUnit);
 
   /// Call this method with a Function to see if \a function passes the
   /// filter.
   ///
   /// \param[in] function
   ///    The Functions to check against the filter.
   ///
public:
   ~SearchFilterByModule() override;
 
   bool ModulePasses(const lldb::ModuleSP &module_sp) override;
 
   bool ModulePasses(const FileSpec &spec) override;
 
   bool AddressPasses(Address &address) override;
 
-  bool CompUnitPasses(FileSpec &fileSpec) override;
-
-  bool CompUnitPasses(CompileUnit &compUnit) override;
-
   void GetDescription(Stream *s) override;
 
   uint32_t GetFilterRequiredItems() override;
 
   void Dump(Stream *s) const override;
 
   void Search(Searcher &searcher) override;
 
public:
   ~SearchFilterByModuleList() override;
 
   bool ModulePasses(const lldb::ModuleSP &module_sp) override;
 
   bool ModulePasses(const FileSpec &spec) override;
 
   bool AddressPasses(Address &address) override;
 
-  bool CompUnitPasses(FileSpec &fileSpec) override;
-
-  bool CompUnitPasses(CompileUnit &compUnit) override;
-
   void GetDescription(Stream *s) override;
 
   uint32_t GetFilterRequiredItems() override;
 
   void Dump(Stream *s) const override;
 
   void Search(Searcher &searcher) override;
 

lldb/source/Core/SearchFilter.cpp

return (module_sp &&
           FileSpec::Match(m_module_spec, module_sp->GetFileSpec()));
 }
 
 bool SearchFilterByModule::ModulePasses(const FileSpec &spec) {
   return FileSpec::Match(m_module_spec, spec);
 }
 
 bool SearchFilterByModule::AddressPasses(Address &address) {
   // FIXME: Not yet implemented
   return true;
 }
 
-bool SearchFilterByModule::CompUnitPasses(FileSpec &fileSpec) { return true; }
-
-bool SearchFilterByModule::CompUnitPasses(CompileUnit &compUnit) {
-  return true;
-}
-
 void SearchFilterByModule::Search(Searcher &searcher) {
   if (!m_target_sp)
     return;
 
   if (searcher.GetDepth() == lldb::eSearchDepthTarget) {
     SymbolContext empty_sc;
     empty_sc.target_sp = m_target_sp;
     searcher.SearchCallback(*this, empty_sc, nullptr);

 bool SearchFilterByModuleList::ModulePasses(const FileSpec &spec) {
   if (m_module_spec_list.GetSize() == 0)
     return true;
 
   return m_module_spec_list.FindFileIndex(0, spec, true) != UINT32_MAX;
 }
 
 bool SearchFilterByModuleList::AddressPasses(Address &address) {
   // FIXME: Not yet implemented
   return true;
 }
 
-bool SearchFilterByModuleList::CompUnitPasses(FileSpec &fileSpec) {
-  return true;
-}
-
-bool SearchFilterByModuleList::CompUnitPasses(CompileUnit &compUnit) {
-  return true;
-}
-
 void SearchFilterByModuleList::Search(Searcher &searcher) {
   if (!m_target_sp)
     return;
 
   if (searcher.GetDepth() == lldb::eSearchDepthTarget) {
     SymbolContext empty_sc;
     empty_sc.target_sp = m_target_sp;
     searcher.SearchCallback(*this, empty_sc, nullptr);