[AST] Extract Decl::printNestedNameSpecifier helper from Decl::printQualifiedName

[ilya-biryukov]

Summary

To be used in clangd, e.g. in D66647.
Currently the alternative to this function is doing string manipulation on results of printQualifiedName, which is
hard-to-impossible to get right in presence of template arguments.

[aaron.ballman]

clang/include/clang/AST/Decl.h

313 ↗

(On Diff #220998)

This doesn't return anything, so I think a better way to phrase the comment is "Prints only the nested name specifier, including a trailing :: at the end. e.g., if printQualifiedName(D) prints "A::B::i", this function prints "A::B::".`

317–318 ↗

(On Diff #220998)

I'm not keen on "qualifier" here because types have qualifiers. How about printNestedNameSpecifier()?

[ilya-biryukov]

• Update comments
• Rename new function to printNestedNameSpecifier

clang/include/clang/AST/Decl.h

313 ↗

(On Diff #220998)

Good point, thanks!

317–318 ↗

(On Diff #220998)

Now I understand why we clang uses 'nested name specifier' everywhere!
Renamed per your suggestions.
FWIW, I would rather use NameQualifier and TypeQualifier to disambiguate between the two.
NestedNameSpecifier is a bit too long for my taste. But happy to stick with the conventions in the codebase.

[kadircet]

LGTM, I know it is trivial, but some more unittests wouldn't hurt anyone :D

Also could you update the summary to mention "doing string manipulations in the presence of template arguments is hard?"

[ilya-biryukov]

In D67825#1676528, @kadircet wrote:

LGTM, I know it is trivial, but some more unittests wouldn't hurt anyone :D

Yeah, good point, I'll add a few besides tests for printQualifiedName.

Also could you update the summary to mention "doing string manipulations in the presence of template arguments is hard?"

Done.

[aaron.ballman]

LGTM!

clang/include/clang/AST/Decl.h

317–318 ↗

(On Diff #220998)

I think it'd be a pretty large undertaking to hit all of the places where we talk about qualifiers to get to the point where that distinction makes sense. I think we should just stick with nested name specifier, as that's a term of art from the standard for this concept.

[ilya-biryukov]

clang/include/clang/AST/Decl.h

317–318 ↗

(On Diff #220998)

Fully agree, it's too late to change it at this point.

cfe/trunk/include/clang/AST/Decl.h

public:
   /// If the declaration is not a member of context which can be named (record,
   /// namespace), it will return the same result as printName().
   ///
   /// Creating this name is expensive, so it should be called only when
   /// performance doesn't matter.
   void printQualifiedName(raw_ostream &OS) const;
   void printQualifiedName(raw_ostream &OS, const PrintingPolicy &Policy) const;
 
+  /// Print only the nested name specifier part of a fully-qualified name,
+  /// including the '::' at the end. E.g.
+  ///    when `printQualifiedName(D)` prints "A::B::i",
+  ///    this function prints "A::B::".
+  void printNestedNameSpecifier(raw_ostream &OS) const;
+  void printNestedNameSpecifier(raw_ostream &OS,
+                                const PrintingPolicy &Policy) const;
+
   // FIXME: Remove string version.
   std::string getQualifiedNameAsString() const;
 
   /// Appends a human-readable name for this declaration into the given stream.
   ///
   /// This is the method invoked by Sema when displaying a NamedDecl
   /// in a diagnostic.  It does not necessarily produce the same
   /// result as printName(); for example, class template

cfe/trunk/lib/AST/Decl.cpp

 }
 
 void NamedDecl::printQualifiedName(raw_ostream &OS) const {
   printQualifiedName(OS, getASTContext().getPrintingPolicy());
 }
 
 void NamedDecl::printQualifiedName(raw_ostream &OS,
                                    const PrintingPolicy &P) const {
+  printNestedNameSpecifier(OS, P);
+  if (getDeclName() || isa<DecompositionDecl>(this))
+    OS << *this;
+  else
+    OS << "(anonymous)";
+}
+
+void NamedDecl::printNestedNameSpecifier(raw_ostream &OS) const {
+  printNestedNameSpecifier(OS, getASTContext().getPrintingPolicy());
+}
+
+void NamedDecl::printNestedNameSpecifier(raw_ostream &OS,
+                                         const PrintingPolicy &P) const {
   const DeclContext *Ctx = getDeclContext();
 
   // For ObjC methods and properties, look through categories and use the
   // interface as context.
   if (auto *MD = dyn_cast<ObjCMethodDecl>(this))
     if (auto *ID = MD->getClassInterface())
       Ctx = ID;
   if (auto *PD = dyn_cast<ObjCPropertyDecl>(this)) {
     if (auto *MD = PD->getGetterMethodDecl())
       if (auto *ID = MD->getClassInterface())
         Ctx = ID;
   }
 
-  if (Ctx->isFunctionOrMethod()) {
-    printName(OS);
+  if (Ctx->isFunctionOrMethod())
     return;
-  }
 
   using ContextsTy = SmallVector<const DeclContext *, 8>;
   ContextsTy Contexts;
 
   // Collect named contexts.
   while (Ctx) {
     if (isa<NamedDecl>(Ctx))
       Contexts.push_back(Ctx);
if (const auto *Spec = dyn_cast<ClassTemplateSpecializationDecl>(DC)) {
         OS << *ED;
       else
         continue;
     } else {
       OS << *cast<NamedDecl>(DC);
     }
     OS << "::";
   }
-
-  if (getDeclName() || isa<DecompositionDecl>(this))
-    OS << *this;
-  else
-    OS << "(anonymous)";
 }
 
 void NamedDecl::getNameForDiagnostic(raw_ostream &OS,
                                      const PrintingPolicy &Policy,
                                      bool Qualified) const {
   if (Qualified)
     printQualifiedName(OS, Policy);
   else

cfe/trunk/unittests/AST/NamedDeclPrinterTest.cpp

 // These tests have a coding convention:
 // * declaration to be printed is named 'A' unless it should have some special
 // name (e.g., 'operator+');
 // * additional helper declarations are 'Z', 'Y', 'X' and so on.
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/PrettyPrinter.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/SmallString.h"
+#include "llvm/Support/raw_ostream.h"
 #include "gtest/gtest.h"
 
 using namespace clang;
 using namespace ast_matchers;
 using namespace tooling;
 
 namespace {
 
 class PrintMatch : public MatchFinder::MatchCallback {
   SmallString<1024> Printed;
   unsigned NumFoundDecls;
-  bool SuppressUnwrittenScope;
+  std::function<void(llvm::raw_ostream &OS, const NamedDecl *)> Printer;
 
 public:
-  explicit PrintMatch(bool suppressUnwrittenScope)
-    : NumFoundDecls(0), SuppressUnwrittenScope(suppressUnwrittenScope) {}
+  explicit PrintMatch(
+      std::function<void(llvm::raw_ostream &OS, const NamedDecl *)> Printer)
+      : NumFoundDecls(0), Printer(std::move(Printer)) {}
 
   void run(const MatchFinder::MatchResult &Result) override {
     const NamedDecl *ND = Result.Nodes.getNodeAs<NamedDecl>("id");
     if (!ND)
       return;
     NumFoundDecls++;
     if (NumFoundDecls > 1)
       return;
 
     llvm::raw_svector_ostream Out(Printed);
-    PrintingPolicy Policy = Result.Context->getPrintingPolicy();
-    Policy.SuppressUnwrittenScope = SuppressUnwrittenScope;
-    ND->printQualifiedName(Out, Policy);
+    Printer(Out, ND);
   }
 
   StringRef getPrinted() const {
     return Printed;
   }
 
   unsigned getNumFoundDecls() const {
     return NumFoundDecls;
   }
 };
 
-::testing::AssertionResult
-PrintedNamedDeclMatches(StringRef Code, const std::vector<std::string> &Args,
-                        bool SuppressUnwrittenScope,
-                        const DeclarationMatcher &NodeMatch,
-                        StringRef ExpectedPrinted, StringRef FileName) {
-  PrintMatch Printer(SuppressUnwrittenScope);
+::testing::AssertionResult PrintedDeclMatches(
+    StringRef Code, const std::vector<std::string> &Args,
+    const DeclarationMatcher &NodeMatch, StringRef ExpectedPrinted,
+    StringRef FileName,
+    std::function<void(llvm::raw_ostream &, const NamedDecl *)> Print) {
+  PrintMatch Printer(std::move(Print));
   MatchFinder Finder;
   Finder.addMatcher(NodeMatch, &Printer);
   std::unique_ptr<FrontendActionFactory> Factory =
       newFrontendActionFactory(&Finder);
 
   if (!runToolOnCodeWithArgs(Factory->create(), Code, Args, FileName))
     return testing::AssertionFailure()
         << "Parsing error in \"" << Code.str() << "\"";
if (Printer.getPrinted() != ExpectedPrinted)
     return ::testing::AssertionFailure()
         << "Expected \"" << ExpectedPrinted.str() << "\", "
            "got \"" << Printer.getPrinted().str() << "\"";
 
   return ::testing::AssertionSuccess();
 }
 
 ::testing::AssertionResult
+PrintedNamedDeclMatches(StringRef Code, const std::vector<std::string> &Args,
+                        bool SuppressUnwrittenScope,
+                        const DeclarationMatcher &NodeMatch,
+                        StringRef ExpectedPrinted, StringRef FileName) {
+  return PrintedDeclMatches(Code, Args, NodeMatch, ExpectedPrinted, FileName,
+                            [=](llvm::raw_ostream &Out, const NamedDecl *ND) {
+                              auto Policy =
+                                  ND->getASTContext().getPrintingPolicy();
+                              Policy.SuppressUnwrittenScope =
+                                  SuppressUnwrittenScope;
+                              ND->printQualifiedName(Out, Policy);
+                            });
+}
+
+::testing::AssertionResult
 PrintedNamedDeclCXX98Matches(StringRef Code, StringRef DeclName,
                              StringRef ExpectedPrinted) {
   std::vector<std::string> Args(1, "-std=c++98");
   return PrintedNamedDeclMatches(Code,
                                  Args,
                                  /*SuppressUnwrittenScope*/ false,
                                  namedDecl(hasName(DeclName)).bind("id"),
                                  ExpectedPrinted,
PrintedWrittenPropertyDeclObjCMatches(StringRef Code, StringRef DeclName,
   return PrintedNamedDeclMatches(Code,
                                  Args,
                                  /*SuppressUnwrittenScope*/ true,
                                  objcPropertyDecl(hasName(DeclName)).bind("id"),
                                  ExpectedPrinted,
                                  "input.m");
 }
 
+::testing::AssertionResult
+PrintedNestedNameSpecifierMatches(StringRef Code, StringRef DeclName,
+                                  StringRef ExpectedPrinted) {
+  std::vector<std::string> Args{"-std=c++11"};
+  return PrintedDeclMatches(Code, Args, namedDecl(hasName(DeclName)).bind("id"),
+                            ExpectedPrinted, "input.cc",
+                            [](llvm::raw_ostream &Out, const NamedDecl *D) {
+                              D->printNestedNameSpecifier(Out);
+                            });
+}
+
 } // unnamed namespace
 
 TEST(NamedDeclPrinter, TestNamespace1) {
   ASSERT_TRUE(PrintedNamedDeclCXX98Matches(
     "namespace { int A; }",
     "A",
     "(anonymous namespace)::A"));
 }
R"(
   @property(nonatomic, getter=myPropertyGetter) int property;
   @end
 )";
   ASSERT_TRUE(PrintedWrittenPropertyDeclObjCMatches(
     Code,
     "property",
     "Obj::property"));
 }
+
+TEST(NamedDeclPrinter, NestedNameSpecifierSimple) {
+  const char *Code =
+      R"(
+  namespace foo { namespace bar { void func(); }  }
+)";
+  ASSERT_TRUE(PrintedNestedNameSpecifierMatches(Code, "func", "foo::bar::"));
+}
+
+TEST(NamedDeclPrinter, NestedNameSpecifierTemplateArgs) {
+  const char *Code =
+      R"(
+        template <class T> struct vector;
+        template <> struct vector<int> { int method(); };
+)";
+  ASSERT_TRUE(
+      PrintedNestedNameSpecifierMatches(Code, "method", "vector<int>::"));
+}