Comment parsing: attaching comments to declarations

We can attach comments to declarations.  Right now we only support comments
that precede the declarations (trailing comments will be supported later).

The implementation approach is different from one we have in Clang.  In Swift
the Lexer attaches the comments to the next token, and parser checks if
comments are present on the first token of the declaration.  This is much
cleaner, and faster than Clang's approach (where we perform a binary search on
source locations and do ad-hoc fixups afterwards).

The comment <-> decl correspondence is modeled as "virtual" attributes that can
not be spelled in the source.  These attributes are not serialized at the
moment -- this will be implemented later.


Swift SVN r14031

include/swift/AST/Attr.def

@@ -26,6 +26,10 @@
 #define IB_ATTR(X) ATTR(X)
 #endif
 
+#ifndef VIRTUAL_ATTR
+#define VIRTUAL_ATTR(X) ATTR(X)
+#endif
+
 // Type attributes
 TYPE_ATTR(auto_closure)
 TYPE_ATTR(cc)
@@ -75,6 +79,10 @@ IB_ATTR(IBAction)
 IB_ATTR(IBLiveView)
 IB_ATTR(IBInspectable)
 
+// "Virtual" attributes can not be spelled in the source code.
+VIRTUAL_ATTR(raw_doc_comment)
+
+#undef VIRTUAL_ATTR
 #undef IB_ATTR
 #undef TYPE_ATTR
 #undef ATTR

include/swift/AST/Attr.h

@@ -212,13 +212,25 @@ public:
   
 /// \brief Attributes that may be applied to declarations.
 class DeclAttributes {
-  /// Source locations for every possible attribute that can be parsed in source.
+  /// Source locations for every possible attribute that can be parsed in
+  /// source.
   SourceLoc AttrLocs[AK_Count];
   bool HasAttr[AK_Count] = { false };
+
+  unsigned NumAttrsSet : 8;
+  unsigned NumVirtualAttrsSet : 8;
+
 public:
-  /// AtLoc - This is the location of the first '@' in the attribute specifier.
+  /// The location of the first '@' in the attribute specifier.
-  /// If this is an empty attribute specifier, then this will be an invalid loc.
+  ///
+  /// This is an invalid location if the declaration does not have any or has
+  /// only virtual attributes.
+  ///
+  /// This could be a valid location even if none of the attributes are set.
+  /// This can happen when the attributes were parsed, but then cleared because
+  /// they are not allowed in that context.
   SourceLoc AtLoc;
+
   StringRef AsmName;
 
   /// When the mutating attribute is present (i.e., we have a location for it),
@@ -226,13 +238,43 @@ public:
   /// Clients should generally use the getMutating() accessor.
   bool MutatingInverted = false;
 
-  DeclAttributes() {}
+  /// Source range of the attached comment.  The comment could be located
+  /// before or after the declaration).
+  CharSourceRange CommentRange;
 
-  bool isValid() const { return AtLoc.isValid(); }
+  DeclAttributes() : NumAttrsSet(0), NumVirtualAttrsSet(0) {}
+
+  bool shouldSaveInAST() const {
+    return AtLoc.isValid() || NumAttrsSet != 0;
+  }
+
+  bool isEmpty() const {
+    return NumAttrsSet == 0;
+  }
+
+  bool hasNonVirtualAttributes() const {
+    return NumAttrsSet - NumVirtualAttrsSet != 0;
+  }
 
   void clearAttribute(AttrKind A) {
+    if (!has(A))
+      return;
+
     AttrLocs[A] = SourceLoc();
     HasAttr[A] = false;
+
+    // Update counters.
+    NumAttrsSet--;
+    switch (A) {
+#define ATTR(X)
+#define VIRTUAL_ATTR(X) case AK_ ## X:
+      NumVirtualAttrsSet--;
+      break;
+#include "swift/AST/Attr.def"
+
+    default:
+      break;
+    }
   }
   
   bool has(AttrKind A) const {
@@ -244,8 +286,26 @@ public:
   }
   
   void setAttr(AttrKind A, SourceLoc L) {
+    bool HadAttribute = has(A);
+
     AttrLocs[A] = L;
     HasAttr[A] = true;
+
+    if (HadAttribute)
+      return;
+
+    // Update counters.
+    NumAttrsSet++;
+    switch (A) {
+#define ATTR(X)
+#define VIRTUAL_ATTR(X) case AK_ ## X:
+#include "swift/AST/Attr.def"
+      NumVirtualAttrsSet++;
+      break;
+
+    default:
+      break;
+    }
   }
 
   void getAttrLocs(SmallVectorImpl<SourceLoc> &Locs) const {
@@ -254,16 +314,6 @@ public:
         Locs.push_back(Loc);
     }
   }
-  
-  // This attribute list is empty if no attributes are specified.  Note that
-  // the presence of the leading @ is not enough to tell, because we want
-  // clients to be able to remove attributes they process until they get to
-  // an empty list.
-  bool empty() const {
-    for (bool elt : HasAttr)
-      if (elt) return false;
-    return true;
-  }
 
   bool isNoReturn() const { return has(AK_noreturn); }
   bool isAssignment() const { return has(AK_assignment); }
@@ -316,6 +366,10 @@ public:
     clearAttribute(AK_unowned);
   }
 
+  bool hasRawDocComment() const {
+    return CommentRange.isValid();
+  }
+
   void print(llvm::raw_ostream &OS) const;
   void print(ASTPrinter &Printer) const;
 };

include/swift/Basic/LangOptions.h

@@ -43,6 +43,9 @@ namespace swift {
 
     /// \brief Enable features useful for running in the debugger.
     bool DebuggerSupport = false;
+
+    /// \brief Keep comments during lexing and attach them to declarations.
+    bool AttachCommentsToDecls = false;
   };
 }
 

