[Lldb-commits] [lldb] 373e2a4 - [lldb] NFC: refactor CompileUnit::ResolveSymbolContext

Konrad Kleine via lldb-commits lldb-commits at lists.llvm.org
Thu Nov 28 05:03:16 PST 2019


Author: Konrad Kleine
Date: 2019-11-28T14:00:38+01:00
New Revision: 373e2a4f69d623e59329ff801f261d8b299e12d2

URL: https://github.com/llvm/llvm-project/commit/373e2a4f69d623e59329ff801f261d8b299e12d2
DIFF: https://github.com/llvm/llvm-project/commit/373e2a4f69d623e59329ff801f261d8b299e12d2.diff

LOG: [lldb] NFC: refactor CompileUnit::ResolveSymbolContext

Summary:
I found the above named method hard to read because it had

a) many nested blocks and
b) one return statement at the end with some logic involved.

I decided to refactor this function by employing an early exit strategy.
In order to capture the logic in the return statement and to not have it
repeated more than once I chose to implement a very small lamda function
that captures all the variables it needs.

This is a non-functional change (NFC).

Reviewers: jdoerfert

Subscribers: lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D70774

Added: 
    

Modified: 
    lldb/include/lldb/Symbol/CompileUnit.h
    lldb/source/API/SBThread.cpp
    lldb/source/Core/AddressResolverFileLine.cpp
    lldb/source/Symbol/CompileUnit.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Symbol/CompileUnit.h b/lldb/include/lldb/Symbol/CompileUnit.h
index 7efbf792b1a9..b5f37f678900 100644
--- a/lldb/include/lldb/Symbol/CompileUnit.h
+++ b/lldb/include/lldb/Symbol/CompileUnit.h
@@ -381,14 +381,11 @@ class CompileUnit : public std::enable_shared_from_this<CompileUnit>,
   ///     A SymbolContext list class that will get any matching
   ///     entries appended to.
   ///
-  /// \return
-  ///     The number of new matches that were added to \a sc_list.
-  ///
   /// \see enum SymbolContext::Scope
-  uint32_t ResolveSymbolContext(const FileSpec &file_spec, uint32_t line,
-                                bool check_inlines, bool exact,
-                                lldb::SymbolContextItem resolve_scope,
-                                SymbolContextList &sc_list);
+  void ResolveSymbolContext(const FileSpec &file_spec, uint32_t line,
+                            bool check_inlines, bool exact,
+                            lldb::SymbolContextItem resolve_scope,
+                            SymbolContextList &sc_list);
 
   /// Get whether compiler optimizations were enabled for this compile unit
   ///

