Comment parsing: Specify argument numbers for some block commands

[aaronpuchert]

Summary

The command traits have a member NumArgs for which all the parsing
infrastructure is in place, but no command was setting it to a value
other than 0. By doing so we get warnings when passing an empty
paragraph to \retval (the first argument is the return value, then comes
the description). We also take \xrefitem along for the ride, although as
the documentation states it's unlikely to be used directly.

[aaronpuchert]

Add an AST test to check that we parse the arguments correctly. (We don't use them for diagnostics currently.)

[thakis]

Hello,

one of your three commits in https://github.com/llvm/llvm-project/compare/562ce1592456...d2396d896ee1 broke building on mac:

• http://45.33.8.238/macm1/35247/step_4.txtp
• https://green.lab.llvm.org/green/job/clang-stage1-RA/

Please take a look, and revert for now if it takes a while to fix.

(Here's the error:

In file included from ../../clang/lib/AST/CommentParser.cpp:9:
In file included from ../../clang/include/clang/AST/CommentParser.h:16:
In file included from ../../clang/include/clang/AST/Comment.h:17:
In file included from ../../clang/include/clang/AST/DeclObjC.h:16:
In file included from ../../clang/include/clang/AST/Decl.h:19:
In file included from ../../clang/include/clang/AST/DeclBase.h:18:
In file included from ../../clang/include/clang/AST/DeclarationName.h:16:
In file included from ../../clang/include/clang/AST/Type.h:21:
In file included from ../../clang/include/clang/AST/NestedNameSpecifier.h:18:
../../clang/include/clang/Basic/Diagnostic.h:1353:8: error: use of overloaded operator '<<' is ambiguous (with operand types 'const clang::StreamingDiagnostic' and 'typename remove_reference<unsigned long &>::type' (aka 'unsigned long'))
    DB << std::move(V);
    ~~ ^  ~~~~~~~~~~~~
../../clang/lib/AST/CommentParser.cpp:417:57: note: in instantiation of function template specialization 'clang::DiagnosticBuilder::operator<<<unsigned long, void>' requested here
        << CommandTok.is(tok::at_command) << Info->Name << Args.size()
                                                        ^
../../clang/include/clang/Basic/Diagnostic.h:1400:35: note: candidate function
inline const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
                                  ^
../../clang/include/clang/Basic/Diagnostic.h:1406:35: note: candidate function
inline const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
                                  ^
../../clang/include/clang/Basic/Diagnostic.h:1422:35: note: candidate function
inline const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
                                  ^
../../clang/include/clang/Basic/Diagnostic.h:1428:35: note: candidate function
inline const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
                                  ^
../../clang/include/clang/Basic/Diagnostic.h:1353:8: note: built-in candidate operator<<(int, unsigned long)
    DB << std::move(V);
       ^
../../clang/include/clang/Basic/Diagnostic.h:1353:8: note: built-in candidate operator<<(int, int)
../../clang/include/clang/Basic/Diagnostic.h:1353:8: note: built-in candidate operator<<(int, long)
../../clang/include/clang/Basic/Diagnostic.h:1353:8: note: built-in candidate operator<<(int, long long)
../../clang/include/clang/Basic/Diagnostic.h:1353:8: note: built-in candidate operator<<(int, __int128)
../../clang/include/clang/Basic/Diagnostic.h:1353:8: note: built-in candidate operator<<(int, unsigned int)
../../clang/include/clang/Basic/Diagnostic.h:1353:8: note: built-in candidate operator<<(int, unsigned long long)
../../clang/include/clang/Basic/Diagnostic.h:1353:8: note: built-in candidate operator<<(int, unsigned __int128)
../../clang/include/clang/Basic/Diagnostic.h:1353:8: note: built-in candidate operator<<(long, unsigned long)
../../clang/include/clang/Basic/Diagnostic.h:1353:8: note: built-in candidate operator<<(long long, unsigned long)
../../clang/include/clang/Basic/Diagnostic.h:1353:8: note: built-in candidate operator<<(__int128, unsigned long)
../../clang/include/clang/Basic/Diagnostic.h:1353:8: note: built-in candidate operator<<(unsigned int, unsigned long)
../../clang/include/clang/Basic/Diagnostic.h:1353:8: note: built-in candidate operator<<(unsigned long, unsigned long)
../../clang/include/clang/Basic/Diagnostic.h:1353:8: note: built-in candidate operator<<(unsigned long long, unsigned long)
../../clang/include/clang/Basic/Diagnostic.h:1353:8: note: built-in candidate operator<<(unsigned __int128, unsigned long)

)

[thakis]

Looking at that error some more, it must've been D125429. Moving the comment over there, sorry for the noise.

clang/include/clang/AST/CommentCommands.td

 def Invariant  : BlockCommand<"invariant">;
 def Li         : BlockCommand<"li">;
 def Note       : BlockCommand<"note">;
 def Par        : BlockCommand<"par">;
 def Post       : BlockCommand<"post">;
 def Pre        : BlockCommand<"pre">;
 def Remark     : BlockCommand<"remark">;
 def Remarks    : BlockCommand<"remarks">;
-def Retval     : BlockCommand<"retval">;
+def Retval     : BlockCommand<"retval"> { let NumArgs = 1; }
 def Sa         : BlockCommand<"sa">;
 def See        : BlockCommand<"see">;
 def Since      : BlockCommand<"since">;
 def Test       : BlockCommand<"test">;
 def Todo       : BlockCommand<"todo">;
 def Version    : BlockCommand<"version">;
 def Warning    : BlockCommand<"warning">;
-def XRefItem   : BlockCommand<"xrefitem">;
+def XRefItem   : BlockCommand<"xrefitem"> { let NumArgs = 3; }
 // HeaderDoc commands
 def Abstract      : BlockCommand<"abstract"> { let IsBriefCommand = 1; }
 def ClassDesign   : RecordLikeDetailCommand<"classdesign">;
 def CoClass       : RecordLikeDetailCommand<"coclass">;
 def Dependency    : RecordLikeDetailCommand<"dependency">;
 def Discussion    : BlockCommand<"discussion">;
 def Helper        : RecordLikeDetailCommand<"helper">;
 def HelperClass   : RecordLikeDetailCommand<"helperclass">;

clang/test/AST/ast-dump-comment.cpp

 
 /// \brief Aaa
 int Test_BlockCommandComment;
 // CHECK:      VarDecl{{.*}}Test_BlockCommandComment
 // CHECK:        BlockCommandComment{{.*}} Name="brief"
 // CHECK-NEXT:     ParagraphComment
 // CHECK-NEXT:       TextComment{{.*}} Text=" Aaa"
 
+/// \retval 42 Aaa
+int Test_BlockCommandComment_WithArgs();
+// CHECK:      FunctionDecl{{.*}}Test_BlockCommandComment_WithArgs
+// CHECK:        BlockCommandComment{{.*}} Name="retval" Arg[0]="42"
+// CHECK-NEXT:     ParagraphComment
+// CHECK-NEXT:       TextComment{{.*}} Text=" Aaa"
+
 /// \param Aaa xxx
 /// \param [in,out] Bbb yyy
 void Test_ParamCommandComment(int Aaa, int Bbb);
 // CHECK:      FunctionDecl{{.*}}Test_ParamCommandComment
 // CHECK:        ParamCommandComment{{.*}} [in] implicitly Param="Aaa" ParamIndex=0
 // CHECK-NEXT:     ParagraphComment
 // CHECK-NEXT:       TextComment{{.*}} Text=" xxx"
 // CHECK:        ParamCommandComment{{.*}} [in,out] explicitly Param="Bbb" ParamIndex=1

clang/test/Sema/warn-documentation.cpp

 /// \returns Aaa
 ///
 /// Bbb
 ///
 /// \return Ccc
 int test_multiple_returns4(int);
 
 
+/// expected-warning@+1 {{empty paragraph passed to '\retval' command}}
+/// \retval 0
+int test_retval_no_paragraph();
+
+/// \retval 0 Everything is fine.
+int test_retval_fine();
+
+
 // expected-warning@+1 {{'\param' command used in a comment that is not attached to a function declaration}}
 /// \param a Blah blah.
 int test_param1_backslash;
 
 // rdar://13066276
 // Check that the diagnostic uses the same command marker as the comment.
 // expected-warning@+1 {{'@param' command used in a comment that is not attached to a function declaration}}
 /// @param a Blah blah.