r176468 - doc parsing. We want to issue a strong warning when

Dmitri Gribenko gribozavr at gmail.com
Mon Mar 4 17:23:05 PST 2013


On Tuesday, March 5, 2013, Fariborz Jahanian wrote:

> Author: fjahanian
> Date: Mon Mar  4 19:05:07 2013
> New Revision: 176468
>
> URL: http://llvm.org/viewvc/llvm-project?rev=176468&view=rev
> Log:
> doc parsing. We want to issue a strong warning when
> an @function comment is not followed by a function decl.
> // rdar://13094352
>
> Modified:
>     cfe/trunk/include/clang/AST/CommentCommandTraits.h
>     cfe/trunk/include/clang/AST/CommentCommands.td
>     cfe/trunk/include/clang/AST/CommentSema.h
>     cfe/trunk/include/clang/Basic/DiagnosticCommentKinds.td
>     cfe/trunk/lib/AST/CommentParser.cpp
>     cfe/trunk/lib/AST/CommentSema.cpp
>     cfe/trunk/test/Sema/warn-documentation.cpp
>     cfe/trunk/utils/TableGen/ClangCommentCommandInfoEmitter.cpp
>
> Modified: cfe/trunk/include/clang/AST/CommentCommandTraits.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/CommentCommandTraits.h?rev=176468&r1=176467&r2=176468&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/AST/CommentCommandTraits.h (original)
> +++ cfe/trunk/include/clang/AST/CommentCommandTraits.h Mon Mar  4 19:05:07
> 2013
> @@ -100,7 +100,10 @@ struct CommandInfo {
>    ///   \fn void f(int a);
>    /// \endcode
>    unsigned IsDeclarationCommand : 1;
> -
> +
> +  /// \brief True if verbatim-like line command is a function declaraton.
> +  unsigned IsFunctionDeclarationCommand : 1;
> +
>    /// \brief True if this command is unknown.  This \c CommandInfo object
> was
>    /// created during parsing.
>    unsigned IsUnknownCommand : 1;
>
> Modified: cfe/trunk/include/clang/AST/CommentCommands.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/CommentCommands.td?rev=176468&r1=176467&r2=176468&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/AST/CommentCommands.td (original)
> +++ cfe/trunk/include/clang/AST/CommentCommands.td Mon Mar  4 19:05:07 2013
> @@ -24,6 +24,7 @@ class Command<string name> {
>    bit IsVerbatimBlockEndCommand = 0;
>    bit IsVerbatimLineCommand = 0;
>    bit IsDeclarationCommand = 0;
> +  bit IsFunctionDeclarationCommand = 0;
>  }
>
>  class InlineCommand<string name> : Command<name> {
> @@ -59,6 +60,12 @@ class DeclarationVerbatimLineCommand<str
>    let IsDeclarationCommand = 1;
>  }
>
> +class FunctionDeclarationVerbatimLineCommand<string name> :
> +      VerbatimLineCommand<name> {
> +  let IsDeclarationCommand = 1;
> +  let IsFunctionDeclarationCommand = 1;
> +}
> +
>
>  //===----------------------------------------------------------------------===//
>  // InlineCommand
>
>  //===----------------------------------------------------------------------===//
> @@ -179,7 +186,7 @@ def Interface : DeclarationVerbatimLineC
>  def Protocol  : DeclarationVerbatimLineCommand<"protocol">;
>  def Category  : DeclarationVerbatimLineCommand<"category">;
>  def Template  : DeclarationVerbatimLineCommand<"template">;
> -def Function  : DeclarationVerbatimLineCommand<"function">;
> +def Function  : FunctionDeclarationVerbatimLineCommand<"function">;
>  def Method    : DeclarationVerbatimLineCommand<"method">;
>  def Callback  : DeclarationVerbatimLineCommand<"callback">;


Do we want the same warning for \method and \callback?


>  def Const     : DeclarationVerbatimLineCommand<"const">;
>
> Modified: cfe/trunk/include/clang/AST/CommentSema.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/CommentSema.h?rev=176468&r1=176467&r2=176468&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/AST/CommentSema.h (original)
> +++ cfe/trunk/include/clang/AST/CommentSema.h Mon Mar  4 19:05:07 2013
> @@ -198,6 +198,8 @@ public:
>    void checkBlockCommandDuplicate(const BlockCommandComment *Command);
>
>    void checkDeprecatedCommand(const BlockCommandComment *Comment);
> +
> +  void checkFunctionDeclVerbatimLine(const BlockCommandComment *Comment);
>
>    /// Resolve parameter names to parameter indexes in function
> declaration.
>    /// Emit diagnostics about unknown parametrs.
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticCommentKinds.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticCommentKinds.td?rev=176468&r1=176467&r2=176468&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticCommentKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticCommentKinds.td Mon Mar  4
> 19:05:07 2013
> @@ -73,6 +73,11 @@ def warn_doc_param_not_attached_to_a_fun
>    "a function declaration">,
>    InGroup<Documentation>, DefaultIgnore;
>
> +def warn_doc_function_not_attached_to_a_function_decl : Warning<
> +  "'@function' command used in a comment that is attached to "
> +  "a non-function declaration immediately following it">,
> +  InGroup<Documentation>, DefaultIgnore;
> +
>  def warn_doc_param_duplicate : Warning<
>    "parameter '%0' is already documented">,
>    InGroup<Documentation>, DefaultIgnore;
>
> Modified: cfe/trunk/lib/AST/CommentParser.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/CommentParser.cpp?rev=176468&r1=176467&r2=176468&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/AST/CommentParser.cpp (original)
> +++ cfe/trunk/lib/AST/CommentParser.cpp Mon Mar  4 19:05:07 2013
> @@ -706,6 +706,8 @@ VerbatimLineComment *Parser::parseVerbat
>                                                  TextBegin,
>                                                  Text);
>    consumeToken();
> +  S.checkFunctionDeclVerbatimLine(VL);
> +
>    return VL;
>  }
>
>
> Modified: cfe/trunk/lib/AST/CommentSema.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/CommentSema.cpp?rev=176468&r1=176467&r2=176468&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/AST/CommentSema.cpp (original)
> +++ cfe/trunk/lib/AST/CommentSema.cpp Mon Mar  4 19:05:07 2013
> @@ -88,6 +88,15 @@ ParamCommandComment *Sema::actOnParamCom
>    return Command;
>  }
>
> +void Sema::checkFunctionDeclVerbatimLine(const BlockCommandComment
> *Comment) {
> +  const CommandInfo *Info =
> Traits.getCommandInfo(Comment->getCommandID());
> +  if (Info->IsFunctionDeclarationCommand &&
> +      !isFunctionDecl())
> +    Diag(Comment->getLocation(),
> +         diag::warn_doc_function_not_attached_to_a_function_decl)
> +    << Comment->getSourceRange();
> +}


