Skip to content
This repository was archived by the owner on Nov 1, 2021. It is now read-only.

Commit 354737f

Browse files
committed
[clang-format] Handle trailing comment sections in import statement lines
Summary: This patch updates the handling of multiline trailing comment sections in import statement lines to make it more consistent with the case in general. This includes updating the parsing logic to collect the trailing comment sections and the formatting logic to not insert escaped newlines at the end of comment lines in import statement lines. Specifically, before this patch this code: ``` #include <a> // line 1 // line 2 ``` will be turned into two unwrapped lines, whereas this code: ``` int i; // line 1 // line 2 ``` is turned into a single unwrapped line, enabling reflowing across comments. An example where the old behaviour is bad is when partially formatting the lines 3 to 4 of this code: ``` #include <a> // line 1 // line 2 int i; ``` which gets turned into: ``` #include <a> // line 1 // line 2 int i; ``` because the two comment lines were independent and the indent was copied. Reviewers: djasper Reviewed By: djasper Subscribers: cfe-commits, klimek Differential Revision: https://reviews.llvm.org/D33351 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@303415 91177308-0d34-0410-b5e6-96231b3b80d8
1 parent 6343907 commit 354737f

File tree

4 files changed

+31
-11
lines changed

4 files changed

+31
-11
lines changed

Diff for: lib/Format/ContinuationIndenter.cpp

+3-1
Original file line numberDiff line numberDiff line change
@@ -587,8 +587,10 @@ unsigned ContinuationIndenter::addTokenOnNewLine(LineState &State,
587587
if (!DryRun) {
588588
unsigned Newlines = std::max(
589589
1u, std::min(Current.NewlinesBefore, Style.MaxEmptyLinesToKeep + 1));
590+
bool ContinuePPDirective =
591+
State.Line->InPPDirective && State.Line->Type != LT_ImportStatement;
590592
Whitespaces.replaceWhitespace(Current, Newlines, State.Column, State.Column,
591-
State.Line->InPPDirective);
593+
ContinuePPDirective);
592594
}
593595

594596
if (!Current.isTrailingComment())

Diff for: lib/Format/TokenAnnotator.cpp

+6-3
Original file line numberDiff line numberDiff line change
@@ -703,9 +703,12 @@ class AnnotatingParser {
703703

704704
void parseIncludeDirective() {
705705
if (CurrentToken && CurrentToken->is(tok::less)) {
706-
next();
707-
while (CurrentToken) {
708-
if (CurrentToken->isNot(tok::comment) || CurrentToken->Next)
706+
next();
707+
while (CurrentToken) {
708+
// Mark tokens up to the trailing line comments as implicit string
709+
// literals.
710+
if (CurrentToken->isNot(tok::comment) &&
711+
!CurrentToken->TokenText.startswith("//"))
709712
CurrentToken->Type = TT_ImplicitStringLiteral;
710713
next();
711714
}

Diff for: lib/Format/UnwrappedLineParser.cpp

+14-7
Original file line numberDiff line numberDiff line change
@@ -55,13 +55,18 @@ class ScopedDeclarationState {
5555
std::vector<bool> &Stack;
5656
};
5757

58+
static bool isLineComment(const FormatToken &FormatTok) {
59+
return FormatTok.is(tok::comment) &&
60+
FormatTok.TokenText.startswith("//");
61+
}
62+
5863
class ScopedMacroState : public FormatTokenSource {
5964
public:
6065
ScopedMacroState(UnwrappedLine &Line, FormatTokenSource *&TokenSource,
6166
FormatToken *&ResetToken)
6267
: Line(Line), TokenSource(TokenSource), ResetToken(ResetToken),
6368
PreviousLineLevel(Line.Level), PreviousTokenSource(TokenSource),
64-
Token(nullptr) {
69+
Token(nullptr), PreviousToken(nullptr) {
6570
TokenSource = this;
6671
Line.Level = 0;
6772
Line.InPPDirective = true;
@@ -78,6 +83,7 @@ class ScopedMacroState : public FormatTokenSource {
7883
// The \c UnwrappedLineParser guards against this by never calling
7984
// \c getNextToken() after it has encountered the first eof token.
8085
assert(!eof());
86+
PreviousToken = Token;
8187
Token = PreviousTokenSource->getNextToken();
8288
if (eof())
8389
return getFakeEOF();
@@ -87,12 +93,17 @@ class ScopedMacroState : public FormatTokenSource {
8793
unsigned getPosition() override { return PreviousTokenSource->getPosition(); }
8894

8995
FormatToken *setPosition(unsigned Position) override {
96+
PreviousToken = nullptr;
9097
Token = PreviousTokenSource->setPosition(Position);
9198
return Token;
9299
}
93100

94101
private:
95-
bool eof() { return Token && Token->HasUnescapedNewline; }
102+
bool eof() {
103+
return Token && Token->HasUnescapedNewline &&
104+
!(PreviousToken && isLineComment(*PreviousToken) &&
105+
isLineComment(*Token) && Token->NewlinesBefore == 1);
106+
}
96107

97108
FormatToken *getFakeEOF() {
98109
static bool EOFInitialized = false;
@@ -112,6 +123,7 @@ class ScopedMacroState : public FormatTokenSource {
112123
FormatTokenSource *PreviousTokenSource;
113124

114125
FormatToken *Token;
126+
FormatToken *PreviousToken;
115127
};
116128

117129
} // end anonymous namespace
@@ -2092,11 +2104,6 @@ bool UnwrappedLineParser::isOnNewLine(const FormatToken &FormatTok) {
20922104
FormatTok.NewlinesBefore > 0;
20932105
}
20942106

2095-
static bool isLineComment(const FormatToken &FormatTok) {
2096-
return FormatTok.is(tok::comment) &&
2097-
FormatTok.TokenText.startswith("//");
2098-
}
2099-
21002107
// Checks if \p FormatTok is a line comment that continues the line comment
21012108
// section on \p Line.
21022109
static bool continuesLineComment(const FormatToken &FormatTok,

Diff for: unittests/Format/FormatTestSelective.cpp

+8
Original file line numberDiff line numberDiff line change
@@ -530,6 +530,14 @@ TEST_F(FormatTestSelective, SelectivelyRequoteJavaScript) {
530530
20, 0));
531531
}
532532

533+
TEST_F(FormatTestSelective, KeepsIndentAfterCommentSectionImport) {
534+
std::string Code = "#include <a> // line 1\n" // 23 chars long
535+
" // line 2\n" // 23 chars long
536+
"\n" // this newline is char 47
537+
"int i;"; // this line is not indented
538+
EXPECT_EQ(Code, format(Code, 47, 1));
539+
}
540+
533541
} // end namespace
534542
} // end namespace format
535543
} // end namespace clang

0 commit comments

Comments
 (0)