JDevlieghere added inline comments.
================
Comment at: lldb/include/lldb/Utility/SourceLocationSpec.h:13-15
+#include "lldb/lldb-defines.h"
+
+#include "llvm/ADT/Optional.h"
----------------
================
Comment at: lldb/include/lldb/Utility/SourceLocationSpec.h:108-138
+ /// Convert to boolean operator.
+ ///
+ /// This allows code to check a SourceLocationSpec object to see if it
+ /// contains anything valid using code such as:
+ ///
+ /// \code
+ /// SourceLocationSpec location_spec(...);
----------------
I guess this is no longer needed now that the factory guarantees the spec is
valid?
================
Comment at: lldb/include/lldb/Utility/SourceLocationSpec.h:183
+
+ bool HasColumn() const { return m_column.hasValue(); }
+
----------------
Is there any value to having this? I assume that if you want to know if there
is a column, it's to use it after, so you might as well call get column and use
the value if it's set.
================
Comment at: lldb/include/lldb/Utility/SourceLocationSpec.h:195-196
+ llvm::Optional<uint16_t> m_column;
+ bool m_check_inlines;
+ bool m_exact_match;
+};
----------------
Maybe add a Doxygen comment to specify what these two mean exactly.
================
Comment at: lldb/source/Utility/SourceLocationSpec.cpp:59-61
+bool SourceLocationSpec::operator!=(const SourceLocationSpec &rhs) const {
+ return !(*this == rhs);
+}
----------------
Isn't this the default implementation of `operator !=`? I think we can probably
omit it?
================
Comment at: lldb/unittests/Utility/SourceLocationSpecTest.cpp:96
+
+ // mutating FileSpec + const Inlined, ExactMatch, Line
+ EXPECT_TRUE(
----------------
================
Comment at: lldb/unittests/Utility/SourceLocationSpecTest.cpp:136-143
+ auto Create =
+ [](bool check_inlines, bool exact_match, FileSpec fs, uint32_t line,
+ uint16_t column = LLDB_INVALID_COLUMN_NUMBER) -> SourceLocationSpec {
+ auto location = SourceLocationSpec::Create(fs, line, column, check_inlines,
+ exact_match);
+ lldbassert(location && "Invalid SourceLocationSpec.");
+ return *location;
----------------
This looks identical to the `Create` above. If it is, make it a static function
to avoid code duplication.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D100962/new/
https://reviews.llvm.org/D100962
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
