[lldb] Return all line entries matchign a line if no column is specified

[kimanh]

Summary

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.

[mib]

LGTM. Thanks for fixing that.

[kimanh]

Thanks for the review!

lldb/source/Symbol/CompileUnit.cpp

void CompileUnit::ResolveSymbolContext(
   }
   
   // 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.
   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
     // 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 {

lldb/unittests/Symbol/TestLineEntry.cpp

class LineEntryTest : public testing::Test {
   SubsystemRAII<FileSystem, HostInfo, ObjectFileMachO, SymbolFileDWARF,
                 TypeSystemClang>
       subsystem;
 
 public:
   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;
 };
 
 void LineEntryTest::SetUp() {
   auto ExpectedFile = TestFile::fromYamlFile("inlined-functions.yaml");
   ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded());
   m_file.emplace(std::move(*ExpectedFile));
   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");
   m_module_sp->ResolveSymbolContextsForFileSpec(
       file_spec, line, /*check_inlines=*/true, lldb::eSymbolContextCompUnit,
       sc_comp_units);
   if (sc_comp_units.GetSize() == 0)
     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);
 
   sc_comp_units[0].comp_unit->ResolveSymbolContext(
       location_spec, eSymbolContextLineEntry, sc_line_entries);
   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);
 }
 
 /*
 # inlined-functions.cpp
 inline __attribute__((always_inline)) int sum2(int a, int b) {
     int result = a + b;
     return result;