[Support] Provide access to the full mapping in llvm::Annotations

[li.zhe.hua]

Summary

Providing access to the mapping of annotations allows test helpers to
be expressive by using the annotations as expectations. For example, a
matcher could verify that all annotated points were matched by a
matcher, or that an refactoring surgically modifies specific ranges.

[ymandel]

llvm/include/llvm/Testing/Support/Annotations.h

76

llvm/unittests/Support/AnnotationsTest.cpp

21–24

Use using decls for these?

93

why ElementsAre vs UnorderedElementsAre?

[li.zhe.hua]

Address comments

llvm/unittests/Support/AnnotationsTest.cpp

93

I've added a sentence to the documentation, clarifying the ordering.

[gribozavr2]

llvm/include/llvm/Testing/Support/Annotations.h

77

"The positions are sorted." ?

88

"The ranges are sorted by their start position." ?

[sammccall]

It would be great to understand a bit more about how/where this is used. (It looks like out-of-tree, but I can't find the actual testcases in question).

using the annotations as expectations

This was something deliberately left out of the original design. Names for lookup vs payload metadata on the points are IMO better expressed as separate concerns. The former was much more pressing and we had less experience with the latter so left it out. We're currently coming back to it in D137909, a syntax like $foo(bar) will be used to attach data bar to a point named foo.

For these methods (which I just learned about in reviewing that patch), we could do some combination of:

• leave them exactly as-is (payloads not exposed)
• adapt them mechanically to the data model with payloads
• remove them and then redesign something for the use case using payloads (e.g. if they're not really used and payload is a better fit for the planned usecase)

[li.zhe.hua]

So the two functions I added were because previously using Annotations required knowledge of the annotation names a priori. If you are constructing one and using it directly yourself in a test, great. If you handed the Annotations object to a helper function, you'd also need to hand the two sets of names with it (one for points and one for ranges), otherwise the helper function doesn't know what annotations exist. In our case, the helper function is a googletest matcher verifying matches on AST nodes are bound to IDs matching the annotations listed. However, if no nodes are matched, we can't verify that all annotated nodes were matched without a lot of extra (possibly error-prone) boilerplate.

Ultimately, we only really need the names. Returning the full map was mostly for simplicity (if you want all the names, you probably will query all the values eventually), but really the key (... heh) piece of data we need is the names from the maps.

[sammccall]

Yes, this makes sense. I think exposing just the names is a bit "clever".

If we changed the internal structure to be an array of struct {

StringRef Name;
StringRef Payload;
size_t Start;
size_t End; // -1 for a point

}
Then how do you feel if we expose that array and drop the current maps?

[li.zhe.hua]

Yeah, that seems like it'd work just fine for us.

llvm/include/llvm/Testing/Support/Annotations.h

public:
   llvm::StringRef code() const { return Code; }
 
   /// 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 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 mapping of all names of points marked in the text to their
+  /// position. Unnamed points are mapped to the empty string. Order of points
+  /// for each name matches the order within the text.
+  const 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 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 mapping of all names of ranges marked in the text to their
+  /// location. Unnamed ranges are mapped to the empty string. Order of ranges
+  /// for each name matches the order of start positions within the text.
+  const llvm::StringMap<llvm::SmallVector<Range, 1>> &all_ranges() const;
 
 private:
   std::string Code;
   llvm::StringMap<llvm::SmallVector<size_t, 1>> Points;
   llvm::StringMap<llvm::SmallVector<Range, 1>> Ranges;
 };
 
 llvm::raw_ostream &operator<<(llvm::raw_ostream &O,
                               const llvm::Annotations::Range &R);
 
 } // namespace llvm
 
 #endif

llvm/lib/Testing/Support/Annotations.cpp

 
 std::vector<size_t> Annotations::points(llvm::StringRef Name) const {
   auto I = Points.find(Name);
   if (I == Points.end())
     return {};
   return {I->getValue().begin(), I->getValue().end()};
 }
 
+const llvm::StringMap<llvm::SmallVector<size_t, 1>> &
+Annotations::all_points() const {
+  return Points;
+}
+
 Annotations::Range Annotations::range(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];
 }
 
 std::vector<Annotations::Range>
 Annotations::ranges(llvm::StringRef Name) const {
   auto I = Ranges.find(Name);
   if (I == Ranges.end())
     return {};
   return {I->getValue().begin(), I->getValue().end()};
 }
 
