[lldb/Core] Add SourceLocationSpec class (NFC)

[mib]

Summary

A source location specifier class that holds a Declaration object containing
a FileSpec with line and column information. The column line is optional.
It also holds search flags that can be fetched by resolvers to look inlined
declarations and/or exact matches.

It describes a specific location in a source file and allows the user
to perform checks and comparaisons between multiple instances of that class.

Signed-off-by: Med Ismail Bennani <[e-mail]>

[JDevlieghere]

lldb/include/lldb/Utility/SourceLocationSpec.h

13–20

91

127

The class looks mostly immutable (which is good) so I think we can omit this.

154

161

Would it make sense to merge this into Equal and replace the bool with an enum value that represents

• file + line must match,
• file + line + col must match,
• file + line + col must match if lhs has it?

179

If 0 is not a valid column, we should enforce that rather than having two ways of representing it. Can we add an assert to the ctor and make it a precondition for the class?

185

Are there situations where the line is optional too?

lldb/unittests/Utility/SourceLocationSpecTest.cpp

2–3

[mib]

lldb/include/lldb/Utility/SourceLocationSpec.h

185

Technically, m_line could be equal to 0 (if there is no line information) but that would basically mean this class is a FileSpec container.

I guess I can add an assert in the constructor checking if it's not null.

[JDevlieghere]

lldb/include/lldb/Utility/SourceLocationSpec.h

185

Indeed. Another option would be to make it a precondition that a SourceLocationSpec is always valid, assuming that unlike the column number, the file and line are usually valid. One way to do that is by adding asserts to the ctor or if this class is often created from user input, then I'd make the ctor private and add a static CreateSoureLocationSpec factory, that checks our preconditions and returns an Expected/Optional. That has the advantage of not having the change all the call sites for the few scenarios where those are actually invalid. This has the advantage of paying the price upfront when creating the LocationSpec rather than having to check in every place it's used.

[mib]

lldb/include/lldb/Utility/SourceLocationSpec.h

161

I simplified the API removing the Match method since it's the same as calling Exact with bool full = false.

[mib]

Addressed @JDevlieghere comments.

[mib]

• Move boolean attributes to the end of the class.
• Fix deleted default constructor issue.

[JDevlieghere]

lldb/include/lldb/Utility/SourceLocationSpec.h

13–15

108–138

I guess this is no longer needed now that the factory guarantees the spec is valid?

183

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.

195–196

Maybe add a Doxygen comment to specify what these two mean exactly.

lldb/source/Utility/SourceLocationSpec.cpp

59–61

Isn't this the default implementation of operator !=? I think we can probably omit it?

lldb/unittests/Utility/SourceLocationSpecTest.cpp

96

136–143

This looks identical to the Create above. If it is, make it a static function to avoid code duplication.

[mib]

lldb/source/Utility/SourceLocationSpec.cpp

59–61

No, it has to be implemented separately.

[mib]

Address @JDevlieghere comments.

[JDevlieghere]

LGTM

[mib]

Fix unit test failure.

[mib]

Group the FileSpec, the Line and Column number into a Declaration.
Remove factory function to use constructor and added an IsValid method (and boolean operator)

[mib]

Wrap SourceLocationSpec column argument into an llvm::Optional

[JDevlieghere]

LGTM

lldb/unittests/Utility/CMakeLists.txt

42

Let's exclude the change to this file from this patch.

[JDevlieghere]

lldb/source/Core/SourceLocationSpec.cpp

48–61 ↗