I don't think this short function will be ever reused, so you can just
inline it into sema::actOnVerbatimLine.  Also, parser is not calling any
check() functions currently.  In fact, there's no separate concept of
check() like we have in C++ AST because of template instantiation.

+
>  void Sema::actOnParamCommandDirectionArg(ParamCommandComment *Command,
>                                           SourceLocation ArgLocBegin,
>                                           SourceLocation ArgLocEnd,
>
> Modified: cfe/trunk/test/Sema/warn-documentation.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-documentation.cpp?rev=176468&r1=176467&r2=176468&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/Sema/warn-documentation.cpp (original)
> +++ cfe/trunk/test/Sema/warn-documentation.cpp Mon Mar  4 19:05:07 2013
> @@ -911,3 +911,12 @@ int test_nocrash12();
>  ///@param x at param y
>  int test_nocrash13(int x, int y);
>
> +// expected-warning at +3 {{'@function' command used in a comment that is
> attached to a non-function declaration immediately following it}}
> +// expected-warning at +3 {{'@param' command used in a comment that is not
> attached to a function declaration}}
> +// expected-warning at +3 {{'@result' command used in a comment that is not
> attached to a function or method declaration}}
> +/*!    @function Base64EncodeEx
> +       @param  inFlags  This is error flag
> +       @result Error
> +*/
> +typedef unsigned int Base64Flags;
> +unsigned Base64EncodeEx(Base64Flags    inFlags);


Please tear this test apart so that it tests only \function, and move it to
appropriate block, if there is one.  Crashers are conveniently located at
the end of this test.

>
> Modified: cfe/trunk/utils/TableGen/ClangCommentCommandInfoEmitter.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/ClangCommentCommandInfoEmitter.cpp?rev=176468&r1=176467&r2=176468&view=diff
>
> ==============================================================================
> --- cfe/trunk/utils/TableGen/ClangCommentCommandInfoEmitter.cpp (original)
> +++ cfe/trunk/utils/TableGen/ClangCommentCommandInfoEmitter.cpp Mon Mar  4
> 19:05:07 2013
> @@ -47,6 +47,7 @@ void EmitClangCommentCommandInfo(RecordK
>         << Tag.getValueAsBit("IsVerbatimBlockEndCommand") << ", "
>         << Tag.getValueAsBit("IsVerbatimLineCommand") << ", "
>         << Tag.getValueAsBit("IsDeclarationCommand") << ", "
> +       << Tag.getValueAsBit("IsFunctionDeclarationCommand") << ", "
>         << /* IsUnknownCommand = */ "0"
>         << " }";
>      if (i + 1 != e)
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu <javascript:;>
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>


-- 
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130305/dcc50e1a/attachment.html>


More information about the cfe-commits mailing list