+const llvm::StringMap<llvm::SmallVector<Annotations::Range, 1>> &
+Annotations::all_ranges() const {
+  return Ranges;
+}
+
 llvm::raw_ostream &llvm::operator<<(llvm::raw_ostream &O,
                                     const llvm::Annotations::Range &R) {
   return O << llvm::formatv("[{0}, {1})", R.Begin, R.End);
 }

llvm/unittests/Support/AnnotationsTest.cpp

 //===----- unittests/AnnotationsTest.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 "llvm/Testing/Support/Annotations.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
+using ::testing::AllOf;
 using ::testing::ElementsAre;
 using ::testing::IsEmpty;
+using ::testing::ResultOf;
+using ::testing::UnorderedElementsAre;
 
 namespace {
+MATCHER_P2(pair, first_matcher, second_matcher, "") {
+  return testing::ExplainMatchResult(
+      AllOf(ResultOf([](const auto &entry) { return entry.getKey(); },
+                     first_matcher),
+            ResultOf([](const auto &entry) { return entry.getValue(); },
+                     second_matcher)),
+      arg, result_listener);
+}
+
 llvm::Annotations::Range range(size_t Begin, size_t End) {
   llvm::Annotations::Range R;
   R.Begin = Begin;
   R.End = End;
   return R;
 }
 
 TEST(AnnotationsTest, CleanedCode) {
TEST(AnnotationsTest, Points) {
 
   // No points.
   EXPECT_THAT(llvm::Annotations("ab[[cd]]").points(), IsEmpty());
 
   // Consecutive points.
   EXPECT_THAT(llvm::Annotations("ab^^^cd").points(), ElementsAre(2u, 2u, 2u));
 }
 
+TEST(AnnotationsTest, AllPoints) {
+  // Multiple points.
+  EXPECT_THAT(llvm::Annotations("0$p1^123$p2^456$p1^$p1^78^9").all_points(),
+              UnorderedElementsAre(pair("", ElementsAre(9u)),
+                                   pair("p1", ElementsAre(1u, 7u, 7u)),
+                                   pair("p2", ElementsAre(4u))));
+
+  // No points.
+  EXPECT_THAT(llvm::Annotations("ab[[cd]]").all_points(), IsEmpty());
+}
+
 TEST(AnnotationsTest, Ranges) {
   // A single range.
   EXPECT_EQ(llvm::Annotations("[[a]]bc").range(), range(0, 1));
   EXPECT_EQ(llvm::Annotations("a[[bc]]d").range(), range(1, 3));
   EXPECT_EQ(llvm::Annotations("ab[[cd]]").range(), range(2, 4));
 
   // Empty range.
   EXPECT_EQ(llvm::Annotations("[[]]ab").range(), range(0, 0));
   EXPECT_EQ(llvm::Annotations("a[[]]b").range(), range(1, 1));
   EXPECT_EQ(llvm::Annotations("ab[[]]").range(), range(2, 2));
 
   // Multiple ranges.
   EXPECT_THAT(llvm::Annotations("[[a]][[b]]cd[[ef]]ef").ranges(),
               ElementsAre(range(0, 1), range(1, 2), range(4, 6)));
 
   // No ranges.
   EXPECT_THAT(llvm::Annotations("ab^c^defef").ranges(), IsEmpty());
 }
 
+TEST(AnnotationsTest, AllRanges) {
+  // Multiple ranges.
+  EXPECT_THAT(
+      llvm::Annotations("[[]]01$outer[[2[[[[$inner[[3]]]]]]456]]7$outer[[89]]")
+          .all_ranges(),
+      UnorderedElementsAre(
+          pair("", ElementsAre(range(0, 0), range(3, 4), range(3, 4))),
+          pair("outer", ElementsAre(range(2, 7), range(8, 10))),
+          pair("inner", ElementsAre(range(3, 4)))));
+
+  // No ranges.
+  EXPECT_THAT(llvm::Annotations("ab^c^defef").all_ranges(), IsEmpty());
+}
+
 TEST(AnnotationsTest, Nested) {
   llvm::Annotations Annotated("a[[f^oo^bar[[b[[a]]z]]]]bcdef");
   EXPECT_THAT(Annotated.points(), ElementsAre(2u, 4u));
   EXPECT_THAT(Annotated.ranges(),
               ElementsAre(range(8, 9), range(7, 10), range(1, 10)));
 }
 
 TEST(AnnotationsTest, Named) {