Skip to content

[Gardening] Do not store start/end line in SingleRawComment #36920

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 16, 2021
Merged

[Gardening] Do not store start/end line in SingleRawComment #36920

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions include/swift/AST/RawComment.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,10 @@ struct SingleRawComment {
StringRef RawText;

unsigned Kind : 8;
unsigned StartColumn : 16;
unsigned StartLine;
unsigned EndLine;
unsigned ColumnIndent : 16;

SingleRawComment(CharSourceRange Range, const SourceManager &SourceMgr);
SingleRawComment(StringRef RawText, unsigned StartColumn);
SingleRawComment(StringRef RawText, unsigned ColumnIndent);

SingleRawComment(const SingleRawComment &) = default;
SingleRawComment &operator=(const SingleRawComment &) = default;
Expand Down
2 changes: 1 addition & 1 deletion lib/AST/ASTPrinter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -641,7 +641,7 @@ class PrintAST : public ASTVisitor<PrintAST> {
Lines.clear();

StringRef RawText = SRC.RawText.rtrim("\n\r");
unsigned WhitespaceToTrim = SRC.StartColumn - 1;
unsigned WhitespaceToTrim = SRC.ColumnIndent - 1;
trimLeadingWhitespaceFromLines(RawText, WhitespaceToTrim, Lines);

for (auto Line : Lines) {
Expand Down
8 changes: 4 additions & 4 deletions lib/AST/Module.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -879,10 +879,10 @@ SourceFile::getBasicLocsForDecl(const Decl *D) const {
Result.SourceFilePath = SM.getDisplayNameForLoc(D->getLoc());

for (const auto &SRC : D->getRawComment(/*SerializedOK*/false).Comments) {
Result.DocRanges.push_back(std::make_pair(
SourcePosition { SRC.StartLine, SRC.StartColumn },
SRC.Range.getByteLength())
);
auto LineAndCol = SM.getLineAndColumnInBuffer(SRC.Range.getStart());
Result.DocRanges.push_back(
std::make_pair(SourcePosition{LineAndCol.first, LineAndCol.second},
SRC.Range.getByteLength()));
}

auto setLineColumn = [&SM](SourcePosition &Home, SourceLoc Loc) {
Expand Down
115 changes: 50 additions & 65 deletions lib/AST/RawComment.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,71 +61,57 @@ SingleRawComment::SingleRawComment(CharSourceRange Range,
const SourceManager &SourceMgr)
: Range(Range), RawText(SourceMgr.extractText(Range)),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RawText and ColumnIndent are the only two fields on RawComment that are actually always valid. Range is only set when it isn't a deserialized RawComment. Ideally we'd remove Range from here as well, but it's used in a fair few places... This change is a middle ground where just StartLine/EndLine are removed.

Kind(static_cast<unsigned>(getCommentKind(RawText))) {
auto StartLineAndColumn =
SourceMgr.getLineAndColumnInBuffer(Range.getStart());
StartLine = StartLineAndColumn.first;
StartColumn = StartLineAndColumn.second;
EndLine = SourceMgr.getLineAndColumnInBuffer(Range.getEnd()).first;
ColumnIndent = SourceMgr.getLineAndColumnInBuffer(Range.getStart()).second;
}

SingleRawComment::SingleRawComment(StringRef RawText, unsigned StartColumn)
SingleRawComment::SingleRawComment(StringRef RawText, unsigned ColumnIndent)
: RawText(RawText), Kind(static_cast<unsigned>(getCommentKind(RawText))),
StartColumn(StartColumn), StartLine(0), EndLine(0) {}

static void addCommentToList(SmallVectorImpl<SingleRawComment> &Comments,
const SingleRawComment &SRC) {
// TODO: consider producing warnings when we decide not to merge comments.

if (SRC.isOrdinary()) {
// Skip gyb comments that are line number markers.
if (SRC.RawText.startswith("// ###"))
return;

Comments.clear();
return;
}

// If this is the first documentation comment, save it (because there isn't
// anything to merge it with).
if (Comments.empty()) {
Comments.push_back(SRC);
return;
}

auto &Last = Comments.back();

// Merge comments if they are on same or consecutive lines.
if (Last.EndLine + 1 < SRC.StartLine) {
Comments.clear();
return;
}

Comments.push_back(SRC);
}
ColumnIndent(ColumnIndent) {}

static RawComment toRawComment(ASTContext &Context, CharSourceRange Range) {
if (Range.isInvalid())
return RawComment();

auto &SourceMgr = Context.SourceMgr;
unsigned BufferID = SourceMgr.findBufferContainingLoc(Range.getStart());
unsigned Offset = SourceMgr.getLocOffsetInBuffer(Range.getStart(), BufferID);
unsigned EndOffset = SourceMgr.getLocOffsetInBuffer(Range.getEnd(), BufferID);
auto &SM = Context.SourceMgr;
unsigned BufferID = SM.findBufferContainingLoc(Range.getStart());
unsigned Offset = SM.getLocOffsetInBuffer(Range.getStart(), BufferID);
unsigned EndOffset = SM.getLocOffsetInBuffer(Range.getEnd(), BufferID);
LangOptions FakeLangOpts;
Lexer L(FakeLangOpts, SourceMgr, BufferID, nullptr, LexerMode::Swift,
HashbangMode::Disallowed,
CommentRetentionMode::ReturnAsTokens,
TriviaRetentionMode::WithoutTrivia,
Offset, EndOffset);
Lexer L(FakeLangOpts, SM, BufferID, nullptr, LexerMode::Swift,
HashbangMode::Disallowed, CommentRetentionMode::ReturnAsTokens,
TriviaRetentionMode::WithoutTrivia, Offset, EndOffset);

SmallVector<SingleRawComment, 16> Comments;
Token Tok;
unsigned LastEnd = 0;
while (true) {
L.lex(Tok);
if (Tok.is(tok::eof))
break;
assert(Tok.is(tok::comment));
addCommentToList(Comments, SingleRawComment(Tok.getRange(), SourceMgr));

auto SRC = SingleRawComment(Tok.getRange(), SM);
if (SRC.isOrdinary()) {
// Skip gyb comments that are line number markers.
if (!SRC.RawText.startswith("// ###")) {
Comments.clear();
LastEnd = 0;
}
continue;
}

// Merge comments if they are on same or consecutive lines.
unsigned Start =
SM.getLineAndColumnInBuffer(Tok.getRange().getStart()).first;
if (LastEnd > 0 && LastEnd + 1 < Start) {
Comments.clear();
LastEnd = 0;
} else {
Comments.push_back(SRC);
LastEnd = SM.getLineAndColumnInBuffer(Tok.getRange().getEnd()).first;
}
}

RawComment Result;
Result.Comments = Context.AllocateCopy(Comments);
return Result;
Expand Down Expand Up @@ -159,25 +145,24 @@ RawComment Decl::getRawComment(bool SerializedOK) const {
switch (Unit->getKind()) {
case FileUnitKind::SerializedAST: {
if (SerializedOK) {
if (const auto *CachedLocs = getSerializedLocs()) {
if (!CachedLocs->DocRanges.empty()) {
SmallVector<SingleRawComment, 4> SRCs;
for (const auto &Range : CachedLocs->DocRanges) {
if (Range.isValid()) {
SRCs.push_back({ Range, Context.SourceMgr });
} else {
// if we've run into an invalid range, don't bother trying to load
// any of the other comments
SRCs.clear();
break;
}
auto *CachedLocs = getSerializedLocs();
if (!CachedLocs->DocRanges.empty()) {
SmallVector<SingleRawComment, 4> SRCs;
for (const auto &Range : CachedLocs->DocRanges) {
if (Range.isValid()) {
SRCs.push_back({Range, Context.SourceMgr});
} else {
// if we've run into an invalid range, don't bother trying to load
// any of the other comments
SRCs.clear();
break;
}
auto RC = RawComment(Context.AllocateCopy(llvm::makeArrayRef(SRCs)));
}

if (!RC.isEmpty()) {
Context.setRawComment(this, RC, true);
return RC;
}
if (!SRCs.empty()) {
auto RC = RawComment(Context.AllocateCopy(llvm::makeArrayRef(SRCs)));
Context.setRawComment(this, RC, true);
return RC;
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions lib/Markup/LineList.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ LineList MarkupContext::getLineList(swift::RawComment RC) {
unsigned NewlineBytes = swift::measureNewline(Cleaned);
Cleaned = Cleaned.drop_front(NewlineBytes);
CleanedStartLoc = CleanedStartLoc.getAdvancedLocOrInvalid(NewlineBytes);
HasASCIIArt = measureASCIIArt(Cleaned, C.StartColumn - 1) != 0;
HasASCIIArt = measureASCIIArt(Cleaned, C.ColumnIndent - 1) != 0;
}

while (!Cleaned.empty()) {
Expand All @@ -129,7 +129,7 @@ LineList MarkupContext::getLineList(swift::RawComment RC) {
// Skip over ASCII art, if present.
if (HasASCIIArt)
if (unsigned ASCIIArtBytes =
measureASCIIArt(Cleaned, C.StartColumn - 1)) {
measureASCIIArt(Cleaned, C.ColumnIndent - 1)) {
Cleaned = Cleaned.drop_front(ASCIIArtBytes);
CleanedStartLoc =
CleanedStartLoc.getAdvancedLocOrInvalid(ASCIIArtBytes);
Expand Down
2 changes: 1 addition & 1 deletion lib/Serialization/SerializeDoc.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ class DeclCommentTableInfo {
out << data.Brief;
writer.write<uint32_t>(data.Raw.Comments.size());
for (auto C : data.Raw.Comments) {
writer.write<uint32_t>(C.StartColumn);
writer.write<uint32_t>(C.ColumnIndent);
writer.write<uint32_t>(C.RawText.size());
out << C.RawText;
}
Expand Down