Skip to content
Snippets Groups Projects
Commit 42572f53 authored by Benjamin Kramer's avatar Benjamin Kramer
Browse files

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
parent 5c805e92
No related branches found
No related tags found
No related merge requests found
...@@ -107,12 +107,9 @@ public: ...@@ -107,12 +107,9 @@ public:
return RawText; return RawText;
} }
SourceRange getSourceRange() const LLVM_READONLY { SourceRange getSourceRange() const LLVM_READONLY { return Range; }
return Range; SourceLocation getLocStart() const LLVM_READONLY { return Range.getBegin(); }
} SourceLocation getLocEnd() const LLVM_READONLY { return Range.getEnd(); }
unsigned getBeginLine(const SourceManager &SM) const;
unsigned getEndLine(const SourceManager &SM) const;
const char *getBriefText(const ASTContext &Context) const { const char *getBriefText(const ASTContext &Context) const {
if (BriefTextValid) if (BriefTextValid)
...@@ -146,11 +143,6 @@ private: ...@@ -146,11 +143,6 @@ private:
/// considered as documentation comments. /// considered as documentation comments.
bool ParseAllComments : 1; 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. /// \brief Constructor for AST deserialization.
RawComment(SourceRange SR, CommentKind K, bool IsTrailingComment, RawComment(SourceRange SR, CommentKind K, bool IsTrailingComment,
bool IsAlmostTrailingComment, bool IsAlmostTrailingComment,
...@@ -158,8 +150,7 @@ private: ...@@ -158,8 +150,7 @@ private:
Range(SR), RawTextValid(false), BriefTextValid(false), Kind(K), Range(SR), RawTextValid(false), BriefTextValid(false), Kind(K),
IsAttached(false), IsTrailingComment(IsTrailingComment), IsAttached(false), IsTrailingComment(IsTrailingComment),
IsAlmostTrailingComment(IsAlmostTrailingComment), IsAlmostTrailingComment(IsAlmostTrailingComment),
ParseAllComments(ParseAllComments), ParseAllComments(ParseAllComments)
BeginLineValid(false), EndLineValid(false)
{ } { }
StringRef getRawTextSlow(const SourceManager &SourceMgr) const; StringRef getRawTextSlow(const SourceManager &SourceMgr) const;
...@@ -178,8 +169,7 @@ public: ...@@ -178,8 +169,7 @@ public:
explicit BeforeThanCompare(const SourceManager &SM) : SM(SM) { } explicit BeforeThanCompare(const SourceManager &SM) : SM(SM) { }
bool operator()(const RawComment &LHS, const RawComment &RHS) { bool operator()(const RawComment &LHS, const RawComment &RHS) {
return SM.isBeforeInTranslationUnit(LHS.getSourceRange().getBegin(), return SM.isBeforeInTranslationUnit(LHS.getLocStart(), RHS.getLocStart());
RHS.getSourceRange().getBegin());
} }
bool operator()(const RawComment *LHS, const RawComment *RHS) { bool operator()(const RawComment *LHS, const RawComment *RHS) {
...@@ -191,8 +181,7 @@ public: ...@@ -191,8 +181,7 @@ public:
/// sorted in order of appearance in the translation unit. /// sorted in order of appearance in the translation unit.
class RawCommentList { class RawCommentList {
public: public:
RawCommentList(SourceManager &SourceMgr) : RawCommentList(SourceManager &SourceMgr) : SourceMgr(SourceMgr) {}
SourceMgr(SourceMgr), OnlyWhitespaceSeen(true) { }
void addComment(const RawComment &RC, llvm::BumpPtrAllocator &Allocator); void addComment(const RawComment &RC, llvm::BumpPtrAllocator &Allocator);
...@@ -203,15 +192,9 @@ public: ...@@ -203,15 +192,9 @@ public:
private: private:
SourceManager &SourceMgr; SourceManager &SourceMgr;
std::vector<RawComment *> Comments; std::vector<RawComment *> Comments;
SourceLocation PrevCommentEndLoc;
bool OnlyWhitespaceSeen;
void addCommentsToFront(const std::vector<RawComment *> &C) { void addCommentsToFront(const std::vector<RawComment *> &C) {
size_t OldSize = Comments.size(); Comments.insert(Comments.begin(), C.begin(), C.end());
Comments.resize(C.size() + OldSize);
std::copy_backward(Comments.begin(), Comments.begin() + OldSize,
Comments.end());
std::copy(C.begin(), C.end(), Comments.begin());
} }
friend class ASTReader; friend class ASTReader;
......
...@@ -68,8 +68,7 @@ RawComment::RawComment(const SourceManager &SourceMgr, SourceRange SR, ...@@ -68,8 +68,7 @@ RawComment::RawComment(const SourceManager &SourceMgr, SourceRange SR,
bool Merged, bool ParseAllComments) : bool Merged, bool ParseAllComments) :
Range(SR), RawTextValid(false), BriefTextValid(false), Range(SR), RawTextValid(false), BriefTextValid(false),
IsAttached(false), IsAlmostTrailingComment(false), IsAttached(false), IsAlmostTrailingComment(false),
ParseAllComments(ParseAllComments), ParseAllComments(ParseAllComments) {
BeginLineValid(false), EndLineValid(false) {
// Extract raw comment text, if possible. // Extract raw comment text, if possible.
if (SR.getBegin() == SR.getEnd() || getRawText(SourceMgr).empty()) { if (SR.getBegin() == SR.getEnd() || getRawText(SourceMgr).empty()) {
Kind = RCK_Invalid; Kind = RCK_Invalid;
...@@ -90,26 +89,6 @@ RawComment::RawComment(const SourceManager &SourceMgr, SourceRange SR, ...@@ -90,26 +89,6 @@ RawComment::RawComment(const SourceManager &SourceMgr, SourceRange SR,
} }
} }
unsigned RawComment::getBeginLine(const SourceManager &SM) const {
if (BeginLineValid)
return BeginLine;
std::pair<FileID, unsigned> 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<FileID, unsigned> LocInfo = SM.getDecomposedLoc(Range.getEnd());
EndLine = SM.getLineNumber(LocInfo.first, LocInfo.second);
EndLineValid = true;
return EndLine;
}
StringRef RawComment::getRawTextSlow(const SourceManager &SourceMgr) const { StringRef RawComment::getRawTextSlow(const SourceManager &SourceMgr) const {
FileID BeginFileID; FileID BeginFileID;
FileID EndFileID; FileID EndFileID;
...@@ -184,13 +163,9 @@ comments::FullComment *RawComment::parse(const ASTContext &Context, ...@@ -184,13 +163,9 @@ comments::FullComment *RawComment::parse(const ASTContext &Context,
return P.parseFullComment(); return P.parseFullComment();
} }
namespace { static bool onlyWhitespaceBetween(SourceManager &SM,
bool containsOnlyWhitespace(StringRef Str) { SourceLocation Loc1, SourceLocation Loc2,
return Str.find_first_not_of(" \t\f\v\r\n") == StringRef::npos; unsigned MaxNewlinesAllowed) {
}
bool onlyWhitespaceBetween(SourceManager &SM,
SourceLocation Loc1, SourceLocation Loc2) {
std::pair<FileID, unsigned> Loc1Info = SM.getDecomposedLoc(Loc1); std::pair<FileID, unsigned> Loc1Info = SM.getDecomposedLoc(Loc1);
std::pair<FileID, unsigned> Loc2Info = SM.getDecomposedLoc(Loc2); std::pair<FileID, unsigned> Loc2Info = SM.getDecomposedLoc(Loc2);
...@@ -203,10 +178,38 @@ bool onlyWhitespaceBetween(SourceManager &SM, ...@@ -203,10 +178,38 @@ bool onlyWhitespaceBetween(SourceManager &SM,
if (Invalid) if (Invalid)
return false; return false;
StringRef Text(Buffer + Loc1Info.second, Loc2Info.second - Loc1Info.second); unsigned NumNewlines = 0;
return containsOnlyWhitespace(Text); 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, void RawCommentList::addComment(const RawComment &RC,
llvm::BumpPtrAllocator &Allocator) { llvm::BumpPtrAllocator &Allocator) {
...@@ -215,23 +218,13 @@ void RawCommentList::addComment(const RawComment &RC, ...@@ -215,23 +218,13 @@ void RawCommentList::addComment(const RawComment &RC,
// Check if the comments are not in source order. // Check if the comments are not in source order.
while (!Comments.empty() && while (!Comments.empty() &&
!SourceMgr.isBeforeInTranslationUnit( !SourceMgr.isBeforeInTranslationUnit(Comments.back()->getLocStart(),
Comments.back()->getSourceRange().getBegin(), RC.getLocStart())) {
RC.getSourceRange().getBegin())) {
// If they are, just pop a few last comments that don't fit. // If they are, just pop a few last comments that don't fit.
// This happens if an \#include directive contains comments. // This happens if an \#include directive contains comments.
Comments.pop_back(); 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. // Ordinary comments are not interesting for us.
if (RC.isOrdinary()) if (RC.isOrdinary())
return; return;
...@@ -240,7 +233,6 @@ void RawCommentList::addComment(const RawComment &RC, ...@@ -240,7 +233,6 @@ void RawCommentList::addComment(const RawComment &RC,
// anything to merge it with). // anything to merge it with).
if (Comments.empty()) { if (Comments.empty()) {
Comments.push_back(new (Allocator) RawComment(RC)); Comments.push_back(new (Allocator) RawComment(RC));
OnlyWhitespaceSeen = true;
return; return;
} }
...@@ -250,21 +242,13 @@ void RawCommentList::addComment(const RawComment &RC, ...@@ -250,21 +242,13 @@ void RawCommentList::addComment(const RawComment &RC,
// Merge comments only if there is only whitespace between them. // Merge comments only if there is only whitespace between them.
// Can't merge trailing and non-trailing comments. // Can't merge trailing and non-trailing comments.
// Merge comments if they are on same or consecutive lines. // Merge comments if they are on same or consecutive lines.
bool Merged = false; if (C1.isTrailingComment() == C2.isTrailingComment() &&
if (OnlyWhitespaceSeen && onlyWhitespaceBetween(SourceMgr, C1.getLocEnd(), C2.getLocStart(),
(C1.isTrailingComment() == C2.isTrailingComment())) { /*MaxNewlinesAllowed=*/1)) {
unsigned C1EndLine = C1.getEndLine(SourceMgr); SourceRange MergedRange(C1.getLocStart(), C2.getLocEnd());
unsigned C2BeginLine = C2.getBeginLine(SourceMgr); *Comments.back() = RawComment(SourceMgr, MergedRange, true,
if (C1EndLine + 1 == C2BeginLine || C1EndLine == C2BeginLine) { RC.isParseAllComments());
SourceRange MergedRange(C1.getSourceRange().getBegin(), } else {
C2.getSourceRange().getEnd());
*Comments.back() = RawComment(SourceMgr, MergedRange, true,
RC.isParseAllComments());
Merged = true;
}
}
if (!Merged)
Comments.push_back(new (Allocator) RawComment(RC)); Comments.push_back(new (Allocator) RawComment(RC));
}
OnlyWhitespaceSeen = true;
} }
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment