Skip to content

Commit

Permalink
[clang-format] Handle trailing comment sections in import statement l…
Browse files Browse the repository at this point in the history
…ines

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

llvm-svn: 303415
  • Loading branch information
krasimirgg committed May 19, 2017
1 parent 76f9386 commit a1c3093
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 11 deletions.
4 changes: 3 additions & 1 deletion clang/lib/Format/ContinuationIndenter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -587,8 +587,10 @@ unsigned ContinuationIndenter::addTokenOnNewLine(LineState &State,
if (!DryRun) {
unsigned Newlines = std::max(
1u, std::min(Current.NewlinesBefore, Style.MaxEmptyLinesToKeep + 1));
bool ContinuePPDirective =
State.Line->InPPDirective && State.Line->Type != LT_ImportStatement;
Whitespaces.replaceWhitespace(Current, Newlines, State.Column, State.Column,
State.Line->InPPDirective);
ContinuePPDirective);
}

if (!Current.isTrailingComment())
Expand Down
9 changes: 6 additions & 3 deletions clang/lib/Format/TokenAnnotator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -703,9 +703,12 @@ class AnnotatingParser {

void parseIncludeDirective() {
if (CurrentToken && CurrentToken->is(tok::less)) {
next();
while (CurrentToken) {
if (CurrentToken->isNot(tok::comment) || CurrentToken->Next)
next();
while (CurrentToken) {
// Mark tokens up to the trailing line comments as implicit string
// literals.
if (CurrentToken->isNot(tok::comment) &&
!CurrentToken->TokenText.startswith("//"))
CurrentToken->Type = TT_ImplicitStringLiteral;
next();
}
Expand Down
21 changes: 14 additions & 7 deletions clang/lib/Format/UnwrappedLineParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,18 @@ class ScopedDeclarationState {
std::vector<bool> &Stack;
};

static bool isLineComment(const FormatToken &FormatTok) {
return FormatTok.is(tok::comment) &&
FormatTok.TokenText.startswith("//");
}

class ScopedMacroState : public FormatTokenSource {
public:
ScopedMacroState(UnwrappedLine &Line, FormatTokenSource *&TokenSource,
FormatToken *&ResetToken)
: Line(Line), TokenSource(TokenSource), ResetToken(ResetToken),
PreviousLineLevel(Line.Level), PreviousTokenSource(TokenSource),
Token(nullptr) {
Token(nullptr), PreviousToken(nullptr) {
TokenSource = this;
Line.Level = 0;
Line.InPPDirective = true;
Expand All @@ -78,6 +83,7 @@ class ScopedMacroState : public FormatTokenSource {
// The \c UnwrappedLineParser guards against this by never calling
// \c getNextToken() after it has encountered the first eof token.
assert(!eof());
PreviousToken = Token;
Token = PreviousTokenSource->getNextToken();
if (eof())
return getFakeEOF();
Expand All @@ -87,12 +93,17 @@ class ScopedMacroState : public FormatTokenSource {
unsigned getPosition() override { return PreviousTokenSource->getPosition(); }

FormatToken *setPosition(unsigned Position) override {
PreviousToken = nullptr;
Token = PreviousTokenSource->setPosition(Position);
return Token;
}

private:
bool eof() { return Token && Token->HasUnescapedNewline; }
bool eof() {
return Token && Token->HasUnescapedNewline &&
!(PreviousToken && isLineComment(*PreviousToken) &&
isLineComment(*Token) && Token->NewlinesBefore == 1);
}

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

FormatToken *Token;
FormatToken *PreviousToken;
};

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

static bool isLineComment(const FormatToken &FormatTok) {
return FormatTok.is(tok::comment) &&
FormatTok.TokenText.startswith("//");
}

// Checks if \p FormatTok is a line comment that continues the line comment
// section on \p Line.
static bool continuesLineComment(const FormatToken &FormatTok,
Expand Down
8 changes: 8 additions & 0 deletions clang/unittests/Format/FormatTestSelective.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,14 @@ TEST_F(FormatTestSelective, SelectivelyRequoteJavaScript) {
20, 0));
}

TEST_F(FormatTestSelective, KeepsIndentAfterCommentSectionImport) {
std::string Code = "#include <a> // line 1\n" // 23 chars long
" // line 2\n" // 23 chars long
"\n" // this newline is char 47
"int i;"; // this line is not indented
EXPECT_EQ(Code, format(Code, 47, 1));
}

} // end namespace
} // end namespace format
} // end namespace clang

0 comments on commit a1c3093

Please sign in to comment.