[clang-tools-extra] 3cf14a7 - [Support] Add support for attaching payloads to points and ranges

Tom Praschan via cfe-commits cfe-commits at lists.llvm.org
Fri Nov 18 05:00:37 PST 2022


Author: Tom Praschan
Date: 2022-11-18T15:00:23+01:00
New Revision: 3cf14a7bdce08f8fe03d987a2d99c6ea47c58c03

URL: https://github.com/llvm/llvm-project/commit/3cf14a7bdce08f8fe03d987a2d99c6ea47c58c03
DIFF: https://github.com/llvm/llvm-project/commit/3cf14a7bdce08f8fe03d987a2d99c6ea47c58c03.diff

LOG: [Support] Add support for attaching payloads to points and ranges

This is useful where tests previously encoded information in the name
names of ranges and points. Currently, this is pretty limited because
names consist of only alphanumeric characters and '_'.

With this patch, we can keep the names simple and attach optional
payloads to ranges and points instead.

The new syntax should be fully backwards compatible (if I haven't missed
anything). I tested this against clangd unit tests and everything still passes.

Differential Revision: https://reviews.llvm.org/D137909

Added: 
    

Modified: 
    clang-tools-extra/clangd/unittests/Annotations.cpp
    clang-tools-extra/clangd/unittests/Annotations.h
    llvm/include/llvm/Testing/Support/Annotations.h
    llvm/lib/Testing/Support/Annotations.cpp
    llvm/unittests/Support/AnnotationsTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/unittests/Annotations.cpp b/clang-tools-extra/clangd/unittests/Annotations.cpp
