[clang-doc] Support default args for functions.

[brettw]

Summary

Adds support for default arguments in the internal representation and reads these values from the source. Implements writing these values to YAML but does not implement this for the HTML or markdown outputs.

[paulkirth]

Thanks for the patch. This is mostly LGTM modulo a few nits.

My one question is regarding documentation. Do you think this needs to be described in the clang-tools-extra/doc/clang-doc.rst? And are there any changes in workflow or tool usage that we should document? I assume not, since this just changes the YAML format. Is that correct?

clang-tools-extra/clang-doc/Serialize.cpp

300

nit: can we avoid introducing unrelated formatting changes, here and elsewhere in the patch?

324–329

can we combine these conditions somehow, given that they do the same thing?

Maybe this? WDYT?

if(!FieldInfo)
  FieldInfo = &I.Params.emplace_back(P->getOriginalType().getAsString(), ...

452

nit: same here

460

nit: ditto

[brettw]

There is no usage change for this. It just adds another field to the YAML.

[paulkirth]

Lgtm

clang-tools-extra/clang-doc/BitcodeReader.cpp

llvm::Error parseRecord(const Record &R, unsigned ID, llvm::StringRef Blob,
   return llvm::Error::success();
 }
 
 llvm::Error parseRecord(const Record &R, unsigned ID, llvm::StringRef Blob,
                         FieldTypeInfo *I) {
   switch (ID) {
   case FIELD_TYPE_NAME:
     return decodeRecord(R, I->Name, Blob);
+  case FIELD_DEFAULT_VALUE:
+    return decodeRecord(R, I->DefaultValue, Blob);
   default:
     return llvm::createStringError(llvm::inconvertibleErrorCode(),
                                    "invalid field for TypeInfo");
   }
 }
 
 llvm::Error parseRecord(const Record &R, unsigned ID, llvm::StringRef Blob,
                         MemberTypeInfo *I) {

clang-tools-extra/clang-doc/BitcodeWriter.h

enum RecordId {
   COMMENT_PARAMNAME,
   COMMENT_CLOSENAME,
   COMMENT_SELFCLOSING,
   COMMENT_EXPLICIT,
   COMMENT_ATTRKEY,
   COMMENT_ATTRVAL,
   COMMENT_ARG,
   FIELD_TYPE_NAME,
+  FIELD_DEFAULT_VALUE,
   MEMBER_TYPE_NAME,
   MEMBER_TYPE_ACCESS,
   NAMESPACE_USR,
   NAMESPACE_NAME,
   NAMESPACE_PATH,
   ENUM_USR,
   ENUM_NAME,
   ENUM_DEFLOCATION,

clang-tools-extra/clang-doc/BitcodeWriter.cpp

RecordIdNameMap = []() {
           {COMMENT_PARAMNAME, {"ParamName", &StringAbbrev}},
           {COMMENT_CLOSENAME, {"CloseName", &StringAbbrev}},
           {COMMENT_SELFCLOSING, {"SelfClosing", &BoolAbbrev}},
           {COMMENT_EXPLICIT, {"Explicit", &BoolAbbrev}},
           {COMMENT_ATTRKEY, {"AttrKey", &StringAbbrev}},
           {COMMENT_ATTRVAL, {"AttrVal", &StringAbbrev}},
           {COMMENT_ARG, {"Arg", &StringAbbrev}},
           {FIELD_TYPE_NAME, {"Name", &StringAbbrev}},
+          {FIELD_DEFAULT_VALUE, {"DefaultValue", &StringAbbrev}},
           {MEMBER_TYPE_NAME, {"Name", &StringAbbrev}},
           {MEMBER_TYPE_ACCESS, {"Access", &IntAbbrev}},
           {NAMESPACE_USR, {"USR", &SymbolIDAbbrev}},
           {NAMESPACE_NAME, {"Name", &StringAbbrev}},
           {NAMESPACE_PATH, {"Path", &StringAbbrev}},
           {ENUM_USR, {"USR", &SymbolIDAbbrev}},
           {ENUM_NAME, {"Name", &StringAbbrev}},
           {ENUM_DEFLOCATION, {"DefLocation", &LocationAbbrev}},
RecordsByBlock{
         // Comment Block
         {BI_COMMENT_BLOCK_ID,
          {COMMENT_KIND, COMMENT_TEXT, COMMENT_NAME, COMMENT_DIRECTION,
           COMMENT_PARAMNAME, COMMENT_CLOSENAME, COMMENT_SELFCLOSING,
           COMMENT_EXPLICIT, COMMENT_ATTRKEY, COMMENT_ATTRVAL, COMMENT_ARG}},
         // Type Block
         {BI_TYPE_BLOCK_ID, {}},
         // FieldType Block
-        {BI_FIELD_TYPE_BLOCK_ID, {FIELD_TYPE_NAME}},
+        {BI_FIELD_TYPE_BLOCK_ID, {FIELD_TYPE_NAME, FIELD_DEFAULT_VALUE}},
         // MemberType Block
         {BI_MEMBER_TYPE_BLOCK_ID, {MEMBER_TYPE_NAME, MEMBER_TYPE_ACCESS}},
         // Enum Block
         {BI_ENUM_BLOCK_ID,
          {ENUM_USR, ENUM_NAME, ENUM_DEFLOCATION, ENUM_LOCATION, ENUM_MEMBER,
           ENUM_SCOPED}},
         // Namespace Block
         {BI_NAMESPACE_BLOCK_ID,
void ClangDocBitcodeWriter::emitBlock(const TypeInfo &T) {
   StreamSubBlockGuard Block(Stream, BI_TYPE_BLOCK_ID);
   emitBlock(T.Type, FieldId::F_type);
 }
 
 void ClangDocBitcodeWriter::emitBlock(const FieldTypeInfo &T) {
   StreamSubBlockGuard Block(Stream, BI_FIELD_TYPE_BLOCK_ID);
   emitBlock(T.Type, FieldId::F_type);
   emitRecord(T.Name, FIELD_TYPE_NAME);
+  emitRecord(T.DefaultValue, FIELD_DEFAULT_VALUE);
 }
 
 void ClangDocBitcodeWriter::emitBlock(const MemberTypeInfo &T) {
   StreamSubBlockGuard Block(Stream, BI_MEMBER_TYPE_BLOCK_ID);
   emitBlock(T.Type, FieldId::F_type);
   emitRecord(T.Name, MEMBER_TYPE_NAME);
   emitRecord(T.Access, MEMBER_TYPE_ACCESS);
   for (const auto &CI : T.Description)

clang-tools-extra/clang-doc/Representation.h

FieldTypeInfo(SymbolID Type, StringRef Field, InfoType IT, StringRef Path,
                 llvm::StringRef Name)
       : TypeInfo(Type, Field, IT, Path), Name(Name) {}
   FieldTypeInfo(llvm::StringRef RefName, llvm::StringRef Name)
       : TypeInfo(RefName), Name(Name) {}
   FieldTypeInfo(llvm::StringRef RefName, StringRef Path, llvm::StringRef Name)
       : TypeInfo(RefName, Path), Name(Name) {}
 
   bool operator==(const FieldTypeInfo &Other) const {
-    return std::tie(Type, Name) == std::tie(Other.Type, Other.Name);
+    return std::tie(Type, Name, DefaultValue) ==
+           std::tie(Other.Type, Other.Name, Other.DefaultValue);
   }
 
   SmallString<16> Name; // Name associated with this info.
+
+  // When used for function parameters, contains the string representing the
+  // expression of the default value, if any.
+  SmallString<16> DefaultValue;
 };
 
 // Info for member types.
 struct MemberTypeInfo : public FieldTypeInfo {
   MemberTypeInfo() = default;
   MemberTypeInfo(SymbolID Type, StringRef Field, InfoType IT, StringRef Path,
                  llvm::StringRef Name, AccessSpecifier Access)
       : FieldTypeInfo(Type, Field, IT, Path, Name), Access(Access) {}

clang-tools-extra/clang-doc/Serialize.cpp

 //===-- Serialize.cpp - ClangDoc Serializer ---------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "Serialize.h"
 #include "BitcodeWriter.h"
 #include "clang/AST/Comment.h"
 #include "clang/Index/USRGeneration.h"
+#include "clang/Lex/Lexer.h"
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/SHA1.h"
 
 using clang::comments::FullComment;
 
 namespace clang {
 namespace doc {
if (const auto *T = getDeclForType(F->getTypeSourceInfo()->getType())) {
         I.Members.emplace_back(
             getUSRForDecl(T), N->getNameAsString(), InfoType::IT_record,
             getInfoRelativePath(N), F->getNameAsString(),
             getFinalAccessSpecifier(Access, N->getAccessUnsafe()));
         continue;
       }
     }
 
     auto& member = I.Members.emplace_back(
         F->getTypeSourceInfo()->getType().getAsString(), F->getNameAsString(),
         getFinalAccessSpecifier(Access, F->getAccessUnsafe()));
     populateMemberTypeInfo(member, F);
   }
 }
 
 static void parseEnumerators(EnumInfo &I, const EnumDecl *D) {
   for (const EnumConstantDecl *E : D->enumerators())
     I.Members.emplace_back(E->getNameAsString());
 }
 
 static void parseParameters(FunctionInfo &I, const FunctionDecl *D) {
   for (const ParmVarDecl *P : D->parameters()) {
+    FieldTypeInfo *FieldInfo = nullptr;
     if (const auto *T = getDeclForType(P->getOriginalType())) {
       if (const auto *N = dyn_cast<EnumDecl>(T)) {
-        I.Params.emplace_back(getUSRForDecl(N), N->getNameAsString(),
-                              InfoType::IT_enum, getInfoRelativePath(N),
-                              P->getNameAsString());
-        continue;
+        FieldInfo = &I.Params.emplace_back(
+            getUSRForDecl(N), N->getNameAsString(), InfoType::IT_enum,
+            getInfoRelativePath(N), P->getNameAsString());
       } else if (const auto *N = dyn_cast<RecordDecl>(T)) {
-        I.Params.emplace_back(getUSRForDecl(N), N->getNameAsString(),
-                              InfoType::IT_record, getInfoRelativePath(N),
-                              P->getNameAsString());
-        continue;
+        FieldInfo = &I.Params.emplace_back(
+            getUSRForDecl(N), N->getNameAsString(), InfoType::IT_record,
+            getInfoRelativePath(N), P->getNameAsString());
       }
+      // Otherwise fall through to the default case below.
     }
-    I.Params.emplace_back(P->getOriginalType().getAsString(),
+
+    if (!FieldInfo) {
+      FieldInfo = &I.Params.emplace_back(P->getOriginalType().getAsString(),
                                          P->getNameAsString());
     }
+
+    if (const Expr *DefaultArg = P->getDefaultArg()) {
+      FieldInfo->DefaultValue = Lexer::getSourceText(
+          CharSourceRange::getTokenRange(DefaultArg->getSourceRange()),
+          D->getASTContext().getSourceManager(),
+          D->getASTContext().getLangOpts());
+    }
+  }
 }
 
 // TODO: Remove the serialization of Parents and VirtualParents, this
 // information is also extracted in the other definition of parseBases.
 static void parseBases(RecordInfo &I, const CXXRecordDecl *D) {
   // Don't parse bases if this isn't a definition.
   if (!D->isThisDeclarationADefinition())
     return;
if (const auto *T = getDeclForType(D->getReturnType())) {
     I.ReturnType = TypeInfo(D->getReturnType().getAsString());
   }
   parseParameters(I, D);
 }
 
 static void populateMemberTypeInfo(MemberTypeInfo &I, const FieldDecl *D) {
   assert(D && "Expect non-null FieldDecl in populateMemberTypeInfo");
 
   ASTContext& Context = D->getASTContext();
   // TODO investigate whether we can use ASTContext::getCommentForDecl instead
   // of this logic. See also similar code in Mapper.cpp.
   RawComment *Comment = Context.getRawCommentForDeclNoCache(D);
   if (!Comment)
     return;
 
   Comment->setAttached();
   if (comments::FullComment* fc = Comment->parse(Context, nullptr, D)) {
     I.Description.emplace_back();
     parseFullComment(fc, I.Description.back());
   }
 }
 
 static void
 parseBases(RecordInfo &I, const CXXRecordDecl *D, bool IsFileInRootDir,
            bool PublicOnly, bool IsParent,

clang-tools-extra/clang-doc/YAMLGenerator.cpp

 
 static void TypeInfoMapping(IO &IO, TypeInfo &I) {
   IO.mapOptional("Type", I.Type, Reference());
 }
 
 static void FieldTypeInfoMapping(IO &IO, FieldTypeInfo &I) {
   TypeInfoMapping(IO, I);
   IO.mapOptional("Name", I.Name, SmallString<16>());
+  IO.mapOptional("DefaultValue", I.DefaultValue, SmallString<16>());
 }
 
 static void InfoMapping(IO &IO, Info &I) {
   IO.mapRequired("USR", I.USR);
   IO.mapOptional("Name", I.Name, SmallString<16>());
   IO.mapOptional("Path", I.Path, SmallString<128>());
   IO.mapOptional("Namespace", I.Namespace, llvm::SmallVector<Reference, 4>());
   IO.mapOptional("Description", I.Description);

 template <> struct MappingTraits<TypeInfo> {
   static void mapping(IO &IO, TypeInfo &I) { TypeInfoMapping(IO, I); }
 };
 
 template <> struct MappingTraits<FieldTypeInfo> {
   static void mapping(IO &IO, FieldTypeInfo &I) {
     TypeInfoMapping(IO, I);
     IO.mapOptional("Name", I.Name, SmallString<16>());
+    IO.mapOptional("DefaultValue", I.DefaultValue, SmallString<16>());
   }
 };
 
 template <> struct MappingTraits<MemberTypeInfo> {
   static void mapping(IO &IO, MemberTypeInfo &I) {
     FieldTypeInfoMapping(IO, I);
     // clang::AccessSpecifier::AS_none is used as the default here because it's
     // the AS that shouldn't be part of the output. Even though AS_public is the

clang-tools-extra/unittests/clang-doc/SerializeTest.cpp

class J : public I<int> {} ;)raw",
   CheckRecordInfo(&ExpectedJ, J);
 }
 
 TEST(SerializeTest, emitModulePublicLFunctions) {
   EmittedInfoList Infos;
   std::vector<std::string> Args;
   Args.push_back("-fmodules-ts");
   ExtractInfosFromCodeWithArgs(R"raw(export module M;
-int moduleFunction(int x);
+int moduleFunction(int x, double d = 3.2 - 1.0);
 static int staticModuleFunction(int x);
 export double exportedModuleFunction(double y);)raw",
                                2, /*Public=*/true, Infos, Args);
 
   NamespaceInfo *BWithFunction = InfoAsNamespace(Infos[0].get());
   NamespaceInfo ExpectedBWithFunction(EmptySID);
   FunctionInfo F;
   F.Name = "moduleFunction";
   F.ReturnType = TypeInfo(EmptySID, "int", InfoType::IT_default);
   F.Loc.emplace_back(0, llvm::SmallString<16>{"test.cpp"});
   F.Params.emplace_back("int", "x");
+  F.Params.emplace_back("double", "d");
+  F.Params.back().DefaultValue = "3.2 - 1.0";
   F.Access = AccessSpecifier::AS_none;
   ExpectedBWithFunction.ChildFunctions.emplace_back(std::move(F));
   CheckNamespaceInfo(&ExpectedBWithFunction, BWithFunction);
 
   NamespaceInfo *BWithExportedFunction = InfoAsNamespace(Infos[1].get());
   NamespaceInfo ExpectedBWithExportedFunction(EmptySID);
   FunctionInfo ExportedF;
   ExportedF.Name = "exportedModuleFunction";

clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp

TEST(YAMLGeneratorTest, emitFunctionYAML) {
   I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
   I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
   I.Access = AccessSpecifier::AS_none;
 
   I.ReturnType =
       TypeInfo(EmptySID, "void", InfoType::IT_default, "path/to/void");
   I.Params.emplace_back("int", "path/to/int", "P");
+  I.Params.emplace_back("double", "path/to/double", "D");
+  I.Params.back().DefaultValue = "2.0 * M_PI";
   I.IsMethod = true;
   I.Parent = Reference(EmptySID, "Parent", InfoType::IT_record);
 
   auto G = getYAMLGenerator();
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
   auto Err = G->generateDocForInfo(&I, Actual, ClangDocContext());

 Parent:
   Type:            Record
   Name:            'Parent'
 Params:
   - Type:
       Name:            'int'
       Path:            'path/to/int'
     Name:            'P'
+  - Type:
+      Name:            'double'
+      Path:            'path/to/double'
+    Name:            'D'
+    DefaultValue:    '2.0 * M_PI'
 ReturnType:
   Type:
     Name:            'void'
     Path:            'path/to/void'
 ...
 )raw";
   EXPECT_EQ(Expected, Actual.str());
 }