[lldb] Let languages see all SymbolContexts at once when filtering breakpoints

[felipepiovezan]

This allows languages to make decisions based on the whole set of symbol contexts, giving them strictly more power than when they are only allowed to see one at a time.

[felipepiovezan]

The assumption above comes from the docstring for BreakpointResolver::SetSCMatchesByLine

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Felipe de Azevedo Piovezan (felipepiovezan)

Changes

This allows languages to make decisions based on the whole set of symbol contexts, giving them strictly more power than when they are only allowed to see one at a time.

---

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

2 Files Affected:

• (modified) lldb/include/lldb/Target/Language.h (+7-8)
• (modified) lldb/source/Breakpoint/BreakpointResolver.cpp (+9-10)

diff --git a/lldb/include/lldb/Target/Language.h b/lldb/include/lldb/Target/Language.h
index 38ca458159edc..b699a90aff8e4 100644
--- a/lldb/include/lldb/Target/Language.h
+++ b/lldb/include/lldb/Target/Language.h
@@ -354,14 +354,13 @@ class Language : public PluginInterface {
 
   virtual llvm::StringRef GetInstanceVariableName() { return {}; }
 
-  /// Returns true if this SymbolContext should be ignored when setting
-  /// breakpoints by line (number or regex). Helpful for languages that create
-  /// artificial functions without meaningful user code associated with them
-  /// (e.g. code that gets expanded in late compilation stages, like by
-  /// CoroSplitter).
-  virtual bool IgnoreForLineBreakpoints(const SymbolContext &) const {
-    return false;
-  }
+  /// Given a symbol context list of matches which supposedly represent the
+  /// same file and line number in a CU, erases those that should be ignored
+  /// when setting breakpoints by line (number or regex). Helpful for languages
+  /// that create split a single source-line into many functions (e.g. call
+  /// sites transformed by CoroSplitter).
+  virtual void
+  FilterForLineBreakpoints(llvm::SmallVectorImpl<SymbolContext> &) const {}
 
   /// Returns a boolean indicating whether two symbol contexts are equal for the
   /// purposes of frame comparison. If the plugin has no opinion, it should
diff --git a/lldb/source/Breakpoint/BreakpointResolver.cpp b/lldb/source/Breakpoint/BreakpointResolver.cpp
index 5fe544908c39e..91fdff4a455da 100644
--- a/lldb/source/Breakpoint/BreakpointResolver.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolver.cpp
@@ -207,16 +207,15 @@ bool operator<(const SourceLoc lhs, const SourceLoc rhs) {
 void BreakpointResolver::SetSCMatchesByLine(
     SearchFilter &filter, SymbolContextList &sc_list, bool skip_prologue,
     llvm::StringRef log_ident, uint32_t line, std::optional<uint16_t> column) {
-  llvm::SmallVector<SymbolContext, 16> all_scs;
-
-  for (const auto &sc : sc_list) {
-    if (Language::GetGlobalLanguageProperties()
-            .GetEnableFilterForLineBreakpoints())
-      if (Language *lang = Language::FindPlugin(sc.GetLanguage());
-          lang && lang->IgnoreForLineBreakpoints(sc))
-        continue;
-    all_scs.push_back(sc);
-  }
+  llvm::SmallVector<SymbolContext, 16> all_scs(sc_list.begin(), sc_list.end());
+
+  // Let the language plugin filter `sc_list`. Because all symbol contexts in
+  // sc_list are assumed to belong to the same File, Line and CU, the code below
+  // assumes they have the same language.
+  if (!sc_list.IsEmpty() && Language::GetGlobalLanguageProperties()
+                                .GetEnableFilterForLineBreakpoints())
+    if (Language *lang = Language::FindPlugin(sc_list[0].GetLanguage()))
+      lang->FilterForLineBreakpoints(all_scs);
 
   while (all_scs.size()) {
     uint32_t closest_line = UINT32_MAX;

[jimingham]

I can't see how this would hurt, and there no guarantee anywhere that these potential hits will be treated strictly independently.

So this LGTM.

[JDevlieghere]

Should this take a SymbolContextList instead?

[felipepiovezan]

The callsite doesn't use that, I suspect because it intends to manipulate the contents of the list heavily (like this method will)