index edb0ea9a3af1b..32b916d54d870 100644
--- a/clang-tools-extra/clangd/unittests/Annotations.cpp
+++ b/clang-tools-extra/clangd/unittests/Annotations.cpp
@@ -13,41 +13,78 @@ namespace clang {
 namespace clangd {
 
 Position Annotations::point(llvm::StringRef Name) const {
-  return offsetToPosition(code(), Base::point(Name));
+  return pointWithPayload(Name).first;
+}
+
+std::pair<Position, llvm::StringRef>
+Annotations::pointWithPayload(llvm::StringRef Name) const {
+  auto [BasePoint, Payload] = Base::pointWithPayload(Name);
+  return {offsetToPosition(code(), BasePoint), Payload};
 }
 
 std::vector<Position> Annotations::points(llvm::StringRef Name) const {
-  auto Offsets = Base::points(Name);
+  auto BasePoints = Base::points(Name);
 
   std::vector<Position> Ps;
-  Ps.reserve(Offsets.size());
-  for (size_t O : Offsets)
-    Ps.push_back(offsetToPosition(code(), O));
+  Ps.reserve(BasePoints.size());
+  for (const auto Point : BasePoints)
+    Ps.push_back(offsetToPosition(code(), Point));
+
+  return Ps;
+}
+
+std::vector<std::pair<Position, llvm::StringRef>>
+Annotations::pointsWithPayload(llvm::StringRef Name) const {
+  auto BasePoints = Base::pointsWithPayload(Name);
+
+  std::vector<std::pair<Position, llvm::StringRef>> Ps;
+  Ps.reserve(BasePoints.size());
+  for (const auto &[Point, Payload] : BasePoints)
+    Ps.push_back({offsetToPosition(code(), Point), Payload});
 
   return Ps;
 }
 
-static clangd::Range toLSPRange(llvm::StringRef Code, Annotations::Range R) {
+static clangd::Range toLSPRange(llvm::StringRef Code,
+                                llvm::Annotations::Range R) {
   clangd::Range LSPRange;
   LSPRange.start = offsetToPosition(Code, R.Begin);
   LSPRange.end = offsetToPosition(Code, R.End);
   return LSPRange;
 }
 
-clangd::Range Annotations::range(llvm::StringRef Name) const {
-  return toLSPRange(code(), Base::range(Name));
+Range Annotations::range(llvm::StringRef Name) const {
+  return rangeWithPayload(Name).first;
 }
 
-std::vector<clangd::Range> Annotations::ranges(llvm::StringRef Name) const {
+std::pair<clangd::Range, llvm::StringRef>
+Annotations::rangeWithPayload(llvm::StringRef Name) const {
+  auto [BaseRange, Payload] = Base::rangeWithPayload(Name);
+  return {toLSPRange(code(), BaseRange), Payload};
+}
+
+std::vector<Range> Annotations::ranges(llvm::StringRef Name) const {
   auto OffsetRanges = Base::ranges(Name);
 
   std::vector<clangd::Range> Rs;
   Rs.reserve(OffsetRanges.size());
-  for (Annotations::Range R : OffsetRanges)
+  for (const auto &R : OffsetRanges)
     Rs.push_back(toLSPRange(code(), R));
 
   return Rs;
 }
 
+std::vector<std::pair<clangd::Range, llvm::StringRef>>
+Annotations::rangesWithPayload(llvm::StringRef Name) const {
+  auto OffsetRanges = Base::rangesWithPayload(Name);
+
+  std::vector<std::pair<clangd::Range, llvm::StringRef>> Rs;
+  Rs.reserve(OffsetRanges.size());
+  for (const auto &[R, Payload] : OffsetRanges)
+    Rs.push_back({toLSPRange(code(), R), Payload});
+
+  return Rs;
+}
+
 } // namespace clangd
 } // namespace clang

diff  --git a/clang-tools-extra/clangd/unittests/Annotations.h b/clang-tools-extra/clangd/unittests/Annotations.h
index 9c296810e44dd..60a2d62dda9c3 100644
--- a/clang-tools-extra/clangd/unittests/Annotations.h
+++ b/clang-tools-extra/clangd/unittests/Annotations.h
@@ -27,10 +27,18 @@ class Annotations : public llvm::Annotations {
   using llvm::Annotations::Annotations;
 
   Position point(llvm::StringRef Name = "") const;
+  std::pair<Position, llvm::StringRef>
+  pointWithPayload(llvm::StringRef Name = "") const;
   std::vector<Position> points(llvm::StringRef Name = "") const;
+  std::vector<std::pair<Position, llvm::StringRef>>
+  pointsWithPayload(llvm::StringRef Name = "") const;
 
   clangd::Range range(llvm::StringRef Name = "") const;
+  std::pair<clangd::Range, llvm::StringRef>
+  rangeWithPayload(llvm::StringRef Name = "") const;
   std::vector<clangd::Range> ranges(llvm::StringRef Name = "") const;
+  std::vector<std::pair<clangd::Range, llvm::StringRef>>
+  rangesWithPayload(llvm::StringRef Name = "") const;
 };
 
 } // namespace clangd

diff  --git a/llvm/include/llvm/Testing/Support/Annotations.h b/llvm/include/llvm/Testing/Support/Annotations.h
index fc9e195a4ad4b..81da9f39d26be 100644
--- a/llvm/include/llvm/Testing/Support/Annotations.h
+++ b/llvm/include/llvm/Testing/Support/Annotations.h
@@ -8,6 +8,7 @@
 #ifndef LLVM_TESTING_SUPPORT_ANNOTATIONS_H
 #define LLVM_TESTING_SUPPORT_ANNOTATIONS_H
 
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
@@ -24,7 +25,9 @@ class raw_ostream;
 ///       int complete() { x.pri^ }         // ^ indicates a point
 ///       void err() { [["hello" == 42]]; } // [[this is a range]]
 ///       $definition^class Foo{};          // points can be named: "definition"
-///       $fail[[static_assert(false, "")]] // ranges can be named too: "fail"
+///       $(foo)^class Foo{};               // ...or have a payload: "foo"
+///       $definition(foo)^class Foo{};     // ...or both
+///       $fail(runtime)[[assert(false)]]   // ranges can have names/payloads too
 ///    )cpp");
 ///
 ///    StringRef Code = Example.code();             // annotations stripped.
@@ -35,6 +38,9 @@ class raw_ostream;
 /// Points/ranges are coordinated into `code()` which is stripped of
 /// annotations.
 ///
+/// Names consist of only alphanumeric characters or '_'.
+/// Payloads can contain any character expect '(' and ')'.
+///
 /// Ranges may be nested (and points can be inside ranges), but there's no way
 /// to define general overlapping ranges.
 ///
@@ -69,29 +75,55 @@ class Annotations {
   /// Returns the position of the point marked by ^ (or $name^) in the text.
   /// Crashes if there isn't exactly one.
   size_t point(llvm::StringRef Name = "") const;
+  /// Returns the position of the point with \p Name and its payload (if any).
+  std::pair<size_t, llvm::StringRef>
+  pointWithPayload(llvm::StringRef Name = "") const;
   /// Returns the position of all points marked by ^ (or $name^) in the text.
   /// Order matches the order within the text.
   std::vector<size_t> points(llvm::StringRef Name = "") const;
+  /// Returns the positions and payloads (if any) of all points named \p Name
+  std::vector<std::pair<size_t, llvm::StringRef>>
+  pointsWithPayload(llvm::StringRef Name = "") const;
   /// Returns the mapping of all names of points marked in the text to their
   /// position. Unnamed points are mapped to the empty string. The positions are
   /// sorted.
-  const llvm::StringMap<llvm::SmallVector<size_t, 1>> &all_points() const;
+  /// FIXME Remove this and expose `All` directly (currently used out-of-tree)
+  llvm::StringMap<llvm::SmallVector<size_t, 1>> all_points() const;
 
   /// Returns the location of the range marked by [[ ]] (or $name[[ ]]).
   /// Crashes if there isn't exactly one.
   Range range(llvm::StringRef Name = "") const;
+  /// Returns the location and payload of the range marked by [[ ]]
+  /// (or $name(payload)[[ ]]). Crashes if there isn't exactly one.
+  std::pair<Range, llvm::StringRef>
+  rangeWithPayload(llvm::StringRef Name = "") const;
   /// Returns the location of all ranges marked by [[ ]] (or $name[[ ]]).
   /// They are ordered by start position within the text.
   std::vector<Range> ranges(llvm::StringRef Name = "") const;
+  /// Returns the location of all ranges marked by [[ ]]
+  /// (or $name(payload)[[ ]]).
+  /// They are ordered by start position within the text.
+  std::vector<std::pair<Range, llvm::StringRef>>
+  rangesWithPayload(llvm::StringRef Name = "") const;
   /// Returns the mapping of all names of ranges marked in the text to their
   /// location. Unnamed ranges are mapped to the empty string. The ranges are
   /// sorted by their start position.
-  const llvm::StringMap<llvm::SmallVector<Range, 1>> &all_ranges() const;
+  llvm::StringMap<llvm::SmallVector<Range, 1>> all_ranges() const;
 
 private:
   std::string Code;
+  /// Either a Point (Only Start) or a Range (Start and End)
+  struct Annotation {
+    size_t Begin;
+    size_t End = -1;
+    bool isPoint() const { return End == size_t(-1); }
+    llvm::StringRef Name;
+    llvm::StringRef Payload;
+  };
+  std::vector<Annotation> All;
+  // Values are the indices into All
   llvm::StringMap<llvm::SmallVector<size_t, 1>> Points;
-  llvm::StringMap<llvm::SmallVector<Range, 1>> Ranges;
+  llvm::StringMap<llvm::SmallVector<size_t, 1>> Ranges;
 };
 
 llvm::raw_ostream &operator<<(llvm::raw_ostream &O,

diff  --git a/llvm/lib/Testing/Support/Annotations.cpp b/llvm/lib/Testing/Support/Annotations.cpp
index 388215dda1813..fdb1bf94b3bff 100644
--- a/llvm/lib/Testing/Support/Annotations.cpp
+++ b/llvm/lib/Testing/Support/Annotations.cpp
@@ -28,27 +28,35 @@ Annotations::Annotations(llvm::StringRef Text) {
     require(Assertion, Msg, Text);
   };
   llvm::Optional<llvm::StringRef> Name;
-  llvm::SmallVector<std::pair<llvm::StringRef, size_t>, 8> OpenRanges;
+  llvm::Optional<llvm::StringRef> Payload;
+  llvm::SmallVector<Annotation, 8> OpenRanges;
 
   Code.reserve(Text.size());
   while (!Text.empty()) {
     if (Text.consume_front("^")) {
-      Points[Name.value_or("")].push_back(Code.size());
+      All.push_back(
+          {Code.size(), size_t(-1), Name.value_or(""), Payload.value_or("")});
+      Points[Name.value_or("")].push_back(All.size() - 1);
       Name = llvm::None;
+      Payload = llvm::None;
       continue;
     }
     if (Text.consume_front("[[")) {
-      OpenRanges.emplace_back(Name.value_or(""), Code.size());
+      OpenRanges.push_back(
+          {Code.size(), size_t(-1), Name.value_or(""), Payload.value_or("")});
       Name = llvm::None;
+      Payload = llvm::None;
       continue;
     }
     Require(!Name, "$name should be followed by ^ or [[");
     if (Text.consume_front("]]")) {
       Require(!OpenRanges.empty(), "unmatched ]]");
-      Range R;
-      R.Begin = OpenRanges.back().second;
-      R.End = Code.size();
-      Ranges[OpenRanges.back().first].push_back(R);
+
+      const Annotation &NewRange = OpenRanges.back();
+      All.push_back(
+          {NewRange.Begin, Code.size(), NewRange.Name, NewRange.Payload});
+      Ranges[NewRange.Name].push_back(All.size() - 1);
+
       OpenRanges.pop_back();
       continue;
     }
@@ -56,6 +64,13 @@ Annotations::Annotations(llvm::StringRef Text) {
       Name =
           Text.take_while([](char C) { return llvm::isAlnum(C) || C == '_'; });
       Text = Text.drop_front(Name->size());
+
+      if (Text.consume_front("(")) {
+        Payload = Text.take_while([](char C) { return C != ')'; });
+        Require(Text.size() > Payload->size(), "unterminated payload");
+        Text = Text.drop_front(Payload->size() + 1);
+      }
+
       continue;
     }
     Code.push_back(Text.front());
@@ -66,42 +81,95 @@ Annotations::Annotations(llvm::StringRef Text) {
 }
 
 size_t Annotations::point(llvm::StringRef Name) const {
+  return pointWithPayload(Name).first;
+}
+
+std::pair<size_t, llvm::StringRef>
+Annotations::pointWithPayload(llvm::StringRef Name) const {
   auto I = Points.find(Name);
   require(I != Points.end() && I->getValue().size() == 1,
           "expected exactly one point", Code);
-  return I->getValue()[0];
+  const Annotation &P = All[I->getValue()[0]];
+  return {P.Begin, P.Payload};
 }
 
 std::vector<size_t> Annotations::points(llvm::StringRef Name) const {
-  auto I = Points.find(Name);
-  if (I == Points.end())
+  auto Pts = pointsWithPayload(Name);
+  std::vector<size_t> Positions;
+  Positions.reserve(Pts.size());
+  for (const auto &[Point, Payload] : Pts)
+    Positions.push_back(Point);
+  return Positions;
+}
+
+std::vector<std::pair<size_t, llvm::StringRef>>
+Annotations::pointsWithPayload(llvm::StringRef Name) const {
+  auto Iter = Points.find(Name);
+  if (Iter == Points.end())
     return {};
-  return {I->getValue().begin(), I->getValue().end()};
+
+  std::vector<std::pair<size_t, llvm::StringRef>> Res;
+  Res.reserve(Iter->getValue().size());
+  for (size_t I : Iter->getValue())
+    Res.push_back({All[I].Begin, All[I].Payload});
+
+  return Res;
 }
 
-const llvm::StringMap<llvm::SmallVector<size_t, 1>> &
-Annotations::all_points() const {
-  return Points;
+llvm::StringMap<llvm::SmallVector<size_t, 1>> Annotations::all_points() const {
+  llvm::StringMap<llvm::SmallVector<size_t, 1>> Result;
+  for (const auto &Name : Points.keys()) {
+    auto Pts = points(Name);
+    Result[Name] = {Pts.begin(), Pts.end()};
+  }
+  return Result;
 }
 
 Annotations::Range Annotations::range(llvm::StringRef Name) const {
+  return rangeWithPayload(Name).first;
+}
+
+std::pair<Annotations::Range, llvm::StringRef>
+Annotations::rangeWithPayload(llvm::StringRef Name) const {
   auto I = Ranges.find(Name);
   require(I != Ranges.end() && I->getValue().size() == 1,
           "expected exactly one range", Code);
-  return I->getValue()[0];
+  const Annotation &R = All[I->getValue()[0]];
+  return {{R.Begin, R.End}, R.Payload};
 }
 
 std::vector<Annotations::Range>
 Annotations::ranges(llvm::StringRef Name) const {
-  auto I = Ranges.find(Name);
-  if (I == Ranges.end())
+  auto WithPayload = rangesWithPayload(Name);
+  std::vector<Annotations::Range> Res;
+  Res.reserve(WithPayload.size());
+  for (const auto &[Range, Payload] : WithPayload)
+    Res.push_back(Range);
+  return Res;
+}
+std::vector<std::pair<Annotations::Range, llvm::StringRef>>
+Annotations::rangesWithPayload(llvm::StringRef Name) const {
+  auto Iter = Ranges.find(Name);
+  if (Iter == Ranges.end())
     return {};
-  return {I->getValue().begin(), I->getValue().end()};
+
+  std::vector<std::pair<Annotations::Range, llvm::StringRef>> Res;
+  Res.reserve(Iter->getValue().size());
+  for (size_t I : Iter->getValue())
+    Res.emplace_back(Annotations::Range{All[I].Begin, All[I].End},
+                     All[I].Payload);
+
+  return Res;
 }
 
-const llvm::StringMap<llvm::SmallVector<Annotations::Range, 1>> &
+llvm::StringMap<llvm::SmallVector<Annotations::Range, 1>>
 Annotations::all_ranges() const {
-  return Ranges;
+  llvm::StringMap<llvm::SmallVector<Annotations::Range, 1>> Res;
+  for (const llvm::StringRef &Name : Ranges.keys()) {
+    auto R = ranges(Name);
+    Res[Name] = {R.begin(), R.end()};
+  }
+  return Res;
 }
 
 llvm::raw_ostream &llvm::operator<<(llvm::raw_ostream &O,

diff  --git a/llvm/unittests/Support/AnnotationsTest.cpp b/llvm/unittests/Support/AnnotationsTest.cpp
index c7bbb79db800c..96b564d82b5cb 100644
--- a/llvm/unittests/Support/AnnotationsTest.cpp
+++ b/llvm/unittests/Support/AnnotationsTest.cpp
@@ -12,6 +12,7 @@
 using ::testing::AllOf;
 using ::testing::ElementsAre;
 using ::testing::IsEmpty;
+using ::testing::Pair;
 using ::testing::ResultOf;
 using ::testing::UnorderedElementsAre;
 
@@ -105,6 +106,38 @@ TEST(AnnotationsTest, Nested) {
               ElementsAre(range(8, 9), range(7, 10), range(1, 10)));
 }
 
+TEST(AnnotationsTest, Payload) {
+  // // A single unnamed point or range with unspecified payload
+  EXPECT_THAT(llvm::Annotations("a$^b").pointWithPayload(), Pair(1u, ""));
+  EXPECT_THAT(llvm::Annotations("a$[[b]]cdef").rangeWithPayload(),
+              Pair(range(1, 2), ""));
+
+  // A single unnamed point or range with empty payload
+  EXPECT_THAT(llvm::Annotations("a$()^b").pointWithPayload(), Pair(1u, ""));
+  EXPECT_THAT(llvm::Annotations("a$()[[b]]cdef").rangeWithPayload(),
+              Pair(range(1, 2), ""));
+
+  // A single unnamed point or range with payload.
+  EXPECT_THAT(llvm::Annotations("a$(foo)^b").pointWithPayload(),
+              Pair(1u, "foo"));
+  EXPECT_THAT(llvm::Annotations("a$(foo)[[b]]cdef").rangeWithPayload(),
+              Pair(range(1, 2), "foo"));
+
+  // A single named point or range with payload
+  EXPECT_THAT(llvm::Annotations("a$name(foo)^b").pointWithPayload("name"),
+              Pair(1u, "foo"));
+  EXPECT_THAT(
+      llvm::Annotations("a$name(foo)[[b]]cdef").rangeWithPayload("name"),
+      Pair(range(1, 2), "foo"));
+
+  // Multiple named points with payload.
+  llvm::Annotations Annotated("a$p1(p1)^bcd$p2(p2)^123$p1^345");
+  EXPECT_THAT(Annotated.points(), IsEmpty());
+  EXPECT_THAT(Annotated.pointsWithPayload("p1"),
+              ElementsAre(Pair(1u, "p1"), Pair(7u, "")));
+  EXPECT_THAT(Annotated.pointWithPayload("p2"), Pair(4u, "p2"));
+}
+
 TEST(AnnotationsTest, Named) {
   // A single named point or range.
   EXPECT_EQ(llvm::Annotations("a$foo^b").point("foo"), 1u);
@@ -144,6 +177,8 @@ TEST(AnnotationsTest, Errors) {
   EXPECT_DEATH(llvm::Annotations("ff[[fdfd"), "unmatched \\[\\[");
   EXPECT_DEATH(llvm::Annotations("ff[[fdjsfjd]]xxx]]"), "unmatched \\]\\]");
   EXPECT_DEATH(llvm::Annotations("ff$fdsfd"), "unterminated \\$name");
+  EXPECT_DEATH(llvm::Annotations("ff$("), "unterminated payload");
+  EXPECT_DEATH(llvm::Annotations("ff$name("), "unterminated payload");
 #endif
 }
 } // namespace


        


More information about the cfe-commits mailing list