[PATCH] D15506: [ASTMatchers] Allow hasName() to look through inline namespaces

Samuel Benzaquen via cfe-commits cfe-commits at lists.llvm.org
Mon Dec 14 13:56:00 PST 2015


sbenza added a comment.

I am thinking on doing this some other way.
Copying the PrintingPolicy object is very expensive, as it contains quite a few strings and vectors of strings.
In the past I changed this matcher to make no allocations in the common path (ie using SmallString<>, etc) to reduce its cost.
Adding that copy will make it expensive again.

Also, this change is not completely correct. It either uses all namespaces or none of the inline.
It won't partially match.

Eg:

  namespace A {
  inline namespace B {
  inline namespace C {
  void F();
  }
  }
  }

We should be able to match `F` as `A::C::F` and that doesn't work right now.


================
Comment at: lib/ASTMatchers/ASTMatchersInternal.cpp:320
@@ -323,3 +319,3 @@
 
-  if (Pattern.startswith("::"))
-    return FullName == Pattern;
+  for (bool SkipUnwritten : {false, true}) {
+    llvm::SmallString<128> NodeName = StringRef("::");
----------------
aaron.ballman wrote:
> Cute, but this won't work with MSVC 2013. You'll get: error C3312: no callable 'begin' function found for type 'initializer-list'
Thanks. I assumed it worked because we already allow range-for in LLVM.
Extracted the array into a variable.

================
Comment at: lib/ASTMatchers/ASTMatchersInternal.cpp:334
@@ +333,3 @@
+
+    if (Pattern.startswith("::")) {
+      if (FullName == Pattern) return true;
----------------
aaron.ballman wrote:
> Should elide braces and format using clang-format.
Which braces do you want to remove?
I need these to make the else bind with the outer if().

Applied clang-format.


http://reviews.llvm.org/D15506





More information about the cfe-commits mailing list