r176387 - comment parsing. Keep the original command format

jahanian fjahanian at apple.com
Fri Mar 1 17:26:39 PST 2013


On Mar 1, 2013, at 4:29 PM, Dmitri Gribenko <gribozavr at gmail.com> wrote:

> 
> 
> On Saturday, March 2, 2013, Fariborz Jahanian wrote:
> Author: fjahanian
> Date: Fri Mar  1 16:51:30 2013
> New Revision: 176387
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=176387&view=rev
> Log:
> comment parsing. Keep the original command format
> in AST for source fidelity and use it in diagnostics
> to refer to the original format. // rdar://13066276
> 
> Modified:
>     cfe/trunk/include/clang/AST/Comment.h
>     cfe/trunk/include/clang/AST/CommentLexer.h
>     cfe/trunk/include/clang/Basic/DiagnosticCommentKinds.td
>     cfe/trunk/lib/AST/CommentLexer.cpp
>     cfe/trunk/lib/AST/CommentParser.cpp
>     cfe/trunk/lib/AST/CommentSema.cpp
>     cfe/trunk/test/Sema/warn-documentation.cpp
> 
> Modified: cfe/trunk/include/clang/AST/Comment.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Comment.h?rev=176387&r1=176386&r2=176387&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/AST/Comment.h (original)
> +++ cfe/trunk/include/clang/AST/Comment.h Fri Mar  1 16:51:30 2013
> @@ -570,13 +570,16 @@ protected:
> 
>    /// Paragraph argument.
>    ParagraphComment *Paragraph;
> -
> +
> +  /// Header Doc command, if true
> +  bool HDCommand;
> +
>    BlockCommandComment(CommentKind K,
>                        SourceLocation LocBegin,
>                        SourceLocation LocEnd,
>                        unsigned CommandID) :
>        BlockContentComment(K, LocBegin, LocEnd),
> -      Paragraph(NULL) {
> +      Paragraph(NULL), HDCommand(false) {
>      setLocation(getCommandNameBeginLoc());
>      BlockCommandCommentBits.CommandID = CommandID;
>    }
> @@ -586,7 +589,7 @@ public:
>                        SourceLocation LocEnd,
>                        unsigned CommandID) :
>        BlockContentComment(BlockCommandCommentKind, LocBegin, LocEnd),
> -      Paragraph(NULL) {
> +      Paragraph(NULL), HDCommand(false) {
>      setLocation(getCommandNameBeginLoc());
>      BlockCommandCommentBits.CommandID = CommandID;
>    }
> @@ -657,6 +660,14 @@ public:
>      if (NewLocEnd.isValid())
>        setSourceRange(SourceRange(getLocStart(), NewLocEnd));
>    }
> +
> +  bool getHDCommand() const LLVM_READONLY {
> +    return HDCommand;
> +  }
> +
> +  void setHDCommand(bool HDC) {
> +    HDCommand = HDC;
> +  }
>  };
> 
>  /// Doxygen \\param command.
> 
> Modified: cfe/trunk/include/clang/AST/CommentLexer.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/CommentLexer.h?rev=176387&r1=176386&r2=176387&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/AST/CommentLexer.h (original)
> +++ cfe/trunk/include/clang/AST/CommentLexer.h Fri Mar  1 16:51:30 2013
> @@ -75,6 +75,9 @@ class Token {
>    /// unused (command spelling can be found with CommandTraits).  Otherwise,
>    /// contains the length of the string that starts at TextPtr.
>    unsigned IntVal;
> +
> +  /// This command is a Header Doc command (command starts with '@').
> +  bool     HDCommand;
> 
> This is unfortunate that we increase sizeof Token.  We use Token as a value type that should be cheap to copy.  I think we could carry this information in the token kind, by defining 'backslash command' and 'at command' tokens.

OK.

>  
> 
>  public:
>    SourceLocation getLocation() const LLVM_READONLY { return Loc; }
> @@ -122,6 +125,10 @@ public:
>      return IntVal;
>    }
> 
> +  bool getHDCommand() const LLVM_READONLY {
> +    return HDCommand;
> +  }
> +
>    void setCommandID(unsigned ID) {
>      assert(is(tok::command));
>      IntVal = ID;
> 
> Modified: cfe/trunk/include/clang/Basic/DiagnosticCommentKinds.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticCommentKinds.td?rev=176387&r1=176386&r2=176387&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticCommentKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticCommentKinds.td Fri Mar  1 16:51:30 2013
> @@ -44,18 +44,18 @@ def note_doc_html_end_tag : Note<
>  // Commands
> 
>  def warn_doc_block_command_empty_paragraph : Warning<
> -  "empty paragraph passed to '\\%0' command">,
> +  "empty paragraph passed to '%select{\\|@}0%1' command">,
>    InGroup<Documentation>, DefaultIgnore;
> 
>  def warn_doc_block_command_duplicate : Warning<
> -  "duplicated command '\\%0'">,
> +  "duplicated command '%select{\\|@}0%1'">,
>    InGroup<Documentation>, DefaultIgnore;
> 
>  def note_doc_block_command_previous : Note<
> -  "previous command '\\%0' here">;
> +  "previous command '%select{\\|@}0%1' here">;
> 
>  def note_doc_block_command_previous_alias : Note<
> -  "previous command '\\%0' (an alias of '\\%1') here">;
> +  "previous command '%select{\\|@}0%1' (an alias of '\\%2') here">;
> 
>  // \param command
> 
> @@ -111,14 +111,14 @@ def note_doc_tparam_name_suggestion : No
>  // \returns command
> 
>  def warn_doc_returns_not_attached_to_a_function_decl : Warning<
> -  "'\\%0' command used in a comment that is not attached to "
> +  "'%select{\\|@}0%1' command used in a comment that is not attached to "
>    "a function or method declaration">,
>    InGroup<Documentation>, DefaultIgnore;
> 
>  def warn_doc_returns_attached_to_a_void_function : Warning<
> -  "'\\%0' command used in a comment that is attached to a "
> +  "'%select{\\|@}0%1' command used in a comment that is attached to a "
>    "%select{function returning void|constructor|destructor|"
> -  "method returning void}1">,
> +  "method returning void}2">,
>    InGroup<Documentation>, DefaultIgnore;
> 
>  // \deprecated command
> @@ -134,7 +134,7 @@ def note_add_deprecation_attr : Note<
>  // verbatim block commands
> 
>  def warn_verbatim_block_end_without_start : Warning<
> -  "'\\%0' command does not terminate a verbatim text block">,
> +  "'%select{\\|@}0%1' command does not terminate a verbatim text block">,
>    InGroup<Documentation>, DefaultIgnore;
> 
>  } // end of documentation issue category
> 
> Modified: cfe/trunk/lib/AST/CommentLexer.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/CommentLexer.cpp?rev=176387&r1=176386&r2=176387&view=diff
> ==============================================================================
> --- cfe/trunk/lib/AST/CommentLexer.cpp (original)
> +++ cfe/trunk/lib/AST/CommentLexer.cpp Fri Mar  1 16:51:30 2013
> @@ -298,6 +298,7 @@ void Lexer::lexCommentText(Token &T) {
>      switch(*TokenPtr) {
>        case '\\':
>        case '@': {
> +        T.HDCommand = (*TokenPtr == '@');
>          TokenPtr++;
>          if (TokenPtr == CommentEnd) {
>            formTextToken(T, TokenPtr);
> 
> Modified: cfe/trunk/lib/AST/CommentParser.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/CommentParser.cpp?rev=176387&r1=176386&r2=176387&view=diff
> ==============================================================================
> --- cfe/trunk/lib/AST/CommentParser.cpp (original)
> +++ cfe/trunk/lib/AST/CommentParser.cpp Fri Mar  1 16:51:30 2013
> @@ -313,15 +313,18 @@ BlockCommandComment *Parser::parseBlockC
>      PC = S.actOnParamCommandStart(Tok.getLocation(),
>                                    Tok.getEndLocation(),
>                                    Tok.getCommandID());
> +    PC->setHDCommand(Tok.getHDCommand());
> 
> This kind of fixups that are done after AST node is created is fragile -- we could forget to insert a fixup in a new place where we create ASTnodes.  This flag (or an enum?!) should be an argument for the onstructor.

Yeh, I was first going to pass down to these 3 actOn…. routines but changes would have become substantive. So I did not.
But, I agree with he underlying argument.

- fj


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130301/310e15ec/attachment.html>


More information about the cfe-commits mailing list