[DeclPrinter] Honor TerseOutput for constructors

[nik]

Summary

...by not printing the body (as for functions) and not printing the
member initialization list.

[nik]

• [DeclPrinter] Allow printing fully qualified name of function declaration

[nik]

Ops, restored to previous state.

[nik]

Try to restore, II.

[nik]

Sorry for the noise, I'm just doing my first steps with "arc diff".

I've managed to restore this - please review or add more appropriate reviewers.

[arphaman]

LGTM

lib/AST/DeclPrinter.cpp

618

NIT: This function is getting a bit unwieldy. You might want to extract the initializer printing code into a new function in a pre/post commit.

[nik]

Thanks for the fast review! :)

Please land/submit this for me as I don't have permissions for this.

lib/AST/DeclPrinter.cpp

618

Will do.

[nik]

lib/AST/DeclPrinter.cpp

618

https://reviews.llvm.org/D40066

[arphaman]

Thanks. Sure, I will commit the patches today.

[arphaman]

FYI, please remember to CC cfe-commits for your future patches.

[nik]

Thanks Alex!

FYI, please remember to CC cfe-commits for your future patches.

Oh, OK. I guess I somehow assumed arc diff would took care of that.

include/clang/AST/PrettyPrinter.h

: Indentation(2), SuppressSpecifiers(false),
       SuppressStrongLifetime(false), SuppressLifetimeQualifiers(false),
       SuppressTemplateArgsInCXXConstructors(false),
       Bool(LO.Bool), Restrict(LO.C99),
       Alignof(LO.CPlusPlus11), UnderscoreAlignof(LO.C11),
       UseVoidForZeroParams(!LO.CPlusPlus),
       TerseOutput(false), PolishForDeclaration(false),
       Half(LO.Half), MSWChar(LO.MicrosoftExt && !LO.WChar),
       IncludeNewlines(true), MSVCFormatting(false),
-      ConstantsAsWritten(false), SuppressImplicitBase(false) { }
+      ConstantsAsWritten(false), SuppressImplicitBase(false),
+      FullyQualifiedName(false) { }
 
   /// Adjust this printing policy for cases where it's known that we're
   /// printing C++ code (for instance, if AST dumping reaches a C++-only
   /// construct). This should not be used if a real LangOptions object is
   /// available.
   void adjustForCPlusPlus() {
     SuppressTagKeyword = true;
     Bool = true;
struct PrintingPolicy {
   /// \code
   /// 0x10
   /// 2.5e3
   /// \endcode
   bool ConstantsAsWritten : 1;
 
   /// When true, don't print the implicit 'self' or 'this' expressions.
   bool SuppressImplicitBase : 1;
+
+  /// When true, print the fully qualified name of function declarations.
+  /// This is the opposite of SuppressScope and thus overrules it.
+  bool FullyQualifiedName : 1;
 };
 
 } // end namespace clang
 
 #endif

lib/AST/DeclPrinter.cpp

if ((CDecl && CDecl->isExplicitSpecified()) ||
         (ConversionDecl && ConversionDecl->isExplicitSpecified()) ||
         (GuideDecl && GuideDecl->isExplicitSpecified()))
       Out << "explicit ";
   }
 
   PrintingPolicy SubPolicy(Policy);
   SubPolicy.SuppressSpecifiers = false;
   std::string Proto;
+
+  if (Policy.FullyQualifiedName) {
+    Proto += D->getQualifiedNameAsString();
+  } else {
     if (!Policy.SuppressScope) {
       if (const NestedNameSpecifier *NS = D->getQualifier()) {
         llvm::raw_string_ostream OS(Proto);
         NS->print(OS, Policy);
       }
     }
     Proto += D->getNameInfo().getAsString();
+  }
+
   if (GuideDecl)
     Proto = GuideDecl->getDeducedTemplate()->getDeclName().getAsString();
   if (const TemplateArgumentList *TArgs = D->getTemplateSpecializationArgs()) {
     llvm::raw_string_ostream POut(Proto);
     DeclPrinter TArgPrinter(POut, SubPolicy, Context, Indentation);
     TArgPrinter.printTemplateArguments(*TArgs);
   }
 
