[PATCH] D100638: [AST][Introspection] Avoid creating temporary strings when comparing LocationCalls.

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added a reviewer: steveire.
njames93 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Adds a bit of boiler plate, but removing the need to build temporary strings to compare the calls definitely worth it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D100638

Files:
  clang/lib/Tooling/NodeIntrospection.cpp


Index: clang/lib/Tooling/NodeIntrospection.cpp
===================================================================
--- clang/lib/Tooling/NodeIntrospection.cpp
+++ clang/lib/Tooling/NodeIntrospection.cpp
@@ -13,6 +13,9 @@
 #include "clang/Tooling/NodeIntrospection.h"
 
 #include "clang/AST/AST.h"
+#include "llvm/Support/Compiler.h"
+#include "llvm/Support/Debug.h"
+#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
 
 namespace clang {
@@ -49,6 +52,56 @@
   return Result;
 }
 
+/// lexicographical 3 way comparison.
+static int compareStringArray(llvm::ArrayRef<std::string> LHS,
+                              llvm::ArrayRef<std::string> RHS) {
+  // Most accessors don't accept any arguments
+  if (LLVM_LIKELY(LHS.empty() && RHS.empty()))
+    return 0;
+  auto LI = LHS.begin(), LE = LHS.end(), RI = RHS.begin(), RE = RHS.end();
+  for (; LI != LE && RI != RE; ++LI, ++RI) {
+    if (auto Cmp = LI->compare(*RI))
+      return Cmp;
+  }
+  if (LI == LE && RI == RE)
+    return 0;
+  // Return -1 if Left contains less items, 1 if more items.
+  return LI == LE ? -1 : 1;
+}
+
+/// A 3 way comparison of what LHS and RHS would look like when formatted as a
+/// cpp string.
+static int compareLocationCall(const LocationCall &LHS,
+                               const LocationCall &RHS) {
+  // Expand the call stacks into a vector so we can traverse it in reverse
+  // order.
+  llvm::SmallVector<const LocationCall *> Left, Right;
+  for (const LocationCall *Item = &LHS; Item; Item = Item->on())
+    Left.push_back(Item);
+  for (const LocationCall *Item = &RHS; Item; Item = Item->on())
+    Right.push_back(Item);
+
+  auto LI = Left.rbegin(), LE = Left.rend(), RI = Right.rbegin();
+  for (; LI != LE; ++LI, ++RI) {
+    // If we haven't returned up to here, both call stacks are the same up the
+    // here are the same. If we detect that RI is at the end, that means the
+    // rights last item was a SourceLocation/Range accessor. Therefore the
+    // preceding item in left should also return a Loc/Range and it should be
+    // the last item in its stack.
+    assert(RI != Right.rend());
+    if (auto Strcmp = (*LI)->name().compare((*RI)->name()))
+      return Strcmp;
+    if (auto ArgCmp = compareStringArray((*LI)->args(), (*RI)->args()))
+      return ArgCmp;
+    // As both calls are the same, they should both either return by value or
+    // both return by pointer.
+    assert((*LI)->returnsPointer() == (*RI)->returnsPointer());
+  }
+  // As before but flipped.
+  assert(RI == Right.rend());
+  return 0;
+}
+
 namespace internal {
 bool RangeLessThan::operator()(
     std::pair<SourceRange, SharedLocationCall> const &LHS,
@@ -66,15 +119,13 @@
   else if (LHS.first.getEnd() != RHS.first.getEnd())
     return false;
 
-  return LocationCallFormatterCpp::format(*LHS.second) <
-         LocationCallFormatterCpp::format(*RHS.second);
+  return compareLocationCall(*LHS.second, *RHS.second) < 0;
 }
 bool RangeLessThan::operator()(
     std::pair<SourceLocation, SharedLocationCall> const &LHS,
     std::pair<SourceLocation, SharedLocationCall> const &RHS) const {
   if (LHS.first == RHS.first)
-    return LocationCallFormatterCpp::format(*LHS.second) <
-           LocationCallFormatterCpp::format(*RHS.second);
+    return compareLocationCall(*LHS.second, *RHS.second) < 0;
   return LHS.first < RHS.first;
 }
 } // namespace internal


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D100638.338056.patch
Type: text/x-patch
Size: 3411 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210416/dfa7b34d/attachment.bin>