From: Benjamin Kramer Date: Sat, 28 Sep 2013 15:06:27 +0000 (+0000) Subject: Refactor comment merging. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=42572f532b99230bf7aa3e3593a0fbb1174bce7c;p=clang Refactor comment merging. - We scan for whitespace between comments anyways, remember any newlines seen along the way. - Use this newline number to decide whether two comments are adjacent. - Since the newline check is now free remove the caching and unused code. - Remove unnecessary boolean state from the comment list. - No behavioral change. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@191614 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/AST/RawCommentList.h b/include/clang/AST/RawCommentList.h index 84a6e96fa0..a4fcc108eb 100644 --- a/include/clang/AST/RawCommentList.h +++ b/include/clang/AST/RawCommentList.h @@ -107,12 +107,9 @@ public: return RawText; } - SourceRange getSourceRange() const LLVM_READONLY { - return Range; - } - - unsigned getBeginLine(const SourceManager &SM) const; - unsigned getEndLine(const SourceManager &SM) const; + SourceRange getSourceRange() const LLVM_READONLY { return Range; } + SourceLocation getLocStart() const LLVM_READONLY { return Range.getBegin(); } + SourceLocation getLocEnd() const LLVM_READONLY { return Range.getEnd(); } const char *getBriefText(const ASTContext &Context) const { if (BriefTextValid) @@ -146,11 +143,6 @@ private: /// considered as documentation comments. bool ParseAllComments : 1; - mutable bool BeginLineValid : 1; ///< True if BeginLine is valid - mutable bool EndLineValid : 1; ///< True if EndLine is valid - mutable unsigned BeginLine; ///< Cached line number - mutable unsigned EndLine; ///< Cached line number - /// \brief Constructor for AST deserialization. RawComment(SourceRange SR, CommentKind K, bool IsTrailingComment, bool IsAlmostTrailingComment, @@ -158,8 +150,7 @@ private: Range(SR), RawTextValid(false), BriefTextValid(false), Kind(K), IsAttached(false), IsTrailingComment(IsTrailingComment), IsAlmostTrailingComment(IsAlmostTrailingComment), - ParseAllComments(ParseAllComments), - BeginLineValid(false), EndLineValid(false) + ParseAllComments(ParseAllComments) { } StringRef getRawTextSlow(const SourceManager &SourceMgr) const; @@ -178,8 +169,7 @@ public: explicit BeforeThanCompare(const SourceManager &SM) : SM(SM) { } bool operator()(const RawComment &LHS, const RawComment &RHS) { - return SM.isBeforeInTranslationUnit(LHS.getSourceRange().getBegin(), - RHS.getSourceRange().getBegin()); + return SM.isBeforeInTranslationUnit(LHS.getLocStart(), RHS.getLocStart()); } bool operator()(const RawComment *LHS, const RawComment *RHS) { @@ -191,8 +181,7 @@ public: /// sorted in order of appearance in the translation unit. class RawCommentList { public: - RawCommentList(SourceManager &SourceMgr) : - SourceMgr(SourceMgr), OnlyWhitespaceSeen(true) { } + RawCommentList(SourceManager &SourceMgr) : SourceMgr(SourceMgr) {} void addComment(const RawComment &RC, llvm::BumpPtrAllocator &Allocator); @@ -203,15 +192,9 @@ public: private: SourceManager &SourceMgr; std::vector Comments; - SourceLocation PrevCommentEndLoc; - bool OnlyWhitespaceSeen; void addCommentsToFront(const std::vector &C) { - size_t OldSize = Comments.size(); - Comments.resize(C.size() + OldSize); - std::copy_backward(Comments.begin(), Comments.begin() + OldSize, - Comments.end()); - std::copy(C.begin(), C.end(), Comments.begin()); + Comments.insert(Comments.begin(), C.begin(), C.end()); } friend class ASTReader; diff --git a/lib/AST/RawCommentList.cpp b/lib/AST/RawCommentList.cpp index 92b96dc8e5..1fa7cea1d4 100644 --- a/lib/AST/RawCommentList.cpp +++ b/lib/AST/RawCommentList.cpp @@ -68,8 +68,7 @@ RawComment::RawComment(const SourceManager &SourceMgr, SourceRange SR, bool Merged, bool ParseAllComments) : Range(SR), RawTextValid(false), BriefTextValid(false), IsAttached(false), IsAlmostTrailingComment(false), - ParseAllComments(ParseAllComments), - BeginLineValid(false), EndLineValid(false) { + ParseAllComments(ParseAllComments) { // Extract raw comment text, if possible. if (SR.getBegin() == SR.getEnd() || getRawText(SourceMgr).empty()) { Kind = RCK_Invalid; @@ -90,26 +89,6 @@ RawComment::RawComment(const SourceManager &SourceMgr, SourceRange SR, } } -unsigned RawComment::getBeginLine(const SourceManager &SM) const { - if (BeginLineValid) - return BeginLine; - - std::pair LocInfo = SM.getDecomposedLoc(Range.getBegin()); - BeginLine = SM.getLineNumber(LocInfo.first, LocInfo.second); - BeginLineValid = true; - return BeginLine; -} - -unsigned RawComment::getEndLine(const SourceManager &SM) const { - if (EndLineValid) - return EndLine; - - std::pair LocInfo = SM.getDecomposedLoc(Range.getEnd()); - EndLine = SM.getLineNumber(LocInfo.first, LocInfo.second); - EndLineValid = true; - return EndLine; -} - StringRef RawComment::getRawTextSlow(const SourceManager &SourceMgr) const { FileID BeginFileID; FileID EndFileID; @@ -184,13 +163,9 @@ comments::FullComment *RawComment::parse(const ASTContext &Context, return P.parseFullComment(); } -namespace { -bool containsOnlyWhitespace(StringRef Str) { - return Str.find_first_not_of(" \t\f\v\r\n") == StringRef::npos; -} - -bool onlyWhitespaceBetween(SourceManager &SM, - SourceLocation Loc1, SourceLocation Loc2) { +static bool onlyWhitespaceBetween(SourceManager &SM, + SourceLocation Loc1, SourceLocation Loc2, + unsigned MaxNewlinesAllowed) { std::pair Loc1Info = SM.getDecomposedLoc(Loc1); std::pair Loc2Info = SM.getDecomposedLoc(Loc2); @@ -203,10 +178,38 @@ bool onlyWhitespaceBetween(SourceManager &SM, if (Invalid) return false; - StringRef Text(Buffer + Loc1Info.second, Loc2Info.second - Loc1Info.second); - return containsOnlyWhitespace(Text); + unsigned NumNewlines = 0; + assert(Loc1Info.second <= Loc2Info.second && "Loc1 after Loc2!"); + // Look for non-whitespace characters and remember any newlines seen. + for (unsigned I = Loc1Info.second; I != Loc2Info.second; ++I) { + switch (Buffer[I]) { + default: + return false; + case ' ': + case '\t': + case '\f': + case '\v': + break; + case '\r': + case '\n': + ++NumNewlines; + + // Check if we have found more than the maximum allowed number of + // newlines. + if (NumNewlines > MaxNewlinesAllowed) + return false; + + // Collapse \r\n and \n\r into a single newline. + if (I + 1 != Loc2Info.second && + (Buffer[I + 1] == '\n' || Buffer[I + 1] == '\r') && + Buffer[I] != Buffer[I + 1]) + ++I; + break; + } + } + + return true; } -} // unnamed namespace void RawCommentList::addComment(const RawComment &RC, llvm::BumpPtrAllocator &Allocator) { @@ -215,23 +218,13 @@ void RawCommentList::addComment(const RawComment &RC, // Check if the comments are not in source order. while (!Comments.empty() && - !SourceMgr.isBeforeInTranslationUnit( - Comments.back()->getSourceRange().getBegin(), - RC.getSourceRange().getBegin())) { + !SourceMgr.isBeforeInTranslationUnit(Comments.back()->getLocStart(), + RC.getLocStart())) { // If they are, just pop a few last comments that don't fit. // This happens if an \#include directive contains comments. Comments.pop_back(); } - if (OnlyWhitespaceSeen) { - if (!onlyWhitespaceBetween(SourceMgr, - PrevCommentEndLoc, - RC.getSourceRange().getBegin())) - OnlyWhitespaceSeen = false; - } - - PrevCommentEndLoc = RC.getSourceRange().getEnd(); - // Ordinary comments are not interesting for us. if (RC.isOrdinary()) return; @@ -240,7 +233,6 @@ void RawCommentList::addComment(const RawComment &RC, // anything to merge it with). if (Comments.empty()) { Comments.push_back(new (Allocator) RawComment(RC)); - OnlyWhitespaceSeen = true; return; } @@ -250,21 +242,13 @@ void RawCommentList::addComment(const RawComment &RC, // Merge comments only if there is only whitespace between them. // Can't merge trailing and non-trailing comments. // Merge comments if they are on same or consecutive lines. - bool Merged = false; - if (OnlyWhitespaceSeen && - (C1.isTrailingComment() == C2.isTrailingComment())) { - unsigned C1EndLine = C1.getEndLine(SourceMgr); - unsigned C2BeginLine = C2.getBeginLine(SourceMgr); - if (C1EndLine + 1 == C2BeginLine || C1EndLine == C2BeginLine) { - SourceRange MergedRange(C1.getSourceRange().getBegin(), - C2.getSourceRange().getEnd()); - *Comments.back() = RawComment(SourceMgr, MergedRange, true, - RC.isParseAllComments()); - Merged = true; - } - } - if (!Merged) + if (C1.isTrailingComment() == C2.isTrailingComment() && + onlyWhitespaceBetween(SourceMgr, C1.getLocEnd(), C2.getLocStart(), + /*MaxNewlinesAllowed=*/1)) { + SourceRange MergedRange(C1.getLocStart(), C2.getLocEnd()); + *Comments.back() = RawComment(SourceMgr, MergedRange, true, + RC.isParseAllComments()); + } else { Comments.push_back(new (Allocator) RawComment(RC)); - - OnlyWhitespaceSeen = true; + } }