[Lldb-commits] [lldb] [lldb] Make semantics of SupportFile equivalence explicit (PR #97126)
via lldb-commits
lldb-commits at lists.llvm.org
Fri Jun 28 17:02:08 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Jonas Devlieghere (JDevlieghere)
<details>
<summary>Changes</summary>
This is an improved attempt to improve the semantics of SupportFile equivalence, taking into account the feedback from #<!-- -->95606.
Pavel's comment about the lack of a concise name because the concept isn't trivial made me realize that I don't want to abstract this concept away behind a helper function. Instead, I opted for a rather verbose enum that forces the caller to consider exactly what kind of comparison is appropriate for every call.
---
Full diff: https://github.com/llvm/llvm-project/pull/97126.diff
7 Files Affected:
- (modified) lldb/include/lldb/Utility/SupportFile.h (+31-8)
- (modified) lldb/source/Breakpoint/BreakpointResolver.cpp (+3-2)
- (modified) lldb/source/Commands/CommandObjectSource.cpp (+6-2)
- (modified) lldb/source/Symbol/LineEntry.cpp (+2-1)
- (modified) lldb/source/Symbol/LineTable.cpp (+1-1)
- (modified) lldb/source/Target/ThreadPlanStepOverRange.cpp (+9-6)
- (modified) lldb/source/Target/ThreadPlanStepRange.cpp (+3-2)
``````````diff
diff --git a/lldb/include/lldb/Utility/SupportFile.h b/lldb/include/lldb/Utility/SupportFile.h
index 21b986dcaba28..4b672684e4b4e 100644
--- a/lldb/include/lldb/Utility/SupportFile.h
+++ b/lldb/include/lldb/Utility/SupportFile.h
@@ -11,6 +11,7 @@
#include "lldb/Utility/Checksum.h"
#include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/Flags.h"
namespace lldb_private {
@@ -30,15 +31,37 @@ class SupportFile {
virtual ~SupportFile() = default;
- /// Return true if both SupportFiles have the same FileSpec and, if both have
- /// a valid Checksum, the Checksum is the same.
- bool operator==(const SupportFile &other) const {
- if (m_checksum && other.m_checksum)
- return m_file_spec == other.m_file_spec && m_checksum == other.m_checksum;
- return m_file_spec == other.m_file_spec;
- }
+ enum SupportFileEquality : uint8_t {
+ eEqualFileSpec = (1u << 1),
+ eEqualChecksum = (1u << 2),
+ eEqualChecksumIfSet = (1u << 3),
+ eEqualFileSpecAndChecksum = eEqualFileSpec | eEqualChecksum,
+ eEqualFileSpecAndChecksumIfSet = eEqualFileSpec | eEqualChecksumIfSet,
+ };
+
+ bool Equal(const SupportFile &other,
+ SupportFileEquality equality = eEqualFileSpecAndChecksum) const {
+ assert(!(equality & eEqualChecksum & eEqualChecksumIfSet) &&
+ "eEqualChecksum and eEqualChecksumIfSet are mutually exclusive");
+
+ if (equality & eEqualFileSpec) {
+ if (m_file_spec != other.m_file_spec)
+ return false;
+ }
- bool operator!=(const SupportFile &other) const { return !(*this == other); }
+ if (equality & eEqualChecksum) {
+ if (m_checksum != other.m_checksum)
+ return false;
+ }
+
+ if (equality & eEqualChecksumIfSet) {
+ if (m_checksum && other.m_checksum)
+ if (m_checksum != other.m_checksum)
+ return false;
+ }
+
+ return true;
+ }
/// Return the file name only. Useful for resolving breakpoints by file name.
const FileSpec &GetSpecOnly() const { return m_file_spec; };
diff --git a/lldb/source/Breakpoint/BreakpointResolver.cpp b/lldb/source/Breakpoint/BreakpointResolver.cpp
index ff4e2a9985197..2d52cbf827ef9 100644
--- a/lldb/source/Breakpoint/BreakpointResolver.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolver.cpp
@@ -222,8 +222,9 @@ void BreakpointResolver::SetSCMatchesByLine(
auto worklist_begin = std::partition(
all_scs.begin(), all_scs.end(), [&](const SymbolContext &sc) {
if (sc.line_entry.GetFile() == match.line_entry.GetFile() ||
- *sc.line_entry.original_file_sp ==
- *match.line_entry.original_file_sp) {
+ sc.line_entry.original_file_sp->Equal(
+ *match.line_entry.original_file_sp,
+ SupportFile::eEqualFileSpecAndChecksumIfSet)) {
// When a match is found, keep track of the smallest line number.
closest_line = std::min(closest_line, sc.line_entry.line);
return false;
diff --git a/lldb/source/Commands/CommandObjectSource.cpp b/lldb/source/Commands/CommandObjectSource.cpp
index 0c1267456a184..f54b712adfc46 100644
--- a/lldb/source/Commands/CommandObjectSource.cpp
+++ b/lldb/source/Commands/CommandObjectSource.cpp
@@ -747,13 +747,17 @@ class CommandObjectSourceList : public CommandObjectParsed {
bool operator==(const SourceInfo &rhs) const {
return function == rhs.function &&
- *line_entry.original_file_sp == *rhs.line_entry.original_file_sp &&
+ line_entry.original_file_sp->Equal(
+ *rhs.line_entry.original_file_sp,
+ SupportFile::eEqualFileSpecAndChecksumIfSet) &&
line_entry.line == rhs.line_entry.line;
}
bool operator!=(const SourceInfo &rhs) const {
return function != rhs.function ||
- *line_entry.original_file_sp != *rhs.line_entry.original_file_sp ||
+ !line_entry.original_file_sp->Equal(
+ *rhs.line_entry.original_file_sp,
+ SupportFile::eEqualFileSpecAndChecksumIfSet) ||
line_entry.line != rhs.line_entry.line;
}
diff --git a/lldb/source/Symbol/LineEntry.cpp b/lldb/source/Symbol/LineEntry.cpp
index 19e9bb561375b..c941a6927cb93 100644
--- a/lldb/source/Symbol/LineEntry.cpp
+++ b/lldb/source/Symbol/LineEntry.cpp
@@ -199,7 +199,8 @@ AddressRange LineEntry::GetSameLineContiguousAddressRange(
next_line_sc.line_entry.range.GetByteSize() == 0)
break;
- if (*original_file_sp == *next_line_sc.line_entry.original_file_sp &&
+ if (original_file_sp->Equal(*next_line_sc.line_entry.original_file_sp,
+ SupportFile::eEqualFileSpecAndChecksumIfSet) &&
(next_line_sc.line_entry.line == 0 ||
line == next_line_sc.line_entry.line)) {
// Include any line 0 entries - they indicate that this is compiler-
diff --git a/lldb/source/Symbol/LineTable.cpp b/lldb/source/Symbol/LineTable.cpp
index 06cf4f698316c..8fb002cc93171 100644
--- a/lldb/source/Symbol/LineTable.cpp
+++ b/lldb/source/Symbol/LineTable.cpp
@@ -360,7 +360,7 @@ void LineTable::Dump(Stream *s, Target *target, Address::DumpStyle style,
SupportFileSP prev_file;
for (size_t idx = 0; idx < count; ++idx) {
ConvertEntryAtIndexToLineEntry(idx, line_entry);
- line_entry.Dump(s, target, *prev_file != *line_entry.original_file_sp,
+ line_entry.Dump(s, target, !prev_file->Equal(*line_entry.original_file_sp),
style, fallback_style, show_line_ranges);
s->EOL();
prev_file = line_entry.original_file_sp;
diff --git a/lldb/source/Target/ThreadPlanStepOverRange.cpp b/lldb/source/Target/ThreadPlanStepOverRange.cpp
index 3fe02e0bf4faf..abe4d34bd32c2 100644
--- a/lldb/source/Target/ThreadPlanStepOverRange.cpp
+++ b/lldb/source/Target/ThreadPlanStepOverRange.cpp
@@ -220,8 +220,9 @@ bool ThreadPlanStepOverRange::ShouldStop(Event *event_ptr) {
StackFrameSP frame_sp = thread.GetStackFrameAtIndex(0);
sc = frame_sp->GetSymbolContext(eSymbolContextEverything);
if (sc.line_entry.IsValid()) {
- if (*sc.line_entry.original_file_sp !=
- *m_addr_context.line_entry.original_file_sp &&
+ if (!sc.line_entry.original_file_sp->Equal(
+ *m_addr_context.line_entry.original_file_sp,
+ SupportFile::eEqualFileSpecAndChecksumIfSet) &&
sc.comp_unit == m_addr_context.comp_unit &&
sc.function == m_addr_context.function) {
// Okay, find the next occurrence of this file in the line table:
@@ -244,8 +245,9 @@ bool ThreadPlanStepOverRange::ShouldStop(Event *event_ptr) {
LineEntry prev_line_entry;
if (line_table->GetLineEntryAtIndex(entry_idx - 1,
prev_line_entry) &&
- *prev_line_entry.original_file_sp ==
- *line_entry.original_file_sp) {
+ prev_line_entry.original_file_sp->Equal(
+ *line_entry.original_file_sp,
+ SupportFile::eEqualFileSpecAndChecksumIfSet)) {
SymbolContext prev_sc;
Address prev_address =
prev_line_entry.range.GetBaseAddress();
@@ -279,8 +281,9 @@ bool ThreadPlanStepOverRange::ShouldStop(Event *event_ptr) {
if (next_line_function != m_addr_context.function)
break;
- if (*next_line_entry.original_file_sp ==
- *m_addr_context.line_entry.original_file_sp) {
+ if (next_line_entry.original_file_sp->Equal(
+ *m_addr_context.line_entry.original_file_sp,
+ SupportFile::eEqualFileSpecAndChecksumIfSet)) {
const bool abort_other_plans = false;
const RunMode stop_other_threads = RunMode::eAllThreads;
lldb::addr_t cur_pc = thread.GetStackFrameAtIndex(0)
diff --git a/lldb/source/Target/ThreadPlanStepRange.cpp b/lldb/source/Target/ThreadPlanStepRange.cpp
index 998e76cb65d13..801856bd5419b 100644
--- a/lldb/source/Target/ThreadPlanStepRange.cpp
+++ b/lldb/source/Target/ThreadPlanStepRange.cpp
@@ -120,8 +120,9 @@ bool ThreadPlanStepRange::InRange() {
frame->GetSymbolContext(eSymbolContextEverything));
if (m_addr_context.line_entry.IsValid() &&
new_context.line_entry.IsValid()) {
- if (*m_addr_context.line_entry.original_file_sp ==
- *new_context.line_entry.original_file_sp) {
+ if (m_addr_context.line_entry.original_file_sp->Equal(
+ *new_context.line_entry.original_file_sp,
+ SupportFile::eEqualFileSpecAndChecksumIfSet)) {
if (m_addr_context.line_entry.line == new_context.line_entry.line) {
m_addr_context = new_context;
const bool include_inlined_functions =
``````````
</details>
https://github.com/llvm/llvm-project/pull/97126
More information about the lldb-commits
mailing list