if (FT && FT->hasDynamicExceptionSpec()) {
                                            Indentation);
         EOut.flush();
         Proto += EOut.str();
         Proto += ")";
       }
     }
 
     if (CDecl) {
+      if (!Policy.TerseOutput) {
         bool HasInitializerList = false;
         for (const auto *BMInitializer : CDecl->inits()) {
           if (BMInitializer->isInClassMemberInitializer())
             continue;
 
           if (!HasInitializerList) {
             Proto += " : ";
             Out << Proto;
             Proto.clear();
             HasInitializerList = true;
           } else
             Out << ", ";
 
           if (BMInitializer->isAnyMemberInitializer()) {
             FieldDecl *FD = BMInitializer->getAnyMember();
             Out << *FD;
           } else {
-          Out << QualType(BMInitializer->getBaseClass(), 0).getAsString(Policy);
+            Out << QualType(BMInitializer->getBaseClass(), 0)
+                       .getAsString(Policy);
           }
-        
           Out << "(";
           if (!BMInitializer->getInit()) {
             // Nothing to print
           } else {
             Expr *Init = BMInitializer->getInit();
             if (ExprWithCleanups *Tmp = dyn_cast<ExprWithCleanups>(Init))
               Init = Tmp->getSubExpr();
-          
             Init = Init->IgnoreParens();
 
             Expr *SimpleInit = nullptr;
             Expr **Args = nullptr;
             unsigned NumArgs = 0;
             if (ParenListExpr *ParenList = dyn_cast<ParenListExpr>(Init)) {
               Args = ParenList->getExprs();
               NumArgs = ParenList->getNumExprs();
-          } else if (CXXConstructExpr *Construct
-                                        = dyn_cast<CXXConstructExpr>(Init)) {
+            } else if (CXXConstructExpr *Construct =
+                           dyn_cast<CXXConstructExpr>(Init)) {
               Args = Construct->getArgs();
               NumArgs = Construct->getNumArgs();
             } else
               SimpleInit = Init;
-          
             if (SimpleInit)
               SimpleInit->printPretty(Out, nullptr, Policy, Indentation);
             else {
               for (unsigned I = 0; I != NumArgs; ++I) {
                 assert(Args[I] != nullptr && "Expected non-null Expr");
                 if (isa<CXXDefaultArgExpr>(Args[I]))
                   break;
-              
                 if (I)
                   Out << ", ";
                 Args[I]->printPretty(Out, nullptr, Policy, Indentation);
               }
             }
           }
           Out << ")";
           if (BMInitializer->isPackExpansion())
             Out << "...";
         }
+      }
     } else if (!ConversionDecl && !isa<CXXDestructorDecl>(D)) {
       if (FT && FT->hasTrailingReturn()) {
         if (!GuideDecl)
           Out << "auto ";
         Out << Proto << " -> ";
         Proto.clear();
       }
       AFT->getReturnType().print(Out, Policy, Proto);
if (!Policy.TerseOutput) {
         }
         Indentation -= Policy.Indentation;
       } else
         Out << ' ';
 
       if (D->getBody())
         D->getBody()->printPretty(Out, nullptr, SubPolicy, Indentation);
     } else {
-      if (isa<CXXConstructorDecl>(*D))
+      if (!Policy.TerseOutput && isa<CXXConstructorDecl>(*D))
         Out << " {}";
     }
   }
 }
 
 void DeclPrinter::VisitFriendDecl(FriendDecl *D) {
   if (TypeSourceInfo *TSI = D->getFriendType()) {
     unsigned NumTPLists = D->getFriendTypeNumTemplateParameterLists();

test/Index/comment-cplus-decls.cpp

struct data {
         unsigned objectID;
     };
 /**
  * \brief plain c++ data field
 */
     data* reserved;
 };
 // CHECK: <Declaration>class Test {}</Declaration>
-// CHECK: <Declaration>Test() : reserved(new Test::data()) {}</Declaration>
+// CHECK: <Declaration>Test()</Declaration>
 // CHECK: <Declaration>unsigned int getID() const</Declaration>
 // CHECK: <Declaration>~Test(){{( noexcept)?}}</Declaration>
 // CHECK: <Declaration>Test::data *reserved</Declaration>
 
 
 class S {
 /**
  * \brief Aaa

unittests/AST/DeclPrinterTest.cpp

 #include "gtest/gtest.h"
 
 using namespace clang;
 using namespace ast_matchers;
 using namespace tooling;
 
 namespace {
 
-void PrintDecl(raw_ostream &Out, const ASTContext *Context, const Decl *D) {
+using PrintingPolicyModifier = void (*)(PrintingPolicy &policy);
+
+void PrintDecl(raw_ostream &Out, const ASTContext *Context, const Decl *D,
+               PrintingPolicyModifier PolicyModifier) {
   PrintingPolicy Policy = Context->getPrintingPolicy();
   Policy.TerseOutput = true;
+  if (PolicyModifier)
+    PolicyModifier(Policy);
   D->print(Out, Policy, /*Indentation*/ 0, /*PrintInstantiation*/ false);
 }
 
 class PrintMatch : public MatchFinder::MatchCallback {
   SmallString<1024> Printed;
   unsigned NumFoundDecls;
+  PrintingPolicyModifier PolicyModifier;
 
 public:
-  PrintMatch() : NumFoundDecls(0) {}
+  PrintMatch(PrintingPolicyModifier PolicyModifier)
+      : NumFoundDecls(0), PolicyModifier(PolicyModifier) {}
 
   void run(const MatchFinder::MatchResult &Result) override {
     const Decl *D = Result.Nodes.getNodeAs<Decl>("id");
     if (!D || D->isImplicit())
       return;
     NumFoundDecls++;
     if (NumFoundDecls > 1)
       return;
 
     llvm::raw_svector_ostream Out(Printed);
-    PrintDecl(Out, Result.Context, D);
+    PrintDecl(Out, Result.Context, D, PolicyModifier);
   }
 
   StringRef getPrinted() const {
     return Printed;
   }
 
   unsigned getNumFoundDecls() const {
     return NumFoundDecls;
   }
 };
 