include/swift/Basic/SourceManager.h

@@ -127,6 +127,8 @@ public:
     assert(Loc.isValid());
     return LLVMSourceMgr.getLineAndColumn(Loc.Value, BufferID);
   }
+
+  StringRef extractText(CharSourceRange Range) const;
 };
 
 } // namespace swift

include/swift/Parse/Lexer.h

@@ -29,6 +29,12 @@ namespace swift {
 
   template<typename ...T> struct Diag;
 
+enum class CommentRetentionMode {
+  None,
+  AttachToNextToken,
+  ReturnAsTokens,
+};
+
 class Lexer {
   const LangOptions &LangOpts;
   const SourceManager &SourceMgr;
@@ -55,14 +61,29 @@ class Lexer {
   /// Pointer to the next not consumed character.
   const char *CurPtr;
 
+  /// @{
+  /// Members that are *not* permanent lexer state.  The values only make sense
+  /// during the lexImpl() invocation.  These variables are declared as members
+  /// rather than locals so that we don't have to thread them through to all
+  /// lexing helpers.
+
+  /// Points to the point in the source buffer where we started scanning for
+  /// the current token.  Thus, the range [LastCommentBlockStart, CurPtr)
+  /// covers all comments and whitespace that we skipped, and the token itself.
+  const char *LastCommentBlockStart = nullptr;
+
+  /// True if we have seen a comment while scanning for the current token.
+  bool SeenComment = false;
+
+  /// @}
+
   Token NextToken;
   
   /// \brief This is true if we're lexing a .sil file instead of a .swift
   /// file.  This enables the 'sil' keyword.
   const bool InSILMode;
 
-  /// \brief Set to true to return comment tokens, instead of skipping them.
+  const CommentRetentionMode RetainComments;
-  const bool KeepComments;
 
   /// InSILBody - This is true when we're lexing the body of a SIL declaration
   /// in a SIL file.  This enables some context-sensitive lexing.
@@ -97,10 +118,11 @@ private:
   /// everything.
   Lexer(const LangOptions &Options,
         const SourceManager &SourceMgr, DiagnosticEngine *Diags,
-        unsigned BufferID, bool InSILMode, bool KeepComments);
+        unsigned BufferID, bool InSILMode,
+        CommentRetentionMode RetainComments);
 
   /// @{
-  /// Helper routines used in Lexer constructors.
+  /// Helper routines used in \c Lexer constructors.
   void primeLexer();
   void initSubLexer(Lexer &Parent, State BeginState, State EndState);
   /// @}
@@ -121,17 +143,19 @@ public:
   ///   when parsing SIL files.
   Lexer(const LangOptions &Options,
         const SourceManager &SourceMgr, unsigned BufferID,
-        DiagnosticEngine *Diags, bool InSILMode, bool KeepComments = false)
+        DiagnosticEngine *Diags, bool InSILMode,
-      : Lexer(Options, SourceMgr, Diags, BufferID, InSILMode, KeepComments) {
+        CommentRetentionMode RetainComments = CommentRetentionMode::None)
+      : Lexer(Options, SourceMgr, Diags, BufferID, InSILMode, RetainComments) {
     primeLexer();
   }
 
   /// \brief Create a lexer that scans a subrange of the source buffer.
   Lexer(const LangOptions &Options,
         const SourceManager &SourceMgr, unsigned BufferID,
-        DiagnosticEngine *Diags, bool InSILMode, bool KeepComments,
+        DiagnosticEngine *Diags, bool InSILMode,
+        CommentRetentionMode RetainComments,
         unsigned Offset, unsigned EndOffset)
-      : Lexer(Options, SourceMgr, Diags, BufferID, InSILMode, KeepComments) {
+      : Lexer(Options, SourceMgr, Diags, BufferID, InSILMode, RetainComments) {
     assert(Offset <= EndOffset && "invalid range");
     initSubLexer(
         *this,
@@ -147,12 +171,10 @@ public:
   /// \param EndState end of the subrange
   Lexer(Lexer &Parent, State BeginState, State EndState)
       : Lexer(Parent.LangOpts, Parent.SourceMgr, Parent.Diags, Parent.BufferID,
-              Parent.InSILMode, Parent.isKeepingComments()) {
+              Parent.InSILMode, Parent.RetainComments) {
     initSubLexer(Parent, BeginState, EndState);
   }
 
-  bool isKeepingComments() const { return KeepComments; }
-
   /// \brief Returns true if this lexer will produce a code completion token.
   bool isCodeCompletion() const {
     return CodeCompletionPtr != nullptr;
@@ -164,6 +186,10 @@ public:
       lexImpl();
   }
 
+  bool isKeepingComments() const {
+    return RetainComments == CommentRetentionMode::ReturnAsTokens;
+  }
+
   /// peekNextToken - Return the next token to be returned by Lex without
   /// actually lexing it.
   const Token &peekNextToken() const { return NextToken; }

include/swift/Parse/Token.h

@@ -57,13 +57,18 @@ class Token {
   tok Kind;
 
   /// \brief Whether this token is the first token on the line.
-  bool AtStartOfLine;
+  unsigned AtStartOfLine : 1;
+
+  /// \brief The length of the comment that precedes the token.
+  ///
+  /// Hopefully 64 Kib is enough.
+  unsigned CommentLength : 16;
 
   /// Text - The actual string covered by the token in the source buffer.
   StringRef Text;
   
 public:
-  Token() : Kind(tok::NUM_TOKENS), AtStartOfLine(false) {}
+  Token() : Kind(tok::NUM_TOKENS), AtStartOfLine(false), CommentLength(0) {}
   
   tok getKind() const { return Kind; }
   void setKind(tok K) { Kind = K; }
@@ -133,16 +138,26 @@ public:
     return SourceLoc(llvm::SMLoc::getFromPointer(Text.begin()));
   }
 
+  bool hasComment() const {
+    return CommentLength != 0;
+  }
+
+  CharSourceRange getCommentRange() const {
+    return CharSourceRange(
+        SourceLoc(llvm::SMLoc::getFromPointer(Text.begin() - CommentLength)),
+        CommentLength);
+  }
+
   StringRef getText() const { return Text; }
   void setText(StringRef T) { Text = T; }
   
   unsigned getLength() const { return Text.size(); }
   
-  /// setToken - Set the token to the specified kind and source range.
+  /// \brief Set the token to the specified kind and source range.
-  ///
+  void setToken(tok K, StringRef T, unsigned CommentLength = 0) {
-  void setToken(tok K, StringRef T) {
     Kind = K;
     Text = T;
+    this->CommentLength = CommentLength;
   }
 };
   

lib/AST/Attr.cpp

@@ -40,7 +40,7 @@ void DeclAttributes::print(llvm::raw_ostream &OS) const {
 }
 
 void DeclAttributes::print(ASTPrinter &Printer) const {
-    if (empty())
+  if (NumAttrsSet == 0)
     return;
 
   if (isAssignment())

lib/Basic/SourceLoc.cpp

@@ -63,6 +63,15 @@ unsigned SourceManager::getByteDistance(SourceLoc Start, SourceLoc End) const {
   return End.Value.getPointer() - Start.Value.getPointer();
 }
 
+StringRef SourceManager::extractText(CharSourceRange Range) const {
+  assert(Range.isValid() && "range should be valid");
+
+  unsigned BufferID = findBufferContainingLoc(Range.getStart());
+  StringRef Buffer = LLVMSourceMgr.getMemoryBuffer(BufferID)->getBuffer();
+  return Buffer.substr(getLocOffsetInBuffer(Range.getStart(), BufferID),
+                       Range.getByteLength());
+}
+
 void SourceLoc::printLineAndColumn(raw_ostream &OS,
                                    const SourceManager &SM) const {
   if (isInvalid()) {

lib/Parse/Lexer.cpp

@@ -162,9 +162,10 @@ static uint32_t validateUTF8CharacterAndAdvance(const char *&Ptr,
 
 Lexer::Lexer(const LangOptions &Options,
              const SourceManager &SourceMgr, DiagnosticEngine *Diags,
-             unsigned BufferID, bool InSILMode, bool KeepComments)
+             unsigned BufferID, bool InSILMode,
+             CommentRetentionMode RetainComments)
     : LangOpts(Options), SourceMgr(SourceMgr), Diags(Diags), BufferID(BufferID),
-      InSILMode(InSILMode), KeepComments(KeepComments) {
+      InSILMode(InSILMode), RetainComments(RetainComments) {
   // Initialize buffer pointers.
   auto *Buffer = SourceMgr->getMemoryBuffer(BufferID);
   BufferStart = Buffer->getBufferStart();
@@ -219,7 +220,7 @@ Token Lexer::getTokenAt(SourceLoc Loc) {
          "location from the wrong buffer");
 
   Lexer L(LangOpts, SourceMgr, BufferID, Diags, InSILMode,
-          /*KeepComments=*/false);
+          CommentRetentionMode::None);
   L.restoreState(State(Loc));
   Token Result;
   L.lex(Result);
@@ -233,7 +234,12 @@ void Lexer::formToken(tok Kind, const char *TokStart) {
   if (Kind != tok::eof && ArtificialEOF && TokStart >= ArtificialEOF) {
     Kind = tok::eof;
   }
-  NextToken.setToken(Kind, StringRef(TokStart, CurPtr-TokStart));
+  unsigned CommentLength = 0;
+  if (RetainComments == CommentRetentionMode::AttachToNextToken && SeenComment)
+    CommentLength = TokStart - LastCommentBlockStart;
+
+  NextToken.setToken(Kind, StringRef(TokStart, CurPtr-TokStart),
+                     CommentLength);
 }
 
 Lexer::State Lexer::getStateForBeginningOfTokenLoc(SourceLoc Loc) const {
@@ -1316,6 +1322,10 @@ void Lexer::lexImpl() {
 
   NextToken.setAtStartOfLine(CurPtr == BufferStart);
 
+  // Remember where we started so that we can find the comment range.
+  LastCommentBlockStart = CurPtr;
+  SeenComment = false;
+
 Restart:
   // Remember the start of the token so we can form the text range.
   const char *TokStart = CurPtr;
@@ -1419,13 +1429,15 @@ Restart:
   case '/':
     if (CurPtr[0] == '/') {  // "//"
       skipSlashSlashComment();
-      if (KeepComments)
+      SeenComment = true;
+      if (isKeepingComments())
         return formToken(tok::comment, TokStart);
       goto Restart;
     }
     if (CurPtr[0] == '*') { // "/*"
       skipSlashStarComment();
-      if (KeepComments)
+      SeenComment = true;
+      if (isKeepingComments())
         return formToken(tok::comment, TokStart);
       goto Restart;
     }
@@ -1496,14 +1508,14 @@ SourceLoc Lexer::getLocForEndOfToken(const SourceManager &SM, SourceLoc Loc) {
 
   // Use fake language options; language options only affect validity
   // and the exact token produced.
-  LangOptions fakeLangOpts;
+  LangOptions FakeLangOpts;
-  
+
-  // KeepComments is true because either the caller skipped comments and
+  // Here we return comments as tokens because either the caller skipped
-  // normally we won't be at the beginning of a comment token (making
+  // comments and normally we won't be at the beginning of a comment token
-  // KeepComments irrelevant), or the caller lexed comments and KeepComments
+  // (making this option irrelevant), or the caller lexed comments and
-  // must be true.
+  // we need to lex just the comment token.
-  Lexer L(fakeLangOpts, SM, BufferID, nullptr, /*InSILMode=*/ false,
+  Lexer L(FakeLangOpts, SM, BufferID, nullptr, /*InSILMode=*/ false,
-          /*KeepComments=*/true);
+          CommentRetentionMode::ReturnAsTokens);
   L.restoreState(State(Loc));
   unsigned Length = L.peekNextToken().getLength();
   return Loc.getAdvancedLoc(Length);
@@ -1517,10 +1529,10 @@ static SourceLoc getLocForStartOfTokenInBuf(SourceManager &SM,
                                             bool InInterpolatedString) {
   // Use fake language options; language options only affect validity
   // and the exact token produced.
-  LangOptions fakeLangOptions;
+  LangOptions FakeLangOptions;
 
-  Lexer L(fakeLangOptions, SM, BufferID, nullptr, /*InSILMode=*/false,
+  Lexer L(FakeLangOptions, SM, BufferID, nullptr, /*InSILMode=*/false,
-          /*KeepComments=*/false, BufferStart, BufferEnd);
+          CommentRetentionMode::None, BufferStart, BufferEnd);
 
   // Lex tokens until we find the token that contains the source location.
   Token Tok;

lib/Parse/ParseDecl.cpp

@@ -115,7 +115,9 @@ bool Parser::parseTopLevel() {
   SF.ASTStage = SourceFile::Parsed;
   verify(SF);
 
-  State->markParserPosition(Tok.getLoc(), PreviousLoc);
+  // Next time start relexing from the beginning of the comment so that we can
+  // attach it to the token.
+  State->markParserPosition(Tok.getCommentRange().getStart(), PreviousLoc);
 
   return FoundTopLevelCodeToExecute;
 }
@@ -170,6 +172,7 @@ bool Parser::parseDeclAttribute(DeclAttributes &Attributes) {
   // Determine which attribute it is, and diagnose it if unknown.
   AttrKind attr = llvm::StringSwitch<AttrKind>(Tok.getText())
 #define ATTR(X) .Case(#X, AK_##X)
+#define VIRTUAL_ATTR(X)
 #include "swift/AST/Attr.def"
                .Default(AK_Count);
   
@@ -307,6 +310,7 @@ bool Parser::parseTypeAttribute(TypeAttributes &Attributes) {
     StringRef Text = Tok.getText();
     bool isDeclAttribute = false
 #define ATTR(X) || Text == #X
+#define VIRTUAL_ATTR(X)
 #include "swift/AST/Attr.def"
     ;
     
@@ -625,6 +629,10 @@ ParserStatus Parser::parseDecl(SmallVectorImpl<Decl*> &Entries,
                                   StructureMarkerKind::Declaration);
 
   DeclAttributes Attributes;
+  if (Tok.hasComment()) {
+    Attributes.CommentRange = Tok.getCommentRange();
+    Attributes.setAttr(AK_raw_doc_comment, SourceLoc());
+  }
   parseDeclAttributeList(Attributes);
 
   // If we see the 'static', 'class' or 'mutating' followed by a declaration
@@ -815,7 +823,7 @@ ParserResult<ImportDecl> Parser::parseDeclImport(ParseDeclOptions Flags,
   
   bool Exported = Attributes.isExported();
   Attributes.clearAttribute(AK_exported);
-  if (!Attributes.empty())
+  if (Attributes.hasNonVirtualAttributes())
     diagnose(Attributes.AtLoc, diag::import_attributes);
 
   if (!(Flags & PD_AllowTopLevel)) {
@@ -986,8 +994,8 @@ parseIdentifierDeclName(Parser &P, Identifier &Result, SourceLoc &L,
 ///   extension:
 ///    'extension' attribute-list type-identifier inheritance? '{' decl* '}'
 /// \endverbatim
-ParserResult<ExtensionDecl> Parser::parseDeclExtension(ParseDeclOptions Flags,
+ParserResult<ExtensionDecl>
-                                                       DeclAttributes &Attr) {
+Parser::parseDeclExtension(ParseDeclOptions Flags, DeclAttributes &Attributes) {
   SourceLoc ExtensionLoc = consumeToken(tok::kw_extension);
 
   ParserResult<TypeRepr> Ty = parseTypeIdentifierWithRecovery(
@@ -1020,8 +1028,8 @@ ParserResult<ExtensionDecl> Parser::parseDeclExtension(ParseDeclOptions Flags,
     = new (Context) ExtensionDecl(ExtensionLoc, Ty.get(),
                                   Context.AllocateCopy(Inherited),
                                   CurDeclContext);
-  if (Attr.isValid())
+  if (Attributes.shouldSaveInAST())
-    ED->getMutableAttrs() = Attr;
+    ED->getMutableAttrs() = Attributes;
 
   SmallVector<Decl*, 8> MemberDecls;
   SourceLoc LBLoc, RBLoc;
@@ -1082,7 +1090,7 @@ ParserResult<TypeDecl> Parser::parseDeclTypeAlias(bool WantDefinition,
   SourceLoc IdLoc;
   ParserStatus Status;
   
-  if (!Attributes.empty())
+  if (Attributes.hasNonVirtualAttributes())
     diagnose(Attributes.AtLoc, diag::typealias_attributes);
   
 
@@ -1115,6 +1123,8 @@ ParserResult<TypeDecl> Parser::parseDeclTypeAlias(bool WantDefinition,
                                      CurDeclContext,
                                      TypeAliasLoc, Id, IdLoc,
                                      UnderlyingTy.getPtrOrNull());
+    if (Attributes.shouldSaveInAST())
+      assocType->getMutableAttrs() = Attributes;
     if (!Inherited.empty())
       assocType->setInherited(Context.AllocateCopy(Inherited));
     addToScope(assocType);
@@ -1126,6 +1136,8 @@ ParserResult<TypeDecl> Parser::parseDeclTypeAlias(bool WantDefinition,
     new (Context) TypeAliasDecl(TypeAliasLoc, Id, IdLoc,
                                 UnderlyingTy.getPtrOrNull(),
                                 CurDeclContext);
+  if (Attributes.shouldSaveInAST())
+    TAD->getMutableAttrs() = Attributes;
   addToScope(TAD);
   return makeParserResult(Status, TAD);
 }
@@ -1314,7 +1326,7 @@ bool Parser::parseGetSet(ParseDeclOptions Flags, Pattern *Indices,
       // Set up a function declaration.
       TheDecl = createAccessorFunc(Loc, ValueNamePattern, ElementTy, Indices,
                                    StaticLoc, Flags, Kind, this);
-      if (Attributes.isValid())
+      if (Attributes.shouldSaveInAST())
         TheDecl->getMutableAttrs() = Attributes;
       
       Decls.push_back(TheDecl);
@@ -1392,7 +1404,7 @@ bool Parser::parseGetSet(ParseDeclOptions Flags, Pattern *Indices,
     TheDecl = createAccessorFunc(Loc, ValueNamePattern, ElementTy, Indices,
                                  StaticLoc, Flags, Kind, this);
 
-    if (Attributes.isValid())
+    if (Attributes.shouldSaveInAST())
       TheDecl->getMutableAttrs() = Attributes;
 
     // Parse the body.
@@ -1726,7 +1738,7 @@ ParserStatus Parser::parseDeclVar(ParseDeclOptions Flags,
     pattern.get()->forEachVariable([&](VarDecl *VD) {
       VD->setStatic(StaticLoc.isValid());
       VD->setParentPattern(PBD);
-      if (Attributes.isValid())
+      if (Attributes.shouldSaveInAST())
         VD->getMutableAttrs() = Attributes;
       
       Decls.push_back(VD);
@@ -1853,7 +1865,8 @@ Parser::parseDeclFunc(SourceLoc StaticLoc, StaticSpellingKind StaticSpelling,
   // If the 'mutating' modifier was applied to the func, model it as if the
   // @mutating attribute were specified.
   if (MutatingLoc.isValid()) {
-    if (!Attributes.isValid()) Attributes.AtLoc = MutatingLoc;
+    if (!Attributes.AtLoc.isValid())
+      Attributes.AtLoc = MutatingLoc;
     Attributes.setAttr(AK_mutating, MutatingLoc);
   }
 
@@ -1982,7 +1995,7 @@ Parser::parseDeclFunc(SourceLoc StaticLoc, StaticSpellingKind StaticSpelling,
   // Exit the scope introduced for the generic parameters.
   GenericsScope.reset();
 
-  if (Attributes.isValid())
+  if (Attributes.shouldSaveInAST())
     FD->getMutableAttrs() = Attributes;
   addToScope(FD);
   return makeParserResult(FD);
@@ -2067,7 +2080,7 @@ ParserResult<EnumDecl> Parser::parseDeclEnum(ParseDeclOptions Flags,
                                         { }, GenericParams, CurDeclContext);
   setLocalDiscriminator(UD);
 
-  if (Attributes.isValid())
+  if (Attributes.shouldSaveInAST())
     UD->getMutableAttrs() = Attributes;
 
   // Parse optional inheritance clause within the context of the enum.
@@ -2321,7 +2334,7 @@ ParserResult<StructDecl> Parser::parseDeclStruct(ParseDeclOptions Flags,
                                             CurDeclContext);
   setLocalDiscriminator(SD);
 
-  if (Attributes.isValid())
+  if (Attributes.shouldSaveInAST())
     SD->getMutableAttrs() = Attributes;
 
   // Parse optional inheritance clause within the context of the struct.
@@ -2399,7 +2412,7 @@ ParserResult<ClassDecl> Parser::parseDeclClass(ParseDeclOptions Flags,
   setLocalDiscriminator(CD);
 
   // Attach attributes.
-  if (Attributes.isValid())
+  if (Attributes.shouldSaveInAST())
     CD->getMutableAttrs() = Attributes;
 
   // Parse optional inheritance clause within the context of the class.
@@ -2479,7 +2492,7 @@ parseDeclProtocol(ParseDeclOptions Flags, DeclAttributes &Attributes) {
                                  Context.AllocateCopy(InheritedProtocols));
   // No need to setLocalDiscriminator: protocols can't appear in local contexts.
 
-  if (Attributes.isValid())
+  if (Attributes.shouldSaveInAST())
     Proto->getMutableAttrs() = Attributes;
 
   ContextChange CC(*this, Proto);
@@ -2567,7 +2580,7 @@ ParserStatus Parser::parseDeclSubscript(ParseDeclOptions Flags,
                                                 SubscriptLoc, Indices.get(),
                                                 ArrowLoc, ElementTy.get(),
                                                 CurDeclContext);
-  if (Attributes.isValid())
+  if (Attributes.shouldSaveInAST())
     Subscript->getMutableAttrs() = Attributes;
   
   Decls.push_back(Subscript);
@@ -2733,7 +2746,7 @@ Parser::parseDeclConstructor(ParseDeclOptions Flags,
     }
   }
 
-  if (Attributes.isValid())
+  if (Attributes.shouldSaveInAST())
     CD->getMutableAttrs() = Attributes;
 
   return makeParserResult(CD);
@@ -2815,7 +2828,7 @@ parseDeclDestructor(ParseDeclOptions Flags, DeclAttributes &Attributes) {
     }
   }
 
-  if (Attributes.isValid())
+  if (Attributes.shouldSaveInAST())
     DD->getMutableAttrs() = Attributes;
 
   // Reject 'destructor' functions outside of classes
@@ -2836,7 +2849,7 @@ ParserResult<OperatorDecl> Parser::parseDeclOperator(bool AllowTopLevel,
 
   SourceLoc OperatorLoc = consumeToken(tok::identifier);
 
-  if (!Attributes.empty())
+  if (Attributes.hasNonVirtualAttributes())
     diagnose(Attributes.AtLoc, diag::operator_attributes);
   
   auto kind = llvm::StringSwitch<Optional<DeclKind>>(Tok.getText())

lib/Parse/Parser.cpp

@@ -222,7 +222,9 @@ std::vector<Token> swift::tokenize(const LangOptions &LangOpts,
     EndOffset = Buffer->getBufferSize();
 
   Lexer L(LangOpts, SM, BufferID, /*Diags=*/nullptr, /*InSILMode=*/false,
-          KeepComments, Offset, EndOffset);
+          KeepComments ? CommentRetentionMode::ReturnAsTokens
+                       : CommentRetentionMode::AttachToNextToken,
+          Offset, EndOffset);
   std::vector<Token> Tokens;
   do {
     Tokens.emplace_back();
@@ -249,7 +251,10 @@ Parser::Parser(unsigned BufferID, SourceFile &SF, SILParserState *SIL,
     SF(SF),
     L(new Lexer(SF.getASTContext().LangOpts, SF.getASTContext().SourceMgr,
                 BufferID, &SF.getASTContext().Diags,
-                /*InSILMode=*/SIL != nullptr, /*KeepComments=*/false)),
+                /*InSILMode=*/SIL != nullptr,
+                SF.getASTContext().LangOpts.AttachCommentsToDecls
+                    ? CommentRetentionMode::AttachToNextToken
+                    : CommentRetentionMode::None)),
     SIL(SIL),
     Context(SF.getASTContext()) {
 

lib/Serialization/Serialization.cpp

@@ -1050,10 +1050,14 @@ template <AttrKind ...KINDS>
 static void checkAllowedAttributes(const Decl *D) {
 #ifndef NDEBUG
   DeclAttributes attrs = D->getAttrs();
-  for (AttrKind AK : { KINDS... })
+  for (AttrKind AK : {
+#define ATTR(X)
+#define VIRTUAL_ATTR(X) AK_ ## X,
+#include "swift/AST/Attr.def"
+      KINDS... })
     attrs.clearAttribute(AK);
 
-  if (!attrs.empty()) {
+  if (!attrs.isEmpty()) {
     llvm::errs() << "Serialization: unhandled attributes ";
     attrs.print(llvm::errs());
     llvm::errs() << "\n";
@@ -1062,11 +1066,6 @@ static void checkAllowedAttributes(const Decl *D) {
 #endif
 }
 
-template<>
-void checkAllowedAttributes(const Decl *D) {
-  assert(D->getAttrs().empty());
-}
-
 void Serializer::writeDecl(const Decl *D) {
   using namespace decls_block;
 

test/IDE/comment_attach.swift

@@ -0,0 +1,264 @@
+// NOTE: This file is sensitive to line numbers.  Thus RUN and CHECK lines come
+// below the code.
+//
+// NOTE: Please don't change this file to use FileCheck's feature to match
+// relative line numbers: those lines are comments and we don't want to see
+// anything extra in a test for documentation comments.
+
+//===--- Check that we attach comments to all kinds of declarations.
+
+// Ignore comments above.
+func eatComments1() {}
+
+// decl_func_1 Aaa.
+func decl_func_1() {}
+
+// decl_func_2 Aaa.
+func decl_func_2<T>(t: T) {}
+
+// decl_func_3 Aaa.
+@unknown_attribute
+func decl_func_3() {}
+
+
+// decl_var_1 Aaa.
+var decl_var_1: Int
+
+// decl_var_2 decl_var_3 Aaa.
+var (decl_var_2, decl_var_3): (Int, Float)
+
+// decl_var_4 Aaa.
+@unknown_attribute
+var decl_var_4: Int
+
+// decl_var_5 Aaa.
+var decl_var_5: Int {
+get: return 0
+set:
+}
+
+var decl_var_6: Int {
+// decl_var_6_get Aaa.
+get: return 0
+// decl_var_6_set Aaa.
+set:
+}
+
+// decl_var_7 Aaa.
+var decl_var_7: Int = 0 {
+willSet:
+didSet:
+}
+
+var decl_var_8: Int = 0 {
+// decl_var_8_get Aaa.
+willSet:
+// decl_var_8_set Aaa.
+didSet:
+}
+
+
+// decl_val_1 Aaa.
+val decl_val_1: Int = 0
+
+// decl_val_2 decl_val_3 Aaa.
+val (decl_val_2, decl_val_3): (Int, Float) = (0, 0.0)
+
+// decl_val_4 Aaa.
+@unknown_attribute
+val decl_val_4: Int = 0
+
+
+// decl_typealias_1 Aaa.
+typealias decl_typealias_1 = Int
+
+// decl_struct_1 Aaa.
+struct decl_struct_1 {
+  // instanceVar1 Aaa.
+  var instanceVar1: Int
+
+  // instanceVar2 instanceVar3 Aaa.
+  var (instanceVar2, instanceVar3): (Int, Float)
+
+  // instanceVar4 Aaa.
+  @unknown_attribute
+  var instanceVar4: Int
+
+  // instanceVar5 Aaa.
+  var instanceVar5: Int {
+  get: return 0
+  set:
+  }
+
+  var instanceVar6: Int {
+  // instanceVar6_get Aaa.
+  get: return 0
+  // instanceVar6_set Aaa.
+  set:
+  }
+
+  // instanceVar7 Aaa.
+  var instanceVar7: Int = 0 {
+  willSet:
+  didSet:
+  }
+
+  var instanceVar8: Int = 0 {
+  // instanceVar8_get Aaa.
+  willSet:
+  // instanceVar8_set Aaa.
+  didSet:
+  }
+
+
+  // instanceFunc1 Aaa.
+  func instanceFunc1() {}
+
+  // instanceFunc2 Aaa.
+  @mutable
+  func instanceFunc2() {}
+
+  // instanceFunc3 Aaa.
+  func instanceFunc3<T>(t: T) {}
+
+  // instanceFunc4 Aaa.
+  @unknown_attribute
+  func instanceFunc4() {}
+
+
+  // subscript Aaa.
+  subscript(i: Int) -> Double { return 0.0 }
+
+  // NestedStruct Aaa.
+  struct NestedStruct {}
+
+  // NestedClass Aaa.
+  class NestedClass {}
+
+  // NestedEnum Aaa.
+  enum NestedEnum {}
+
+  // Can not declare a nested protocol.
+  // protocol NestedProtocol {}
+  func eatComments2() {}
+
+  // NestedTypealias Aaa.
+  typealias NestedTypealias = Int
+
+  // staticVar Aaa.
+  static var staticVar: Int = 4
+
+  // staticFunc1 Aaa.
+  static func staticFunc1() {}
+}
+
+// decl_enum_1 Aaa.
+enum decl_enum_1 {
+  // Case1 Aaa.
+  case Case1
+
+  // Case2 Aaa.
+  case Case2(Int)
+
+  // Case3 Aaa.
+  case Case3(Int, Float)
+
+  // Case4 Case5 Aaa.
+  case Case4, Case5
+}
+
+// decl_class_1 Aaa.
+class decl_class_1 {
+}
+
+// decl_protocol_1 Aaa.
+protocol decl_protocol_1 {
+  // NestedTypealias Aaa.
+  typealias NestedTypealias
+}
+
+// FIXME: While there is nothing stopping us from attaching comments to
+// extensions, how would we use those comments?
+
+// decl_extension_1 Aaa.
+extension decl_extension_1 {
+}
+
+//===--- Check how we merge consecutive comments.
+
+// RUN: %swift-ide-test -print-comments -source-filename %s | FileCheck %s
+
+// CHECK: comment_attach.swift:11:6: Func/eatComments1 RawComment=[{{.*}}]
+// CHECK-NEXT: comment_attach.swift:14:6: Func/decl_func_1 RawComment=[// decl_func_1 Aaa.]
+// CHECK-NEXT: comment_attach.swift:17:6: Func/decl_func_2 RawComment=[// decl_func_2 Aaa.]
+// CHECK-NEXT: comment_attach.swift:17:21: Var/t RawComment=none
+// CHECK-NEXT: comment_attach.swift:21:6: Func/decl_func_3 RawComment=[// decl_func_3 Aaa.]
+// CHECK-NEXT: comment_attach.swift:25:5: Var/decl_var_1 RawComment=[// decl_var_1 Aaa.]
+// CHECK-NEXT: comment_attach.swift:28:6: Var/decl_var_2 RawComment=[// decl_var_2 decl_var_3 Aaa.]
+// CHECK-NEXT: comment_attach.swift:28:18: Var/decl_var_3 RawComment=[// decl_var_2 decl_var_3 Aaa.]
+// CHECK-NEXT: comment_attach.swift:32:5: Var/decl_var_4 RawComment=[// decl_var_4 Aaa.]
+// CHECK-NEXT: comment_attach.swift:35:5: Var/decl_var_5 RawComment=[// decl_var_5 Aaa.]
+// CHECK-NEXT: comment_attach.swift:36:1: Func/<getter for decl_var_5> RawComment=none
+// CHECK-NEXT: comment_attach.swift:37:1: Func/<setter for decl_var_5> RawComment=none
+// CHECK-NEXT: comment_attach.swift:40:5: Var/decl_var_6 RawComment=none
+// CHECK-NEXT: comment_attach.swift:42:1: Func/<getter for decl_var_6> RawComment=none
+// CHECK-NEXT: comment_attach.swift:44:1: Func/<setter for decl_var_6> RawComment=none
+// CHECK-NEXT: comment_attach.swift:48:5: Var/decl_var_7 RawComment=[// decl_var_7 Aaa.]
+// CHECK-NEXT: comment_attach.swift:49:1: Func/<willSet for decl_var_7> RawComment=none
+// CHECK-NEXT: comment_attach.swift:50:1: Func/<didSet for decl_var_7> RawComment=none
+// CHECK-NEXT: comment_attach.swift:53:5: Var/decl_var_8 RawComment=none
+// CHECK-NEXT: comment_attach.swift:55:1: Func/<willSet for decl_var_8> RawComment=none
+// CHECK-NEXT: comment_attach.swift:57:1: Func/<didSet for decl_var_8> RawComment=none
+// CHECK-NEXT: comment_attach.swift:62:5: Var/decl_val_1 RawComment=[// decl_val_1 Aaa.]
+// CHECK-NEXT: comment_attach.swift:65:6: Var/decl_val_2 RawComment=[// decl_val_2 decl_val_3 Aaa.]
+// CHECK-NEXT: comment_attach.swift:65:18: Var/decl_val_3 RawComment=[// decl_val_2 decl_val_3 Aaa.]
+// CHECK-NEXT: comment_attach.swift:69:5: Var/decl_val_4 RawComment=[// decl_val_4 Aaa.]
+// CHECK-NEXT: comment_attach.swift:73:11: TypeAlias/decl_typealias_1 RawComment=[// decl_typealias_1 Aaa.]
+// CHECK-NEXT: comment_attach.swift:76:8: Struct/decl_struct_1 RawComment=[// decl_struct_1 Aaa.]
+// CHECK-NEXT: comment_attach.swift:78:7: Var/instanceVar1 RawComment=[// instanceVar1 Aaa.]
+// CHECK-NEXT: comment_attach.swift:81:8: Var/instanceVar2 RawComment=[// instanceVar2 instanceVar3 Aaa.]
+// CHECK-NEXT: comment_attach.swift:81:22: Var/instanceVar3 RawComment=[// instanceVar2 instanceVar3 Aaa.]
+// CHECK-NEXT: comment_attach.swift:85:7: Var/instanceVar4 RawComment=[// instanceVar4 Aaa.]
+// CHECK-NEXT: comment_attach.swift:88:7: Var/instanceVar5 RawComment=[// instanceVar5 Aaa.]
+// CHECK-NEXT: comment_attach.swift:89:3: Func/<getter for instanceVar5> RawComment=none
+// CHECK-NEXT: comment_attach.swift:90:3: Func/<setter for instanceVar5> RawComment=none
+// CHECK-NEXT: comment_attach.swift:93:7: Var/instanceVar6 RawComment=none
+// CHECK-NEXT: comment_attach.swift:95:3: Func/<getter for instanceVar6> RawComment=none
+// CHECK-NEXT: comment_attach.swift:97:3: Func/<setter for instanceVar6> RawComment=none
+// CHECK-NEXT: comment_attach.swift:101:7: Var/instanceVar7 RawComment=[// instanceVar7 Aaa.]
+// CHECK-NEXT: comment_attach.swift:102:3: Func/<willSet for instanceVar7> RawComment=none
+// CHECK-NEXT: comment_attach.swift:103:3: Func/<didSet for instanceVar7> RawComment=none
+// CHECK-NEXT: comment_attach.swift:106:7: Var/instanceVar8 RawComment=none
+// CHECK-NEXT: comment_attach.swift:108:3: Func/<willSet for instanceVar8> RawComment=none
+// CHECK-NEXT: comment_attach.swift:110:3: Func/<didSet for instanceVar8> RawComment=none
+// CHECK-NEXT: comment_attach.swift:115:8: Func/instanceFunc1 RawComment=[// instanceFunc1 Aaa.]
+// CHECK-NEXT: comment_attach.swift:119:8: Func/instanceFunc2 RawComment=[// instanceFunc2 Aaa.]
+// CHECK-NEXT: comment_attach.swift:122:8: Func/instanceFunc3 RawComment=[// instanceFunc3 Aaa.]
+// CHECK-NEXT: comment_attach.swift:122:25: Var/t RawComment=none
+// CHECK-NEXT: comment_attach.swift:126:8: Func/instanceFunc4 RawComment=[// instanceFunc4 Aaa.]
+// CHECK-NEXT: comment_attach.swift:130:3: Subscript/subscript RawComment=[// subscript Aaa.]
+// CHECK-NEXT: comment_attach.swift:130:13: Var/i RawComment=none
+// CHECK-NEXT: comment_attach.swift:130:33: Func/<getter for subscript> RawComment=none
+// CHECK-NEXT: comment_attach.swift:133:10: Struct/NestedStruct RawComment=[// NestedStruct Aaa.]
+// CHECK-NEXT: comment_attach.swift:136:9: Class/NestedClass RawComment=[// NestedClass Aaa.]
+// CHECK-NEXT: comment_attach.swift:139:8: Enum/NestedEnum RawComment=[// NestedEnum Aaa.]
+// CHECK-NEXT: comment_attach.swift:143:8: Func/eatComments2 RawComment=[{{.*}}]
+// CHECK-NEXT: comment_attach.swift:146:13: TypeAlias/NestedTypealias RawComment=[// NestedTypealias Aaa.]
+// CHECK-NEXT: comment_attach.swift:149:14: Var/staticVar RawComment=[// staticVar Aaa.]
+// CHECK-NEXT: comment_attach.swift:152:15: Func/staticFunc1 RawComment=[// staticFunc1 Aaa.]
+// CHECK-NEXT: comment_attach.swift:76:8: Var/instanceVar1 RawComment=none
+// CHECK-NEXT: comment_attach.swift:76:8: Var/instanceVar2 RawComment=none
+// CHECK-NEXT: comment_attach.swift:76:8: Var/instanceVar3 RawComment=none
+// CHECK-NEXT: comment_attach.swift:76:8: Var/instanceVar4 RawComment=none
+// CHECK-NEXT: comment_attach.swift:76:8: Var/instanceVar7 RawComment=none
+// CHECK-NEXT: comment_attach.swift:76:8: Var/instanceVar8 RawComment=none
+// CHECK-NEXT: comment_attach.swift:156:6: Enum/decl_enum_1 RawComment=[// decl_enum_1 Aaa.]
+// CHECK-NEXT: comment_attach.swift:158:8: EnumElement/Case1 RawComment=[// Case1 Aaa.]
+// CHECK-NEXT: comment_attach.swift:161:8: EnumElement/Case2 RawComment=[// Case2 Aaa.]
+// CHECK-NEXT: comment_attach.swift:164:8: EnumElement/Case3 RawComment=[// Case3 Aaa.]
+// CHECK-NEXT: comment_attach.swift:167:8: EnumElement/Case4 RawComment=[// Case4 Case5 Aaa.]
+// CHECK-NEXT: comment_attach.swift:167:15: EnumElement/Case5 RawComment=[// Case4 Case5 Aaa.]
+// CHECK-NEXT: comment_attach.swift:171:7: Class/decl_class_1 RawComment=[// decl_class_1 Aaa.]
+// CHECK-NEXT: comment_attach.swift:175:10: Protocol/decl_protocol_1 RawComment=[// decl_protocol_1 Aaa.]
+// CHECK-NEXT: comment_attach.swift:177:13: AssociatedType/NestedTypealias RawComment=[// NestedTypealias Aaa.]
+