(On Diff #341752)

As this is only using members form Declaration, should it be implemented there?

72–77 ↗

(On Diff #341752)

Looks like Declaration has a Dump method, so I guess this should just call that.

[mib]

Address @JDevlieghere comments.

[JDevlieghere]

A few more comments about things I missed in the previous review.

lldb/source/Core/SourceLocationSpec.cpp

57 ↗

(On Diff #341774)

Should this take a StreamString so we can write to the stream directory? Or alternatively change the dump method in declaration to also take a raw_ostream? Both would also improve the operator<<.

65–71 ↗

(On Diff #341774)

This should probably return a std::string instead. Now we're going to keep the string around forever for every invocation of this method.

lldb/include/lldb/Utility/SourceLocationSpec.h

+//===-- SourceLocationSpec.h ------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_UTILITY_SOURCELOCATIONSPEC_H
+#define LLDB_UTILITY_SOURCELOCATIONSPEC_H
+
+#include "lldb/Utility/FileSpec.h"
+#include "lldb/lldb-defines.h"
+
+#include "llvm/ADT/Optional.h"
+
+#include <cstddef>
+#include <cstdint>
+#include <string>
+
+namespace lldb_private {
+
+/// \class SourceLocationSpec SourceLocationSpec.h
+/// "lldb/Utility/SourceLocationSpec.h" A source location utility class.
+///
+/// A source location specifier class that holds a FileSpec object with line and
+/// column information. The column line is optional.
+/// It also holds search flags that can be fetched by resolvers to look inlined
+/// declarations and/or exact matches.
+class SourceLocationSpec {
+private:
+  /// Constructor.
+  ///
+  /// Takes a \a file_spec with a \a line number and a \a column number. If
+  /// \a column is null or not provided, it is set to llvm::None.
+  ///
+  /// \param[in] file_spec
+  ///     The full or partial path to a file.
+  ///
+  /// \param[in] line
+  ///     The line number in the source file.
+  ///
+  ///  \param[in] column
+  ///     The column number in the line of the source file.
+  ///
+  ///  \param[in] check_inlines
+  ///     Whether to look for a match in inlined declaration.
+  ///
+  ///  \param[in] exact_match
+  ///     Whether to look for an exact match.
+  ///
+  explicit SourceLocationSpec(FileSpec file_spec, uint32_t line,
+                              llvm::Optional<uint16_t> column,
+                              bool check_inlines, bool exact_match);
+
+  SourceLocationSpec() = delete;
+
+public:
+  static llvm::Expected<SourceLocationSpec>
+  Create(FileSpec file_spec, uint32_t line,
+         llvm::Optional<uint16_t> column = llvm::None,
+         bool check_inlines = false, bool exact_match = false);
+
+  static llvm::Expected<SourceLocationSpec>
+  Create(FileSpec file_spec, uint32_t line,
+         uint16_t column = LLDB_INVALID_COLUMN_NUMBER,
+         bool check_inlines = false, bool exact_match = false);
+
+  /// Equal to operator
+  ///
+  /// Tests if this object is equal to \a rhs.
+  ///
+  /// \param[in] rhs
+  ///     A const SourceLocationSpec object reference to compare this object
+  ///     to.
+  ///
+  /// \return
+  ///     \b true if this object is equal to \a rhs, \b false
+  ///     otherwise.
+  bool operator==(const SourceLocationSpec &rhs) const;
+
+  /// Not equal to operator
+  ///
+  /// Tests if this object is not equal to \a rhs.
+  ///
+  /// \param[in] rhs
+  ///     A const SourceLocationSpec object reference to compare this object
+  ///     to.
+  ///
+  /// \return
+  ///     \b true if this object is equal to \a rhs, \b false
+  ///     otherwise.
+  bool operator!=(const SourceLocationSpec &rhs) const;
+
+  /// Less than to operator
+  ///
+  /// Tests if this object is less than \a rhs.
+  ///
+  /// \param[in] rhs
+  ///     A const SourceLocationSpec object reference to compare this object
+  ///     to.
+  ///
+  /// \return
+  ///     \b true if this object is less than \a rhs, \b false
+  ///     otherwise.
+  bool operator<(const SourceLocationSpec &rhs) const;
+
+  /// 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(...);
+  /// if (location_spec)
+  /// { ...
+  /// \endcode
+  ///
+  /// \return
+  ///     A pointer to this object if both the file_spec and the line are valid,
+  ///     nullptr otherwise.
+  explicit operator bool() const;
+
+  /// Logical NOT operator.
+  ///
+  /// This allows code to check a SourceLocationSpec object to see if it is
+  /// invalid using code such as:
+  ///
+  /// \code
+  /// SourceLocationSpec location_spec(...);
+  /// if (!location_spec)
+  /// { ...
+  /// \endcode
+  ///
+  /// \return
+  ///     Returns \b true if the object has an invalid file_spec or line number,
+  ///     \b false otherwise.
+  bool operator!() const;
+
+  /// Compare two SourceLocationSpec objects.
+  ///
+  /// If \a full is true, then the file_spec, the line and column must match.
+  /// If \a full is false, then only the file_spec and line number for \a lhs
+  /// and \a rhs are compared. This allows a SourceLocationSpec object that have
+  /// no column information to match a  SourceLocationSpec objects that have
+  /// column information with matching file_spec and line component.
+  ///
+  /// \param[in] lhs
+  ///     A const reference to the Left Hand Side object to compare.
+  ///
+  /// \param[in] rhs
+  ///     A const reference to the Right Hand Side object to compare.
+  ///
+  /// \param[in] full
+  ///     If true, then the file_spec, the line and column must match for a
+  ///     compare to return zero (equal to). If false, then only the file_spec
+  ///     and line number for \a lhs and \a rhs are compared, else a full
+  ///     comparison is done.
+  ///
+  /// \return -1 if \a lhs is less than \a rhs, 0 if \a lhs is equal to \a rhs,
+  ///     1 if \a lhs is greater than \a rhs
+  static int Compare(const SourceLocationSpec &lhs,
+                     const SourceLocationSpec &rhs, bool full);
+
+  static bool Equal(const SourceLocationSpec &lhs,
+                    const SourceLocationSpec &rhs, bool full);
+
+  /// Dump this object to a Stream.
+  ///
+  /// Dump the object to the supplied stream \a s, starting with the file name,
+  /// then the line number and if available the column number.
+  ///
+  /// \param[in] s
+  ///     The stream to which to dump the object description.
+  void Dump(llvm::raw_ostream &s) const;
+
+  const char *GetCString() const;
+
+  FileSpec GetFileSpec() const { return m_file_spec; }
+
+  uint32_t GetLine() const { return m_line; }
+
+  bool HasColumn() const { return m_column.hasValue(); }
+
+  llvm::Optional<uint16_t> GetColumn() const { return m_column; }
+
+  bool GetCheckInlines() const { return m_check_inlines; }
+
+  bool GetExactMatch() const { return m_exact_match; }
+
+protected:
+  FileSpec m_file_spec;
+  uint32_t m_line;
+  llvm::Optional<uint16_t> m_column;
+  bool m_check_inlines;
+  bool m_exact_match;
+};
+
+/// Dump a SourceLocationSpec object to a stream
+Stream &operator<<(Stream &s, const SourceLocationSpec &loc);
+} // namespace lldb_private
+
+#endif // LLDB_UTILITY_SOURCELOCATIONSPEC_H

lldb/source/Utility/CMakeLists.txt

add_lldb_library(lldbUtility
   RegisterValue.cpp
   RegularExpression.cpp
   Reproducer.cpp
   ReproducerInstrumentation.cpp
   ReproducerProvider.cpp
   Scalar.cpp
   SelectHelper.cpp
   State.cpp
+  SourceLocationSpec.cpp
   Status.cpp
   Stream.cpp
   StreamCallback.cpp
   StreamString.cpp
   StringExtractor.cpp
   StringExtractorGDBRemote.cpp
   StringLexer.cpp
   StringList.cpp

lldb/source/Utility/SourceLocationSpec.cpp

+//===-- SourceLocationSpec.cpp --------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Utility/SourceLocationSpec.h"
+#include "lldb/Utility/LLDBAssert.h"
+#include "lldb/Utility/Stream.h"
+
+#include "llvm/Support/Errc.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+llvm::Expected<SourceLocationSpec>
+SourceLocationSpec::Create(FileSpec file_spec, uint32_t line,
+                           llvm::Optional<uint16_t> column, bool check_inlines,
+                           bool exact_match) {
+  if (!file_spec)
+    return llvm::createStringError(
+        llvm::errc::invalid_argument,
+        "SourceLocationSpec requires a valid FileSpec.");
+
+  if (!line || line == LLDB_INVALID_LINE_NUMBER)
+    return llvm::createStringError(
+        llvm::errc::invalid_argument,
+        "SourceLocationSpec requires a valid line number.");
+
+  return SourceLocationSpec(file_spec, line, column, check_inlines,
+                            exact_match);
+}
+
+llvm::Expected<SourceLocationSpec>
+SourceLocationSpec::Create(FileSpec file_spec, uint32_t line, uint16_t column,
+                           bool check_inlines, bool exact_match) {
+  llvm::Optional<uint16_t> optional_column = llvm::None;
+  if (column != LLDB_INVALID_COLUMN_NUMBER)
+    optional_column = column;
+
+  return Create(file_spec, line, optional_column, check_inlines, exact_match);
+}
+
+SourceLocationSpec::SourceLocationSpec(FileSpec file_spec, uint32_t line,
+                                       llvm::Optional<uint16_t> column,
+                                       bool check_inlines, bool exact_match)
+    : m_file_spec(file_spec), m_line(line), m_column(column),
+      m_check_inlines(check_inlines), m_exact_match(exact_match) {}
+
+bool SourceLocationSpec::operator==(const SourceLocationSpec &rhs) const {
+  return m_file_spec == rhs.GetFileSpec() && m_line == rhs.GetLine() &&
+         m_column == rhs.GetColumn() &&
+         m_check_inlines == rhs.GetCheckInlines() &&
+         m_exact_match == rhs.GetExactMatch();
+}
+
+bool SourceLocationSpec::operator!=(const SourceLocationSpec &rhs) const {
+  return !(*this == rhs);
+}
+
+bool SourceLocationSpec::operator<(const SourceLocationSpec &rhs) const {
+  return SourceLocationSpec::Compare(*this, rhs, true) < 0;
+}
+
+Stream &lldb_private::operator<<(Stream &s, const SourceLocationSpec &loc) {
+  loc.Dump(s.AsRawOstream());
+  return s;
+}
+
+int SourceLocationSpec::Compare(const SourceLocationSpec &a,
+                                const SourceLocationSpec &b, bool full) {
+
+  if (int result = FileSpec::Compare(a.GetFileSpec(), b.GetFileSpec(), full))
+    return result;
+
+  // If both file_spec matches, compare line numbers.
+  if (a.GetLine() != b.GetLine()) {
+    return (a.GetLine() < b.GetLine()) ? -1 : 1;
+  }
+
+  // If both file_spec and line number match, and \a full comparaison is
+  // enabled, compare column number.
+  if (full && a.GetColumn() != b.GetColumn())
+    return (a.GetColumn() < b.GetColumn()) ? -1 : 1;
+
+  return 0;
+}
+
+bool SourceLocationSpec::Equal(const SourceLocationSpec &lhs,
+                               const SourceLocationSpec &rhs, bool full) {
+  return full ? lhs == rhs
+              : (lhs.GetFileSpec() == rhs.GetFileSpec() &&
+                 lhs.GetLine() == rhs.GetLine());
+}
+
+void SourceLocationSpec::Dump(llvm::raw_ostream &s) const {
+  GetFileSpec().Dump(s);
+  s << ':' << GetLine();
+
+  if (HasColumn())
+    s << ':' << GetColumn();
+}
+
+const char *SourceLocationSpec::GetCString() const {
+  std::string data;
+  llvm::raw_string_ostream ss(data);
+  Dump(ss);
+
+  return ConstString{ss.str()}.AsCString();
+}

lldb/unittests/Utility/CMakeLists.txt

add_lldb_unittest(UtilityTests
   RangeMapTest.cpp
   RangeTest.cpp
   RegisterValueTest.cpp
   RegularExpressionTest.cpp
   ReproducerInstrumentationTest.cpp
   ReproducerTest.cpp
   ScalarTest.cpp
   SharedClusterTest.cpp
+  SourceLocationSpecTest.cpp
   StateTest.cpp
   StatusTest.cpp
   StreamTeeTest.cpp
   StreamTest.cpp
   StringExtractorGDBRemoteTest.cpp
   StringExtractorTest.cpp
   StringLexerTest.cpp
   StringListTest.cpp
   StructuredDataTest.cpp
   SubsystemRAIITest.cpp
   TildeExpressionResolverTest.cpp
   TimeoutTest.cpp
   TimerTest.cpp
+  UUIDTest.cpp
   UriParserTest.cpp
   UserIDResolverTest.cpp
-  UUIDTest.cpp
   VASprintfTest.cpp
   VMRangeTest.cpp
   XcodeSDKTest.cpp
 
   LINK_LIBS
       lldbUtility
       lldbUtilityHelpers
       LLVMTestingSupport
   LINK_COMPONENTS
     Support
   )
 
 add_unittest_inputs(UtilityTests
   StructuredData-basic.json
   )

lldb/unittests/Utility/SourceLocationSpecTest.cpp

+//===-- SourceLocationSpecTest.cpp ----------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "gtest/gtest.h"
+
+#include "lldb/Utility/LLDBAssert.h"
+#include "lldb/Utility/SourceLocationSpec.h"
+
+#include "llvm/Testing/Support/Error.h"
+
+using namespace lldb_private;
+
+TEST(SourceLocationSpecTest, CreateSourceLocationSpec) {
+  FileSpec fs("/foo/bar", FileSpec::Style::posix);
+  const uint32_t line = 19;
+  const uint32_t column = 4;
+  const bool check_inlines = false;
+  const bool exact_match = true;
+
+  // Invalid FileSpec
+  ASSERT_THAT_EXPECTED(
+      SourceLocationSpec::Create(FileSpec(), line, check_inlines, exact_match),
+      llvm::Failed());
+  // Null Line
+  ASSERT_THAT_EXPECTED(
+      SourceLocationSpec::Create(fs, 0, check_inlines, exact_match),
+      llvm::Failed());
+  // Invalid Line
+  ASSERT_THAT_EXPECTED(SourceLocationSpec::Create(fs, LLDB_INVALID_LINE_NUMBER,
+                                                  check_inlines, exact_match),
+                       llvm::Failed());
+  // Invalid FileSpec & Line
+  ASSERT_THAT_EXPECTED(
+      SourceLocationSpec::Create(FileSpec(), 0, check_inlines, exact_match),
+      llvm::Failed());
+
+  // Valid FileSpec & Line
+  ASSERT_THAT_EXPECTED(
+      SourceLocationSpec::Create(fs, line, check_inlines, exact_match),
+      llvm::Succeeded());
+  // Valid FileSpec, Line & Column
+  ASSERT_THAT_EXPECTED(
+      SourceLocationSpec::Create(fs, line, column, check_inlines, exact_match),
+      llvm::Succeeded());
+}
+
+TEST(SourceLocationSpecTest, FileLineColumnComponents) {
+  FileSpec fs("/foo/bar", FileSpec::Style::posix);
+  const uint32_t line = 19;
+  const uint16_t column = 4;
+
+  auto location_spec = SourceLocationSpec::Create(
+      fs, line, LLDB_INVALID_COLUMN_NUMBER, false, true);
+  ASSERT_THAT_EXPECTED(location_spec, llvm::Succeeded());
+  SourceLocationSpec without_column = *location_spec;
+  EXPECT_EQ(fs, without_column.GetFileSpec());
+  EXPECT_EQ(line, without_column.GetLine());
+  EXPECT_FALSE(without_column.HasColumn());
+  EXPECT_EQ(llvm::None, without_column.GetColumn());
+  EXPECT_FALSE(without_column.GetCheckInlines());
+  EXPECT_TRUE(without_column.GetExactMatch());
+  EXPECT_STREQ("/foo/bar:19", without_column.GetCString());
+
+  location_spec = SourceLocationSpec::Create(fs, line, column, true, false);
+  ASSERT_THAT_EXPECTED(location_spec, llvm::Succeeded());
+  SourceLocationSpec with_column = *location_spec;
+  EXPECT_TRUE(with_column.HasColumn());
+  EXPECT_EQ(column, with_column.GetColumn());
+  EXPECT_TRUE(with_column.GetCheckInlines());
+  EXPECT_FALSE(with_column.GetExactMatch());
+  EXPECT_STREQ("/foo/bar:19:4", with_column.GetCString());
+}
+
+TEST(SourceLocationSpecTest, Equal) {
+  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;
+  };
+
+  auto Equal = [](SourceLocationSpec lhs, SourceLocationSpec rhs, bool full) {
+    return SourceLocationSpec::Equal(lhs, rhs, full);
+  };
+
+  const FileSpec fs("/foo/bar", FileSpec::Style::posix);
+  const FileSpec other_fs("/foo/baz", FileSpec::Style::posix);
+
+  // mutating FileSpec + const Inlined, ExactMatch, Line
+  EXPECT_TRUE(
+      Equal(Create(false, false, fs, 4), Create(false, false, fs, 4), true));
+  EXPECT_TRUE(
+      Equal(Create(true, true, fs, 4), Create(true, true, fs, 4), false));
+  EXPECT_FALSE(Equal(Create(false, false, fs, 4),
+                     Create(false, false, other_fs, 4), true));
+  EXPECT_FALSE(
+      Equal(Create(true, true, fs, 4), Create(true, true, other_fs, 4), false));
+
+  // mutating FileSpec + const Inlined, ExactMatch, Line, Column
+  EXPECT_TRUE(Equal(Create(false, false, fs, 4, 19),
+                    Create(false, false, fs, 4, 19), true));
+  EXPECT_TRUE(Equal(Create(true, true, fs, 4, 19),
+                    Create(true, true, fs, 4, 19), false));
+  EXPECT_FALSE(Equal(Create(false, false, fs, 4, 19),
+                     Create(false, false, other_fs, 4, 19), true));
+  EXPECT_FALSE(Equal(Create(true, true, fs, 4, 19),
+                     Create(true, true, other_fs, 4, 19), false));
+
+  // Asymetric match
+  EXPECT_FALSE(
+      Equal(Create(true, true, fs, 4), Create(true, true, fs, 4, 19), true));
+  EXPECT_TRUE(Equal(Create(false, false, fs, 4),
+                    Create(false, false, fs, 4, 19), false));
+
+  // mutating Inlined, ExactMatch
+  EXPECT_FALSE(
+      Equal(Create(true, false, fs, 4), Create(false, true, fs, 4), true));
+  EXPECT_TRUE(
+      Equal(Create(false, true, fs, 4), Create(true, false, fs, 4), false));
+
+  // mutating Column
+  EXPECT_FALSE(Equal(Create(true, true, fs, 4, 96),
+                     Create(true, true, fs, 4, 19), true));
+  EXPECT_TRUE(Equal(Create(false, false, fs, 4, 96),
+                    Create(false, false, fs, 4, 19), false));
+}
+
+TEST(SourceLocationSpecTest, Compare) {
+  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;
+  };
+
+  auto Cmp = [](SourceLocationSpec a, SourceLocationSpec b, bool full) {
+    return SourceLocationSpec::Compare(a, b, full);
+  };
+
+  FileSpec fs("/foo/bar", FileSpec::Style::posix);
+  FileSpec other_fs("/foo/baz", FileSpec::Style::posix);
+
+  // Asymetric comparaison
+  EXPECT_EQ(
+      -1, Cmp(Create(true, true, fs, 4), Create(true, true, fs, 4, 19), true));
+  EXPECT_EQ(0, Cmp(Create(false, false, fs, 4), Create(false, false, fs, 4, 19),
+                   false));
+  EXPECT_EQ(
+      1, Cmp(Create(true, true, fs, 4, 19), Create(true, true, fs, 4), true));
+
+  // mutating FS, const Line
+  EXPECT_EQ(-1, Cmp(Create(false, false, fs, 4),
+                    Create(false, false, other_fs, 4), true));
+  EXPECT_EQ(-1, Cmp(Create(true, true, fs, 4), Create(true, true, other_fs, 4),
+                    false));
+  EXPECT_EQ(1, Cmp(Create(false, true, other_fs, 4), Create(false, true, fs, 4),
+                   true));
+  EXPECT_EQ(1, Cmp(Create(true, false, other_fs, 4), Create(true, false, fs, 4),
+                   false));
+
+  // const FS, mutating Line
+  EXPECT_EQ(
+      -1, Cmp(Create(false, false, fs, 1), Create(false, false, fs, 4), true));
+  EXPECT_EQ(-1,
+            Cmp(Create(true, true, fs, 1), Create(true, true, fs, 4), false));
+  EXPECT_EQ(0,
+            Cmp(Create(false, true, fs, 4), Create(false, true, fs, 4), true));
+  EXPECT_EQ(0,
+            Cmp(Create(true, false, fs, 4), Create(true, false, fs, 4), false));
+  EXPECT_EQ(
+      1, Cmp(Create(false, false, fs, 4), Create(false, false, fs, 1), true));
+  EXPECT_EQ(1,
+            Cmp(Create(true, true, fs, 4), Create(true, true, fs, 1), false));
+
+  // const FS, mutating Line, const Column
+  EXPECT_EQ(-1, Cmp(Create(false, true, fs, 1), Create(false, true, fs, 4, 19),
+                    true));
+  EXPECT_EQ(
+      -1, Cmp(Create(true, true, fs, 1), Create(true, true, fs, 4, 19), false));
+  EXPECT_EQ(
+      1, Cmp(Create(true, false, fs, 4, 19), Create(true, false, fs, 1), true));
+  EXPECT_EQ(1, Cmp(Create(true, false, fs, 4, 19), Create(true, false, fs, 1),
+                   false));
+
+  // mutating FS, const Line, const Column
+  EXPECT_EQ(-1, Cmp(Create(false, false, fs, 4, 19),
+                    Create(false, false, other_fs, 4, 19), true));
+  EXPECT_EQ(-1, Cmp(Create(true, true, fs, 4, 19),
+                    Create(true, true, other_fs, 4, 19), false));
+  EXPECT_EQ(0, Cmp(Create(false, false, fs, 4, 19),
+                   Create(false, false, fs, 4, 19), true));
+  EXPECT_EQ(0, Cmp(Create(true, true, fs, 4, 19), Create(true, true, fs, 4, 19),
+                   false));
+  EXPECT_EQ(1, Cmp(Create(false, true, other_fs, 4, 19),
+                   Create(false, true, fs, 4, 19), true));
+  EXPECT_EQ(1, Cmp(Create(true, false, other_fs, 4, 19),
+                   Create(true, false, fs, 4, 19), false));
+
+  // const FS, const Line, mutating Column
+  EXPECT_EQ(-1, Cmp(Create(false, false, fs, 4, 19),
+                    Create(false, false, fs, 4, 96), true));
+  EXPECT_EQ(0, Cmp(Create(true, true, fs, 4, 96), Create(true, true, fs, 4, 19),
+                   false));
+  EXPECT_EQ(1, Cmp(Create(false, true, fs, 4, 96),
+                   Create(false, true, fs, 4, 19), true));
+}