-::testing::AssertionResult PrintedDeclMatches(
-                                  StringRef Code,
-                                  const std::vector<std::string> &Args,
-                                  const DeclarationMatcher &NodeMatch,
-                                  StringRef ExpectedPrinted,
-                                  StringRef FileName) {
-  PrintMatch Printer;
+::testing::AssertionResult
+PrintedDeclMatches(StringRef Code, const std::vector<std::string> &Args,
+                   StringRef ExpectedPrinted, StringRef FileName,
+                   PrintingPolicyModifier PolicyModifier = nullptr) {
+  PrintMatch Printer(PolicyModifier);
   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() << "\"";
PrintedDeclMatches(StringRef Code, const std::vector<std::string> &Args,
   if (Printer.getPrinted() != ExpectedPrinted)
     return ::testing::AssertionFailure()
       << "Expected \"" << ExpectedPrinted.str() << "\", "
          "got \"" << Printer.getPrinted().str() << "\"";
 
   return ::testing::AssertionSuccess();
 }
 
-::testing::AssertionResult PrintedDeclCXX98Matches(StringRef Code,
-                                                   StringRef DeclName,
-                                                   StringRef ExpectedPrinted) {
+::testing::AssertionResult
+PrintedDeclCXX98Matches(StringRef Code, StringRef DeclName,
+                        StringRef ExpectedPrinted,
+                        PrintingPolicyModifier PolicyModifier = nullptr) {
   std::vector<std::string> Args(1, "-std=c++98");
   return PrintedDeclMatches(Code,
                             Args,
                             namedDecl(hasName(DeclName)).bind("id"),
                             ExpectedPrinted,
-                            "input.cc");
+                            "input.cc",
+                            PolicyModifier);
 }
 
-::testing::AssertionResult PrintedDeclCXX98Matches(
-                                  StringRef Code,
-                                  const DeclarationMatcher &NodeMatch,
-                                  StringRef ExpectedPrinted) {
+::testing::AssertionResult
+PrintedDeclCXX98Matches(StringRef Code, const DeclarationMatcher &NodeMatch,
+                        StringRef ExpectedPrinted,
+                        PrintingPolicyModifier PolicyModifier = nullptr) {
   std::vector<std::string> Args(1, "-std=c++98");
   return PrintedDeclMatches(Code,
                             Args,
                             NodeMatch,
                             ExpectedPrinted,
-                            "input.cc");
+                            "input.cc",
+                            PolicyModifier);
 }
 
 ::testing::AssertionResult PrintedDeclCXX11Matches(StringRef Code,
                                                    StringRef DeclName,
                                                    StringRef ExpectedPrinted) {
   std::vector<std::string> Args(1, "-std=c++11");
   return PrintedDeclMatches(Code,
                             Args,

 
 TEST(DeclPrinter, TestFunctionDecl1) {
   ASSERT_TRUE(PrintedDeclCXX98Matches(
     "void A();",
     "A",
     "void A()"));
 }
 
+TEST(DeclPrinter, TestFreeFunctionDecl_FullyQualifiedName) {
+    ASSERT_TRUE(PrintedDeclCXX98Matches(
+      "void A();",
+      "A",
+      "void A()",
+      [](PrintingPolicy &Policy){ Policy.FullyQualifiedName = true; }));
+}
+
+TEST(DeclPrinter, TestFreeFunctionDeclInNamespace_FullyQualifiedName) {
+    ASSERT_TRUE(PrintedDeclCXX98Matches(
+      "namespace X { void A(); };",
+      "A",
+      "void X::A()",
+      [](PrintingPolicy &Policy){ Policy.FullyQualifiedName = true; }));
+}
+
+TEST(DeclPrinter, TestMemberFunction_FullyQualifiedName) {
+    ASSERT_TRUE(PrintedDeclCXX98Matches(
+      "struct X { void A(); };",
+      "A",
+      "void X::A()",
+      [](PrintingPolicy &Policy){ Policy.FullyQualifiedName = true; }));
+}
+
+TEST(DeclPrinter, TestMemberFunctionInNamespace_FullyQualifiedName) {
+    ASSERT_TRUE(PrintedDeclCXX98Matches(
+      "namespace Z { struct X { void A(); }; }",
+      "A",
+      "void Z::X::A()",
+      [](PrintingPolicy &Policy){ Policy.FullyQualifiedName = true; }));
+}
+
+TEST(DeclPrinter, TestMemberFunctionOutside_FullyQualifiedName) {
+    ASSERT_TRUE(PrintedDeclCXX98Matches(
+      "struct X { void A(); };"
+       "void X::A() {}",
+      functionDecl(hasName("A"), isDefinition()).bind("id"),
+      "void X::A()",
+      [](PrintingPolicy &Policy){ Policy.FullyQualifiedName = true; }));
+}
+
 TEST(DeclPrinter, TestFunctionDecl2) {
   ASSERT_TRUE(PrintedDeclCXX98Matches(
     "void A() {}",
     "A",
     "void A()"));
 }
 
 TEST(DeclPrinter, TestFunctionDecl3) {
TEST(DeclPrinter, TestCXXConstructorDecl4) {
   ASSERT_TRUE(PrintedDeclCXX98Matches(
     "struct A {"
     "  A(const A &a, int = 0);"
     "};",
     cxxConstructorDecl(ofClass(hasName("A"))).bind("id"),
     "A(const A &a, int = 0)"));
 }
 
+TEST(DeclPrinter, TestCXXConstructorDeclWithMemberInitializer) {
+  ASSERT_TRUE(PrintedDeclCXX98Matches(
+    "struct A {"
+    "  int m;"
+    "  A() : m(2) {}"
+    "};",
+    cxxConstructorDecl(ofClass(hasName("A"))).bind("id"),
+    "A()"));
+}
+
+TEST(DeclPrinter, TestCXXConstructorDeclWithMemberInitializer_NoTerseOutput) {
+  ASSERT_TRUE(PrintedDeclCXX98Matches(
+    "struct A {"
+    "  int m;"
+    "  A() : m(2) {}"
+    "};",
+    cxxConstructorDecl(ofClass(hasName("A"))).bind("id"),
+    "A() : m(2) {\n}\n",
+    [](PrintingPolicy &Policy){ Policy.TerseOutput = false; }));
+}
+
 TEST(DeclPrinter, TestCXXConstructorDecl5) {
   ASSERT_TRUE(PrintedDeclCXX11Matches(
     "struct A {"
     "  A(const A &&a);"
     "};",
     cxxConstructorDecl(ofClass(hasName("A"))).bind("id"),
     "A(const A &&a)"));
 }

 
 TEST(DeclPrinter, TestCXXConstructorDecl11) {
   ASSERT_TRUE(PrintedDeclCXX11nonMSCMatches(
     "template<typename... T>"
     "struct A : public T... {"
     "  A(T&&... ts) : T(ts)... {}"
     "};",
     cxxConstructorDecl(ofClass(hasName("A"))).bind("id"),
-    "A<T...>(T &&...ts) : T(ts)... {}"));
+    "A<T...>(T &&...ts)"));
 }
 
 TEST(DeclPrinter, TestCXXDestructorDecl1) {
   ASSERT_TRUE(PrintedDeclCXX98Matches(
     "struct A {"
     "  ~A();"
     "};",
     cxxDestructorDecl(ofClass(hasName("A"))).bind("id"),