kimanh created this revision.
kimanh added reviewers: mib, JDevlieghere.
kimanh published this revision for review.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Previously, if no column was specified, ResolveSymbolContext would take
the first match returned by FindLineEntryIndexByFileIndex, and reuse it
to find subsequent exact matches. With the introduction of columns, columns
are now considered when matching the line entries.

This leads to a problem if one wants to get all existing line entries
that match that line, since now the column is also used for the exact match.
This way, all line entries are filtered out that have a different
column number, but the same line number.

This patch changes that by ignoring the column information of the first match
if the original request of ResolveSymbolContext was also ignoring it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108816

Files:
  lldb/source/Symbol/CompileUnit.cpp
  lldb/unittests/Symbol/TestLineEntry.cpp

Index: lldb/unittests/Symbol/TestLineEntry.cpp
===================================================================
--- lldb/unittests/Symbol/TestLineEntry.cpp
+++ lldb/unittests/Symbol/TestLineEntry.cpp
@@ -38,7 +38,8 @@
   void SetUp() override;
 
 protected:
-  llvm::Expected<LineEntry> GetLineEntryForLine(uint32_t line);
+  llvm::Expected<SymbolContextList>
+  GetLineEntriesForLine(uint32_t line, llvm::Optional<uint16_t> column);
   llvm::Optional<TestFile> m_file;
   ModuleSP m_module_sp;
 };
@@ -50,8 +51,9 @@
   m_module_sp = std::make_shared<Module>(m_file->moduleSpec());
 }
 
-llvm::Expected<LineEntry> LineEntryTest::GetLineEntryForLine(uint32_t line) {
   // TODO: Handle SourceLocationSpec column information
+llvm::Expected<SymbolContextList> LineEntryTest::GetLineEntriesForLine(
+    uint32_t line, llvm::Optional<uint16_t> column = llvm::None) {
   SymbolContextList sc_comp_units;
   SymbolContextList sc_line_entries;
   FileSpec file_spec("inlined-functions.cpp");
@@ -62,7 +64,7 @@
     return llvm::createStringError(llvm::inconvertibleErrorCode(),
                                    "No comp unit found on the test object.");
 
-  SourceLocationSpec location_spec(file_spec, line, /*column=*/llvm::None,
+  SourceLocationSpec location_spec(file_spec, line, column,
                                    /*check_inlines=*/true,
                                    /*exact_match=*/true);
 
@@ -71,33 +73,53 @@
   if (sc_line_entries.GetSize() == 0)
     return llvm::createStringError(llvm::inconvertibleErrorCode(),
                                    "No line entry found on the test object.");
-  return sc_line_entries[0].line_entry;
+  return sc_line_entries;
+}
+
+// This tests if we can get all line entries that match the passed line, if
+// no column is specified.
+TEST_F(LineEntryTest, GetAllExactLineMatchesWithoutColumn) {
+  auto sc_line_entries = GetLineEntriesForLine(12);
+  ASSERT_THAT_EXPECTED(sc_line_entries, llvm::Succeeded());
+  ASSERT_EQ(sc_line_entries->NumLineEntriesWithLine(12), 6u);
+}
+
+// This tests if we can get exact line and column matches.
+TEST_F(LineEntryTest, GetAllExactLineColumnMatches) {
+  auto sc_line_entries = GetLineEntriesForLine(12, 39);
+  ASSERT_THAT_EXPECTED(sc_line_entries, llvm::Succeeded());
+  ASSERT_EQ(sc_line_entries->NumLineEntriesWithLine(12), 1u);
+  auto line_entry = sc_line_entries.get()[0].line_entry;
+  ASSERT_EQ(line_entry.column, 39);
 }
 
 TEST_F(LineEntryTest, GetSameLineContiguousAddressRangeNoInlines) {
-  auto line_entry = GetLineEntryForLine(18);
-  ASSERT_THAT_EXPECTED(line_entry, llvm::Succeeded());
+  auto sc_line_entries = GetLineEntriesForLine(18);
+  ASSERT_THAT_EXPECTED(sc_line_entries, llvm::Succeeded());
+  auto line_entry = sc_line_entries.get()[0].line_entry;
   bool include_inlined_functions = false;
   auto range =
-      line_entry->GetSameLineContiguousAddressRange(include_inlined_functions);
+      line_entry.GetSameLineContiguousAddressRange(include_inlined_functions);
   ASSERT_EQ(range.GetByteSize(), (uint64_t)0x24);
 }
 
 TEST_F(LineEntryTest, GetSameLineContiguousAddressRangeOneInline) {
-  auto line_entry = GetLineEntryForLine(18);
-  ASSERT_THAT_EXPECTED(line_entry, llvm::Succeeded());
+  auto sc_line_entries = GetLineEntriesForLine(18);
+  ASSERT_THAT_EXPECTED(sc_line_entries, llvm::Succeeded());
+  auto line_entry = sc_line_entries.get()[0].line_entry;
   bool include_inlined_functions = true;
   auto range =
-      line_entry->GetSameLineContiguousAddressRange(include_inlined_functions);
+      line_entry.GetSameLineContiguousAddressRange(include_inlined_functions);
   ASSERT_EQ(range.GetByteSize(), (uint64_t)0x49);
 }
 
 TEST_F(LineEntryTest, GetSameLineContiguousAddressRangeNestedInline) {
-  auto line_entry = GetLineEntryForLine(12);
-  ASSERT_THAT_EXPECTED(line_entry, llvm::Succeeded());
+  auto sc_line_entries = GetLineEntriesForLine(12);
+  ASSERT_THAT_EXPECTED(sc_line_entries, llvm::Succeeded());
+  auto line_entry = sc_line_entries.get()[0].line_entry;
   bool include_inlined_functions = true;
   auto range =
-      line_entry->GetSameLineContiguousAddressRange(include_inlined_functions);
+      line_entry.GetSameLineContiguousAddressRange(include_inlined_functions);
   ASSERT_EQ(range.GetByteSize(), (uint64_t)0x33);
 }
 
Index: lldb/source/Symbol/CompileUnit.cpp
===================================================================
--- lldb/source/Symbol/CompileUnit.cpp
+++ lldb/source/Symbol/CompileUnit.cpp
@@ -319,8 +319,13 @@
   // subsequent line exact matches below.
   const bool inlines = false;
   const bool exact = true;
-  SourceLocationSpec found_entry(line_entry.file, line_entry.line,
-                                 line_entry.column, inlines, exact);
+  const llvm::Optional<uint16_t> column =
+      src_location_spec.GetColumn().hasValue()
+          ? llvm::Optional<uint16_t>(line_entry.column)
+          : llvm::None;
+
+  SourceLocationSpec found_entry(line_entry.file, line_entry.line, column,
+                                 inlines, exact);
 
   while (line_idx != UINT32_MAX) {
     // If they only asked for the line entry, then we're done, we can
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to