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

[sbenza]

Summary

Allow hasName() to look through inline namespaces.
This will fix the interaction between some clang-tidy checks and libc++.

libc++ defines names in an inline namespace named std::<version_#>.
When we try to match a name using hasName("std::xxx") it fails to match and the clang-tidy check does not work.

Should fix http://llvm.org/PR25804, http://llvm.org/PR25812, http://llvm.org/PR26133.

[aaron.ballman]

lib/ASTMatchers/ASTMatchersInternal.cpp

320

Cute, but this won't work with MSVC 2013. You'll get: error C3312: no callable 'begin' function found for type 'initializer-list'

334

Should elide braces and format using clang-format.

[yaron.keren]

MSVC 2013 Update 5 accepts for (bool SkipUnwritten : {false, true}).
Possibly changed in one of the Updates?

[sbenza]

Minor fixes

[sbenza]

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.

lib/ASTMatchers/ASTMatchersInternal.cpp

320

Thanks. I assumed it worked because we already allow range-for in LLVM.
Extracted the array into a variable.

334

Which braces do you want to remove?
I need these to make the else bind with the outer if().

Applied clang-format.

[aaron.ballman]

lib/ASTMatchers/ASTMatchersInternal.cpp

320–331

Extracted the array into a variable.

Good solution to be on the safe side. Range-for isn't the issue with MSVC 2013, it's initializer lists (which really didn't get enough love until 2015).

334

Which braces do you want to remove?
I need these to make the else bind with the outer if().

Good point; I think they're fine as-is then.

[yaron.keren]

lib/ASTMatchers/ASTMatchersInternal.cpp

322

StringRef not needed, simply

llvm::SmallString<128> NodeName = "::";

323–331

You may be able to do without the if

PrintingPolicy Policy = Node.getASTContext().getPrintingPolicy();
Policy.SuppressUnwrittenScope = SkipUnwritten;
Node.printQualifiedName(OS, Policy);

[sbenza]

Add a faster version of the qualified name matcher.
It falls back to the previous version if it can't handle the name.

lib/ASTMatchers/ASTMatchersInternal.cpp

322

I tried it before. It doesn't work.

/usr/local/google/llvm_git/llvm/tools/clang/lib/ASTMatchers/ASTMatchersInternal.cpp:321:28: error: no viable conversion from 'const char [3]' to

  'llvm::SmallString<128>'
llvm::SmallString<128> NodeName = "::";

[Eugene.Zelenko]

It'll be great to finalize fix before 3.8 branching (planned at January 13).

[alexfh]

Manuel, can you take a look at this?

[klimek]

Adding Benjamin for an additional pair of eyes.
Generally looks good.

lib/ASTMatchers/ASTMatchersInternal.cpp

335

Perhaps comment that Pattern will be changed to the prefix, so it's clear afterwards.
(I wonder whether returning a tuple might be slightly easier to understand here)

[sbenza]

Rename functions.

lib/ASTMatchers/ASTMatchersInternal.cpp

335

Renamed from Match to Consume. Is that better?

[Prazek]

What is the status of it?

[bkramer]

lg

lib/ASTMatchers/ASTMatchersInternal.cpp

if (Node.getDeclName()) {
     llvm::raw_svector_ostream OS(NodeName);
     Node.printName(OS);
     return Name == OS.str();
   }
   return false;
 }
 
 bool HasNameMatcher::matchesNodeFull(const NamedDecl &Node) const {
+  const StringRef Pattern = Name;
+
+  for (bool SkipUnwritten : {false, true}) {
     llvm::SmallString<128> NodeName = StringRef("::");
     llvm::raw_svector_ostream OS(NodeName);
+
+    if (SkipUnwritten) {
+      PrintingPolicy Policy = Node.getASTContext().getPrintingPolicy();
+      Policy.SuppressUnwrittenScope = true;
+      Node.printQualifiedName(OS, Policy);
+    } else {
       Node.printQualifiedName(OS);
+    }
+
     const StringRef FullName = OS.str();
-  const StringRef Pattern = Name;
 
-  if (Pattern.startswith("::"))
-    return FullName == Pattern;
+    if (Pattern.startswith("::")) {
+      if (FullName == Pattern) return true;
+    } else if (FullName.endswith(Pattern) &&
+               FullName.drop_back(Pattern.size()).endswith("::")) {
+      return true;
+    }
+  }
 
-  return FullName.endswith(Pattern) &&
-         FullName.drop_back(Pattern.size()).endswith("::");
+  return false;
 }
 
 bool HasNameMatcher::matchesNode(const NamedDecl &Node) const {
   // FIXME: There is still room for improvement, but it would require copying a
   // lot of the logic from NamedDecl::printQualifiedName(). The benchmarks do
   // not show like that extra complexity is needed right now.
   if (UseUnqualifiedMatch) {
     assert(matchesNodeUnqualified(Node) == matchesNodeFull(Node));
     return matchesNodeUnqualified(Node);
   }
   return matchesNodeFull(Node);
 }
 
 } // end namespace internal
 } // end namespace ast_matchers
 } // end namespace clang

unittests/ASTMatchers/ASTMatchersTest.cpp

EXPECT_TRUE(
                  recordDecl(hasName("::B::C"))));
   EXPECT_TRUE(notMatches("class A { class B { class C; }; };",
               recordDecl(hasName("z::A::B::C"))));
   EXPECT_TRUE(
       notMatches("class A { class B { class C; }; };",
                  recordDecl(hasName("A+B::C"))));
 }
 
+TEST(Matcher, HasNameSupportsInlinedNamesapces) {
+  std::string code = "namespace a { inline namespace b { class C; } }";
+  EXPECT_TRUE(matches(code, recordDecl(hasName("a::b::C"))));
+  EXPECT_TRUE(matches(code, recordDecl(hasName("a::C"))));
+  EXPECT_TRUE(matches(code, recordDecl(hasName("::a::b::C"))));
+  EXPECT_TRUE(matches(code, recordDecl(hasName("::a::C"))));
+}
+
 TEST(Matcher, IsDefinition) {
   DeclarationMatcher DefinitionOfClassA =
       recordDecl(hasName("A"), isDefinition());
   EXPECT_TRUE(matches("class A {};", DefinitionOfClassA));
   EXPECT_TRUE(notMatches("class A;", DefinitionOfClassA));
 
   DeclarationMatcher DefinitionOfVariableA =
       varDecl(hasName("a"), isDefinition());