diff  --git a/lldb/source/API/SBThread.cpp b/lldb/source/API/SBThread.cpp
index 8d4930bf6edb..2dada9a6118d 100644
--- a/lldb/source/API/SBThread.cpp
+++ b/lldb/source/API/SBThread.cpp
@@ -914,9 +914,10 @@ SBError SBThread::StepOverUntil(lldb::SBFrame &sb_frame,
     const bool exact = false;
 
     SymbolContextList sc_list;
-    const uint32_t num_matches = frame_sc.comp_unit->ResolveSymbolContext(
-        step_file_spec, line, check_inlines, exact, eSymbolContextLineEntry,
-        sc_list);
+    frame_sc.comp_unit->ResolveSymbolContext(step_file_spec, line,
+                                             check_inlines, exact,
+                                             eSymbolContextLineEntry, sc_list);
+    const uint32_t num_matches = sc_list.GetSize();
     if (num_matches > 0) {
       SymbolContext sc;
       for (uint32_t i = 0; i < num_matches; ++i) {

diff  --git a/lldb/source/Core/AddressResolverFileLine.cpp b/lldb/source/Core/AddressResolverFileLine.cpp
index 4a14260c6c72..4122b5d3b747 100644
--- a/lldb/source/Core/AddressResolverFileLine.cpp
+++ b/lldb/source/Core/AddressResolverFileLine.cpp
@@ -40,14 +40,13 @@ Searcher::CallbackReturn
 AddressResolverFileLine::SearchCallback(SearchFilter &filter,
                                         SymbolContext &context, Address *addr) {
   SymbolContextList sc_list;
-  uint32_t sc_list_size;
   CompileUnit *cu = context.comp_unit;
 
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_BREAKPOINTS));
 
-  sc_list_size =
-      cu->ResolveSymbolContext(m_file_spec, m_line_number, m_inlines, false,
-                               eSymbolContextEverything, sc_list);
+  cu->ResolveSymbolContext(m_file_spec, m_line_number, m_inlines, false,
+                           eSymbolContextEverything, sc_list);
+  uint32_t sc_list_size = sc_list.GetSize();
   for (uint32_t i = 0; i < sc_list_size; i++) {
     SymbolContext sc;
     if (sc_list.GetContextAtIndex(i, sc)) {

diff  --git a/lldb/source/Symbol/CompileUnit.cpp b/lldb/source/Symbol/CompileUnit.cpp
index b37636c3bafc..62a1d690da42 100644
--- a/lldb/source/Symbol/CompileUnit.cpp
+++ b/lldb/source/Symbol/CompileUnit.cpp
@@ -244,11 +244,11 @@ uint32_t CompileUnit::FindLineEntry(uint32_t start_idx, uint32_t line,
   return UINT32_MAX;
 }
 
-uint32_t CompileUnit::ResolveSymbolContext(const FileSpec &file_spec,
-                                           uint32_t line, bool check_inlines,
-                                           bool exact,
-                                           SymbolContextItem resolve_scope,
-                                           SymbolContextList &sc_list) {
+void CompileUnit::ResolveSymbolContext(const FileSpec &file_spec,
+                                       uint32_t line, bool check_inlines,
+                                       bool exact,
+                                       SymbolContextItem resolve_scope,
+                                       SymbolContextList &sc_list) {
   // First find all of the file indexes that match our "file_spec". If
   // "file_spec" has an empty directory, then only compare the basenames when
   // finding file indexes
@@ -260,7 +260,7 @@ uint32_t CompileUnit::ResolveSymbolContext(const FileSpec &file_spec,
   // If we are not looking for inlined functions and our file spec doesn't
   // match then we are done...
   if (!file_spec_matches_cu_file_spec && !check_inlines)
-    return 0;
+    return;
 
   uint32_t file_idx =
       GetSupportFiles().FindFileIndex(1, file_spec, true);
@@ -271,84 +271,68 @@ uint32_t CompileUnit::ResolveSymbolContext(const FileSpec &file_spec,
 
   const size_t num_file_indexes = file_indexes.size();
   if (num_file_indexes == 0)
-    return 0;
-
-  const uint32_t prev_size = sc_list.GetSize();
+    return;
 
   SymbolContext sc(GetModule());
   sc.comp_unit = this;
 
-  if (line != 0) {
-    LineTable *line_table = sc.comp_unit->GetLineTable();
-
-    if (line_table != nullptr) {
-      uint32_t found_line;
-      uint32_t line_idx;
-
-      if (num_file_indexes == 1) {
-        // We only have a single support file that matches, so use the line
-        // table function that searches for a line entries that match a single
-        // support file index
-        LineEntry line_entry;
-        line_idx = line_table->FindLineEntryIndexByFileIndex(
-            0, file_indexes.front(), line, exact, &line_entry);
-
-        // If "exact == true", then "found_line" will be the same as "line". If
-        // "exact == false", the "found_line" will be the closest line entry
-        // with a line number greater than "line" and we will use this for our
-        // subsequent line exact matches below.
-        found_line = line_entry.line;
-
-        while (line_idx != UINT32_MAX) {
-          // If they only asked for the line entry, then we're done, we can
-          // just copy that over. But if they wanted more than just the line
-          // number, fill it in.
-          if (resolve_scope == eSymbolContextLineEntry) {
-            sc.line_entry = line_entry;
-          } else {
-            line_entry.range.GetBaseAddress().CalculateSymbolContext(
-                &sc, resolve_scope);
-          }
-
-          sc_list.Append(sc);
-          line_idx = line_table->FindLineEntryIndexByFileIndex(
-              line_idx + 1, file_indexes.front(), found_line, true,
-              &line_entry);
-        }
-      } else {
-        // We found multiple support files that match "file_spec" so use the
-        // line table function that searches for a line entries that match a
-        // multiple support file indexes.
-        LineEntry line_entry;
-        line_idx = line_table->FindLineEntryIndexByFileIndex(
-            0, file_indexes, line, exact, &line_entry);
-
-        // If "exact == true", then "found_line" will be the same as "line". If
-        // "exact == false", the "found_line" will be the closest line entry
-        // with a line number greater than "line" and we will use this for our
-        // subsequent line exact matches below.
-        found_line = line_entry.line;
-
-        while (line_idx != UINT32_MAX) {
-          if (resolve_scope == eSymbolContextLineEntry) {
-            sc.line_entry = line_entry;
-          } else {
-            line_entry.range.GetBaseAddress().CalculateSymbolContext(
-                &sc, resolve_scope);
-          }
-
-          sc_list.Append(sc);
-          line_idx = line_table->FindLineEntryIndexByFileIndex(
-              line_idx + 1, file_indexes, found_line, true, &line_entry);
-        }
-      }
-    }
-  } else if (file_spec_matches_cu_file_spec && !check_inlines) {
+  if (line == 0)
+    return;
+  
+  if (file_spec_matches_cu_file_spec && !check_inlines) {
     // only append the context if we aren't looking for inline call sites by
     // file and line and if the file spec matches that of the compile unit
     sc_list.Append(sc);
+    return;
+  }
+
+  LineTable *line_table = sc.comp_unit->GetLineTable();
+
+  if (line_table == nullptr)
+    return;
+
+  uint32_t line_idx;
+  LineEntry line_entry;
+
+  if (num_file_indexes == 1) {
+    // We only have a single support file that matches, so use the line
+    // table function that searches for a line entries that match a single
+    // support file index
+    line_idx = line_table->FindLineEntryIndexByFileIndex(
+        0, file_indexes.front(), line, exact, &line_entry);
+  } else {
+    // We found multiple support files that match "file_spec" so use the
+    // line table function that searches for a line entries that match a
+    // multiple support file indexes.
+    line_idx = line_table->FindLineEntryIndexByFileIndex(0, file_indexes, line,
+                                                         exact, &line_entry);
+  }
+  
+  // If "exact == true", then "found_line" will be the same as "line". If
+  // "exact == false", the "found_line" will be the closest line entry
+  // with a line number greater than "line" and we will use this for our
+  // subsequent line exact matches below.
+  uint32_t found_line = line_entry.line;
+  
+  while (line_idx != UINT32_MAX) {
+    // If they only asked for the line entry, then we're done, we can
+    // just copy that over. But if they wanted more than just the line
+    // number, fill it in.
+    if (resolve_scope == eSymbolContextLineEntry) {
+      sc.line_entry = line_entry;
+    } else {
+      line_entry.range.GetBaseAddress().CalculateSymbolContext(&sc,
+                                                               resolve_scope);
+    }
+
+    sc_list.Append(sc);
+    if (num_file_indexes == 1)
+      line_idx = line_table->FindLineEntryIndexByFileIndex(
+          line_idx + 1, file_indexes.front(), found_line, true, &line_entry);
+    else
+      line_idx = line_table->FindLineEntryIndexByFileIndex(
+          line_idx + 1, file_indexes, found_line, true, &line_entry);
   }
-  return sc_list.GetSize() - prev_size;
 }
 
 bool CompileUnit::GetIsOptimized() {


        


More information about the lldb-commits mailing list