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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to