[PATCH] D35485: [clang-format] Fix comment levels between '}' and PPDirective
This revision was automatically updated to reflect the committed changes.
Closed by commit rL308725: [clang-format] Fix comment levels between '}' and
PPDirective (authored by krasimir).
Repository:
rL LLVM
https://reviews.llvm.org/D35485
Files:
cfe/trunk/lib/Format/UnwrappedLineParser.cpp
cfe/trunk/unittests/Format/FormatTestComments.cpp
Index: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp
@@ -486,14 +486,15 @@
return;
}
+ flushComments(isOnNewLine(*FormatTok));
+ Line->Level = InitialLevel;
nextToken(); // Munch the closing brace.
if (MacroBlock && FormatTok->is(tok::l_paren))
parseParens();
if (MunchSemi && FormatTok->Tok.is(tok::semi))
nextToken();
- Line->Level = InitialLevel;
Line->MatchingOpeningBlockLineIndex = OpeningLineIndex;
if (OpeningLineIndex != UnwrappedLine::kInvalidIndex) {
// Update the opening line to add the forward reference as well
Index: cfe/trunk/unittests/Format/FormatTestComments.cpp
===
--- cfe/trunk/unittests/Format/FormatTestComments.cpp
+++ cfe/trunk/unittests/Format/FormatTestComments.cpp
@@ -836,6 +836,25 @@
" int j;\n"
"}"));
+ EXPECT_EQ("int f(int i) {\n"
+" if (true) {\n"
+"++i;\n"
+" }\n"
+" // comment\n"
+"#ifdef A\n"
+" int j;\n"
+"#endif\n"
+"}",
+format("int f(int i) {\n"
+ " if (true) {\n"
+ "++i;\n"
+ " }\n"
+ " // comment\n"
+ "#ifdef A\n"
+ "int j;\n"
+ "#endif\n"
+ "}"));
+
// Keep the current level if there is an empty line between the comment and
// the preprocessor directive.
EXPECT_EQ("void f() {\n"
@@ -853,6 +872,46 @@
" int j;\n"
"}"));
+ EXPECT_EQ("void f() {\n"
+" int i;\n"
+" return i;\n"
+"}\n"
+"// comment\n"
+"\n"
+"#ifdef A\n"
+"int i;\n"
+"#endif // A",
+format("void f() {\n"
+ " int i;\n"
+ " return i;\n"
+ "}\n"
+ "// comment\n"
+ "\n"
+ "#ifdef A\n"
+ "int i;\n"
+ "#endif // A"));
+
+ EXPECT_EQ("int f(int i) {\n"
+" if (true) {\n"
+"++i;\n"
+" }\n"
+" // comment\n"
+"\n"
+"#ifdef A\n"
+" int j;\n"
+"#endif\n"
+"}",
+format("int f(int i) {\n"
+ " if (true) {\n"
+ "++i;\n"
+ " }\n"
+ " // comment\n"
+ "\n"
+ "#ifdef A\n"
+ " int j;\n"
+ "#endif\n"
+ "}"));
+
// Align with the preprocessor directive if the comment was originally aligned
// with the preprocessor directive.
EXPECT_EQ("void f() {\n"
@@ -867,6 +926,25 @@
"#ifdef A\n"
" int j;\n"
"}"));
+
+ EXPECT_EQ("int f(int i) {\n"
+" if (true) {\n"
+"++i;\n"
+" }\n"
+"// comment\n"
+"#ifdef A\n"
+" int j;\n"
+"#endif\n"
+"}",
+format("int f(int i) {\n"
+ " if (true) {\n"
+ "++i;\n"
+ " }\n"
+ "// comment\n"
+ "#ifdef A\n"
+ " int j;\n"
+ "#endif\n"
+ "}"));
}
TEST_F(FormatTestComments, SplitsLongLinesInComments) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35485: [clang-format] Fix comment levels between '}' and PPDirective
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. https://reviews.llvm.org/D35485 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35485: [clang-format] Fix comment levels between '}' and PPDirective
krasimir added a comment. @djasper: ping https://reviews.llvm.org/D35485 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35485: [clang-format] Fix comment levels between '}' and PPDirective
krasimir updated this revision to Diff 107314.
krasimir added a comment.
- Adapt the fix from comment suggestion
https://reviews.llvm.org/D35485
Files:
lib/Format/UnwrappedLineParser.cpp
unittests/Format/FormatTestComments.cpp
Index: unittests/Format/FormatTestComments.cpp
===
--- unittests/Format/FormatTestComments.cpp
+++ unittests/Format/FormatTestComments.cpp
@@ -836,6 +836,25 @@
" int j;\n"
"}"));
+ EXPECT_EQ("int f(int i) {\n"
+" if (true) {\n"
+"++i;\n"
+" }\n"
+" // comment\n"
+"#ifdef A\n"
+" int j;\n"
+"#endif\n"
+"}",
+format("int f(int i) {\n"
+ " if (true) {\n"
+ "++i;\n"
+ " }\n"
+ " // comment\n"
+ "#ifdef A\n"
+ "int j;\n"
+ "#endif\n"
+ "}"));
+
// Keep the current level if there is an empty line between the comment and
// the preprocessor directive.
EXPECT_EQ("void f() {\n"
@@ -853,6 +872,46 @@
" int j;\n"
"}"));
+ EXPECT_EQ("void f() {\n"
+" int i;\n"
+" return i;\n"
+"}\n"
+"// comment\n"
+"\n"
+"#ifdef A\n"
+"int i;\n"
+"#endif // A",
+format("void f() {\n"
+ " int i;\n"
+ " return i;\n"
+ "}\n"
+ "// comment\n"
+ "\n"
+ "#ifdef A\n"
+ "int i;\n"
+ "#endif // A"));
+
+ EXPECT_EQ("int f(int i) {\n"
+" if (true) {\n"
+"++i;\n"
+" }\n"
+" // comment\n"
+"\n"
+"#ifdef A\n"
+" int j;\n"
+"#endif\n"
+"}",
+format("int f(int i) {\n"
+ " if (true) {\n"
+ "++i;\n"
+ " }\n"
+ " // comment\n"
+ "\n"
+ "#ifdef A\n"
+ " int j;\n"
+ "#endif\n"
+ "}"));
+
// Align with the preprocessor directive if the comment was originally aligned
// with the preprocessor directive.
EXPECT_EQ("void f() {\n"
@@ -867,6 +926,25 @@
"#ifdef A\n"
" int j;\n"
"}"));
+
+ EXPECT_EQ("int f(int i) {\n"
+" if (true) {\n"
+"++i;\n"
+" }\n"
+"// comment\n"
+"#ifdef A\n"
+" int j;\n"
+"#endif\n"
+"}",
+format("int f(int i) {\n"
+ " if (true) {\n"
+ "++i;\n"
+ " }\n"
+ "// comment\n"
+ "#ifdef A\n"
+ " int j;\n"
+ "#endif\n"
+ "}"));
}
TEST_F(FormatTestComments, SplitsLongLinesInComments) {
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -486,14 +486,15 @@
return;
}
+ flushComments(isOnNewLine(*FormatTok));
+ Line->Level = InitialLevel;
nextToken(); // Munch the closing brace.
if (MacroBlock && FormatTok->is(tok::l_paren))
parseParens();
if (MunchSemi && FormatTok->Tok.is(tok::semi))
nextToken();
- Line->Level = InitialLevel;
Line->MatchingOpeningBlockLineIndex = OpeningLineIndex;
if (OpeningLineIndex != UnwrappedLine::kInvalidIndex) {
// Update the opening line to add the forward reference as well
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35485: [clang-format] Fix comment levels between '}' and PPDirective
krasimir added inline comments.
Comment at: lib/Format/UnwrappedLineParser.cpp:489
- nextToken(); // Munch the closing brace.
+ nextToken(InitialLevel); // Munch the closing brace.
djasper wrote:
> krasimir wrote:
> > djasper wrote:
> > > krasimir wrote:
> > > > djasper wrote:
> > > > > What happens if you instead change the Line->Level = InitialLevel;
> > > > > statement from below to before this line? That seems like the more
> > > > > intuitively correct fix.
> > > > This doesn't work since comments before the right brace haven't been
> > > > emitted yet and would get the wrong level.
> > > So that means this seems to be the interesting case:
> > >
> > > void f() {
> > > DoSomething();
> > > // This was a fun function.
> > > }
> > > // Cool macro:
> > > #define A a
> > >
> > > Now, both comments are basically read when we are reading the "}", but
> > > they should have different indentation levels. I have another suggestion,
> > > see below.
> > Here is another breaking test in case we change `Line->Level =
> > InitialLevel` to above this line:
> > ```
> > switch (x) {
> > default: {
> > // Do nothing.
> > }
> > }
> > ```
> > gets reformatted as:
> > ```
> > switch (x) {
> > default: {
> > // Do nothing.
> > }
> > }
> > ```
> I think we can fix all of these cases by doing:
>
> flushComments(isOnNewLine(*FormatTok));
> Line->Level = InitialLevel;
> nextToken(); // Munch the closing brace.
>
> (so add the first two lines here, remove Line->Level = InitialLevel; below.
Cool! Thanks!
https://reviews.llvm.org/D35485
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35485: [clang-format] Fix comment levels between '}' and PPDirective
djasper added inline comments.
Comment at: lib/Format/UnwrappedLineParser.cpp:489
- nextToken(); // Munch the closing brace.
+ nextToken(InitialLevel); // Munch the closing brace.
krasimir wrote:
> djasper wrote:
> > krasimir wrote:
> > > djasper wrote:
> > > > What happens if you instead change the Line->Level = InitialLevel;
> > > > statement from below to before this line? That seems like the more
> > > > intuitively correct fix.
> > > This doesn't work since comments before the right brace haven't been
> > > emitted yet and would get the wrong level.
> > So that means this seems to be the interesting case:
> >
> > void f() {
> > DoSomething();
> > // This was a fun function.
> > }
> > // Cool macro:
> > #define A a
> >
> > Now, both comments are basically read when we are reading the "}", but they
> > should have different indentation levels. I have another suggestion, see
> > below.
> Here is another breaking test in case we change `Line->Level = InitialLevel`
> to above this line:
> ```
> switch (x) {
> default: {
> // Do nothing.
> }
> }
> ```
> gets reformatted as:
> ```
> switch (x) {
> default: {
> // Do nothing.
> }
> }
> ```
I think we can fix all of these cases by doing:
flushComments(isOnNewLine(*FormatTok));
Line->Level = InitialLevel;
nextToken(); // Munch the closing brace.
(so add the first two lines here, remove Line->Level = InitialLevel; below.
https://reviews.llvm.org/D35485
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35485: [clang-format] Fix comment levels between '}' and PPDirective
krasimir updated this revision to Diff 107285.
krasimir marked 2 inline comments as done.
krasimir added a comment.
- Manually messed up tests
https://reviews.llvm.org/D35485
Files:
lib/Format/UnwrappedLineParser.cpp
lib/Format/UnwrappedLineParser.h
unittests/Format/FormatTestComments.cpp
Index: unittests/Format/FormatTestComments.cpp
===
--- unittests/Format/FormatTestComments.cpp
+++ unittests/Format/FormatTestComments.cpp
@@ -836,6 +836,25 @@
" int j;\n"
"}"));
+ EXPECT_EQ("int f(int i) {\n"
+" if (true) {\n"
+"++i;\n"
+" }\n"
+" // comment\n"
+"#ifdef A\n"
+" int j;\n"
+"#endif\n"
+"}",
+format("int f(int i) {\n"
+ " if (true) {\n"
+ "++i;\n"
+ " }\n"
+ " // comment\n"
+ "#ifdef A\n"
+ "int j;\n"
+ "#endif\n"
+ "}"));
+
// Keep the current level if there is an empty line between the comment and
// the preprocessor directive.
EXPECT_EQ("void f() {\n"
@@ -853,6 +872,46 @@
" int j;\n"
"}"));
+ EXPECT_EQ("void f() {\n"
+" int i;\n"
+" return i;\n"
+"}\n"
+"// comment\n"
+"\n"
+"#ifdef A\n"
+"int i;\n"
+"#endif // A",
+format("void f() {\n"
+ " int i;\n"
+ " return i;\n"
+ "}\n"
+ "// comment\n"
+ "\n"
+ "#ifdef A\n"
+ "int i;\n"
+ "#endif // A"));
+
+ EXPECT_EQ("int f(int i) {\n"
+" if (true) {\n"
+"++i;\n"
+" }\n"
+" // comment\n"
+"\n"
+"#ifdef A\n"
+" int j;\n"
+"#endif\n"
+"}",
+format("int f(int i) {\n"
+ " if (true) {\n"
+ "++i;\n"
+ " }\n"
+ " // comment\n"
+ "\n"
+ "#ifdef A\n"
+ " int j;\n"
+ "#endif\n"
+ "}"));
+
// Align with the preprocessor directive if the comment was originally aligned
// with the preprocessor directive.
EXPECT_EQ("void f() {\n"
@@ -867,6 +926,25 @@
"#ifdef A\n"
" int j;\n"
"}"));
+
+ EXPECT_EQ("int f(int i) {\n"
+" if (true) {\n"
+"++i;\n"
+" }\n"
+"// comment\n"
+"#ifdef A\n"
+" int j;\n"
+"#endif\n"
+"}",
+format("int f(int i) {\n"
+ " if (true) {\n"
+ "++i;\n"
+ " }\n"
+ "// comment\n"
+ "#ifdef A\n"
+ " int j;\n"
+ "#endif\n"
+ "}"));
}
TEST_F(FormatTestComments, SplitsLongLinesInComments) {
Index: lib/Format/UnwrappedLineParser.h
===
--- lib/Format/UnwrappedLineParser.h
+++ lib/Format/UnwrappedLineParser.h
@@ -19,6 +19,7 @@
#include "FormatToken.h"
#include "clang/Basic/IdentifierTable.h"
#include "clang/Format/Format.h"
+#include "llvm/ADT/Optional.h"
#include "llvm/Support/Regex.h"
#include
#include
@@ -123,9 +124,9 @@
void tryToParseJSFunction();
void addUnwrappedLine();
bool eof() const;
- void nextToken();
+ void nextToken(llvm::Optional InitialLevel = None);
const FormatToken *getPreviousToken();
- void readToken();
+ void readToken(llvm::Optional InitialLevel = None);
// Decides which comment tokens should be added to the current line and which
// should be added as comments before the next token.
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -486,7 +486,7 @@
return;
}
- nextToken(); // Munch the closing brace.
+ nextToken(InitialLevel); // Munch the closing brace.
if (MacroBlock && FormatTok->is(tok::l_paren))
parseParens();
@@ -2287,13 +2287,13 @@
CommentsBeforeNextToken.clear();
}
-void UnwrappedLineParser::nextToken() {
+void UnwrappedLineParser::nextToken(llvm::Optional InitialLevel) {
if (eof())
return;
flushComments(isOnNewLine(*FormatTok));
pushToken(FormatTok);
if (Style.Language != FormatStyle::LK_JavaScript)
-readToken();
+readToken(InitialLevel);
else
readTokenWithJavaScriptASI();
}
@@ -2362,7 +2362,7 @@
}
}
-void
[PATCH] D35485: [clang-format] Fix comment levels between '}' and PPDirective
krasimir marked 2 inline comments as done.
krasimir added inline comments.
Comment at: lib/Format/UnwrappedLineParser.cpp:489
- nextToken(); // Munch the closing brace.
+ nextToken(InitialLevel); // Munch the closing brace.
djasper wrote:
> krasimir wrote:
> > djasper wrote:
> > > What happens if you instead change the Line->Level = InitialLevel;
> > > statement from below to before this line? That seems like the more
> > > intuitively correct fix.
> > This doesn't work since comments before the right brace haven't been
> > emitted yet and would get the wrong level.
> So that means this seems to be the interesting case:
>
> void f() {
> DoSomething();
> // This was a fun function.
> }
> // Cool macro:
> #define A a
>
> Now, both comments are basically read when we are reading the "}", but they
> should have different indentation levels. I have another suggestion, see
> below.
Here is another breaking test in case we change `Line->Level = InitialLevel` to
above this line:
```
switch (x) {
default: {
// Do nothing.
}
}
```
gets reformatted as:
```
switch (x) {
default: {
// Do nothing.
}
}
```
Comment at: lib/Format/UnwrappedLineParser.cpp:2378
ScopedLineState BlockState(*this, SwitchToPreprocessorLines);
+ if (InitialLevel)
+Line->Level = *InitialLevel;
djasper wrote:
> What happens if we always set the Level to 0 here?
If we always set the Level to 0 here, then
```
void f() {
int i;
/* comment */
#define A
}
```
gets indented as
```
void f() {
int i;
/* comment */
#define A
}
```
https://reviews.llvm.org/D35485
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35485: [clang-format] Fix comment levels between '}' and PPDirective
djasper added inline comments.
Comment at: lib/Format/UnwrappedLineParser.cpp:2378
ScopedLineState BlockState(*this, SwitchToPreprocessorLines);
+ if (InitialLevel)
+Line->Level = *InitialLevel;
What happens if we always set the Level to 0 here?
Comment at: lib/Format/UnwrappedLineParser.cpp:489
- nextToken(); // Munch the closing brace.
+ nextToken(InitialLevel); // Munch the closing brace.
krasimir wrote:
> djasper wrote:
> > What happens if you instead change the Line->Level = InitialLevel;
> > statement from below to before this line? That seems like the more
> > intuitively correct fix.
> This doesn't work since comments before the right brace haven't been emitted
> yet and would get the wrong level.
So that means this seems to be the interesting case:
void f() {
DoSomething();
// This was a fun function.
}
// Cool macro:
#define A a
Now, both comments are basically read when we are reading the "}", but they
should have different indentation levels. I have another suggestion, see below.
Comment at: unittests/Format/FormatTestComments.cpp:848
+"}",
+format("int f(int i) {\n"
+ " if (true) {\n"
krasimir wrote:
> djasper wrote:
> > Generally, mess up the code in some way to ensure that it is actually being
> > formatted.
> Messing up doesn't work in this case, because we rely on the original columns
> of the comment and the previous line. That's why I added a bunch of tests.
I meant *manually* mess up. So add spaces here and there.
https://reviews.llvm.org/D35485
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35485: [clang-format] Fix comment levels between '}' and PPDirective
krasimir updated this revision to Diff 107087.
krasimir added a comment.
- Remove TODO test case
https://reviews.llvm.org/D35485
Files:
lib/Format/UnwrappedLineParser.cpp
lib/Format/UnwrappedLineParser.h
unittests/Format/FormatTestComments.cpp
Index: unittests/Format/FormatTestComments.cpp
===
--- unittests/Format/FormatTestComments.cpp
+++ unittests/Format/FormatTestComments.cpp
@@ -836,6 +836,25 @@
" int j;\n"
"}"));
+ EXPECT_EQ("int f(int i) {\n"
+" if (true) {\n"
+"++i;\n"
+" }\n"
+" // comment\n"
+"#ifdef A\n"
+" int j;\n"
+"#endif\n"
+"}",
+format("int f(int i) {\n"
+ " if (true) {\n"
+ "++i;\n"
+ " }\n"
+ " // comment\n"
+ "#ifdef A\n"
+ " int j;\n"
+ "#endif\n"
+ "}"));
+
// Keep the current level if there is an empty line between the comment and
// the preprocessor directive.
EXPECT_EQ("void f() {\n"
@@ -853,6 +872,46 @@
" int j;\n"
"}"));
+ EXPECT_EQ("void f() {\n"
+" int i;\n"
+" return i;\n"
+"}\n"
+"// comment\n"
+"\n"
+"#ifdef A\n"
+"int i;\n"
+"#endif // A",
+format("void f() {\n"
+ " int i;\n"
+ " return i;\n"
+ "}\n"
+ "// comment\n"
+ "\n"
+ "#ifdef A\n"
+ "int i;\n"
+ "#endif // A"));
+
+ EXPECT_EQ("int f(int i) {\n"
+" if (true) {\n"
+"++i;\n"
+" }\n"
+" // comment\n"
+"\n"
+"#ifdef A\n"
+" int j;\n"
+"#endif\n"
+"}",
+format("int f(int i) {\n"
+ " if (true) {\n"
+ "++i;\n"
+ " }\n"
+ " // comment\n"
+ "\n"
+ "#ifdef A\n"
+ " int j;\n"
+ "#endif\n"
+ "}"));
+
// Align with the preprocessor directive if the comment was originally aligned
// with the preprocessor directive.
EXPECT_EQ("void f() {\n"
@@ -867,6 +926,25 @@
"#ifdef A\n"
" int j;\n"
"}"));
+
+ EXPECT_EQ("int f(int i) {\n"
+" if (true) {\n"
+"++i;\n"
+" }\n"
+"// comment\n"
+"#ifdef A\n"
+" int j;\n"
+"#endif\n"
+"}",
+format("int f(int i) {\n"
+ " if (true) {\n"
+ "++i;\n"
+ " }\n"
+ "// comment\n"
+ "#ifdef A\n"
+ " int j;\n"
+ "#endif\n"
+ "}"));
}
TEST_F(FormatTestComments, SplitsLongLinesInComments) {
Index: lib/Format/UnwrappedLineParser.h
===
--- lib/Format/UnwrappedLineParser.h
+++ lib/Format/UnwrappedLineParser.h
@@ -19,6 +19,7 @@
#include "FormatToken.h"
#include "clang/Basic/IdentifierTable.h"
#include "clang/Format/Format.h"
+#include "llvm/ADT/Optional.h"
#include "llvm/Support/Regex.h"
#include
#include
@@ -123,9 +124,9 @@
void tryToParseJSFunction();
void addUnwrappedLine();
bool eof() const;
- void nextToken();
+ void nextToken(llvm::Optional InitialLevel = None);
const FormatToken *getPreviousToken();
- void readToken();
+ void readToken(llvm::Optional InitialLevel = None);
// Decides which comment tokens should be added to the current line and which
// should be added as comments before the next token.
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -486,7 +486,7 @@
return;
}
- nextToken(); // Munch the closing brace.
+ nextToken(InitialLevel); // Munch the closing brace.
if (MacroBlock && FormatTok->is(tok::l_paren))
parseParens();
@@ -2287,13 +2287,13 @@
CommentsBeforeNextToken.clear();
}
-void UnwrappedLineParser::nextToken() {
+void UnwrappedLineParser::nextToken(llvm::Optional InitialLevel) {
if (eof())
return;
flushComments(isOnNewLine(*FormatTok));
pushToken(FormatTok);
if (Style.Language != FormatStyle::LK_JavaScript)
-readToken();
+readToken(InitialLevel);
else
readTokenWithJavaScriptASI();
}
@@ -2362,7 +2362,7 @@
}
}
-void UnwrappedLineParser::readToken() {
+void
[PATCH] D35485: [clang-format] Fix comment levels between '}' and PPDirective
krasimir updated this revision to Diff 107038.
krasimir marked 2 inline comments as done.
krasimir added a comment.
- Fix formatting
https://reviews.llvm.org/D35485
Files:
lib/Format/UnwrappedLineParser.cpp
lib/Format/UnwrappedLineParser.h
unittests/Format/FormatTestComments.cpp
Index: unittests/Format/FormatTestComments.cpp
===
--- unittests/Format/FormatTestComments.cpp
+++ unittests/Format/FormatTestComments.cpp
@@ -836,6 +836,25 @@
" int j;\n"
"}"));
+ EXPECT_EQ("int f(int i) {\n"
+" if (true) {\n"
+"++i;\n"
+" }\n"
+" // comment\n"
+"#ifdef A\n"
+" int j;\n"
+"#endif\n"
+"}",
+format("int f(int i) {\n"
+ " if (true) {\n"
+ "++i;\n"
+ " }\n"
+ " // comment\n"
+ "#ifdef A\n"
+ " int j;\n"
+ "#endif\n"
+ "}"));
+
// Keep the current level if there is an empty line between the comment and
// the preprocessor directive.
EXPECT_EQ("void f() {\n"
@@ -853,6 +872,46 @@
" int j;\n"
"}"));
+ EXPECT_EQ("void f() {\n"
+" int i;\n"
+" return i;\n"
+"}\n"
+"// comment\n"
+"\n"
+"#ifdef A\n"
+"int i;\n"
+"#endif // A",
+format("void f() {\n"
+ " int i;\n"
+ " return i;\n"
+ "}\n"
+ "// comment\n"
+ "\n"
+ "#ifdef A\n"
+ "int i;\n"
+ "#endif // A"));
+
+ EXPECT_EQ("int f(int i) {\n"
+" if (true) {\n"
+"++i;\n"
+" }\n"
+" // comment\n"
+"\n"
+"#ifdef A\n"
+" int j;\n"
+"#endif\n"
+"}",
+format("int f(int i) {\n"
+ " if (true) {\n"
+ "++i;\n"
+ " }\n"
+ " // comment\n"
+ "\n"
+ "#ifdef A\n"
+ " int j;\n"
+ "#endif\n"
+ "}"));
+
// Align with the preprocessor directive if the comment was originally aligned
// with the preprocessor directive.
EXPECT_EQ("void f() {\n"
@@ -867,6 +926,25 @@
"#ifdef A\n"
" int j;\n"
"}"));
+
+ EXPECT_EQ("int f(int i) {\n"
+" if (true) {\n"
+"++i;\n"
+" }\n"
+"// comment\n"
+"#ifdef A\n"
+" int j;\n"
+"#endif\n"
+"}",
+format("int f(int i) {\n"
+ " if (true) {\n"
+ "++i;\n"
+ " }\n"
+ "// comment\n"
+ "#ifdef A\n"
+ " int j;\n"
+ "#endif\n"
+ "}"));
}
TEST_F(FormatTestComments, SplitsLongLinesInComments) {
@@ -2571,6 +2649,29 @@
"* long */",
getLLVMStyleWithColumns(20)));
}
+
+TEST_F(FormatTestComments, TODO) {
+ EXPECT_EQ("void f() {\n"
+" int i;\n"
+" return i;\n"
+"}\n"
+"// comment\n"
+"\n"
+"#ifdef A\n"
+"int i;\n"
+"#endif // A",
+format("void f() {\n"
+ " int i;\n"
+ " return i;\n"
+ "}\n"
+ "// comment\n"
+ "\n"
+ "#ifdef A\n"
+ "int i;\n"
+ "#endif // A",
+ getLLVMStyleWithColumns(20)));
+}
+
} // end namespace
} // end namespace format
} // end namespace clang
Index: lib/Format/UnwrappedLineParser.h
===
--- lib/Format/UnwrappedLineParser.h
+++ lib/Format/UnwrappedLineParser.h
@@ -19,6 +19,7 @@
#include "FormatToken.h"
#include "clang/Basic/IdentifierTable.h"
#include "clang/Format/Format.h"
+#include "llvm/ADT/Optional.h"
#include "llvm/Support/Regex.h"
#include
#include
@@ -123,9 +124,9 @@
void tryToParseJSFunction();
void addUnwrappedLine();
bool eof() const;
- void nextToken();
+ void nextToken(llvm::Optional InitialLevel = None);
const FormatToken *getPreviousToken();
- void readToken();
+ void readToken(llvm::Optional InitialLevel = None);
// Decides which comment tokens should be added to the current line and which
// should be added as comments before the next token.
Index: lib/Format/UnwrappedLineParser.cpp
[PATCH] D35485: [clang-format] Fix comment levels between '}' and PPDirective
krasimir added inline comments.
Comment at: lib/Format/UnwrappedLineParser.cpp:489
- nextToken(); // Munch the closing brace.
+ nextToken(InitialLevel); // Munch the closing brace.
djasper wrote:
> What happens if you instead change the Line->Level = InitialLevel; statement
> from below to before this line? That seems like the more intuitively correct
> fix.
This doesn't work since comments before the right brace haven't been emitted
yet and would get the wrong level.
Comment at: unittests/Format/FormatTestComments.cpp:848
+"}",
+format("int f(int i) {\n"
+ " if (true) {\n"
djasper wrote:
> Generally, mess up the code in some way to ensure that it is actually being
> formatted.
Messing up doesn't work in this case, because we rely on the original columns
of the comment and the previous line. That's why I added a bunch of tests.
https://reviews.llvm.org/D35485
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35485: [clang-format] Fix comment levels between '}' and PPDirective
djasper added inline comments.
Comment at: lib/Format/UnwrappedLineParser.cpp:489
- nextToken(); // Munch the closing brace.
+ nextToken(InitialLevel); // Munch the closing brace.
What happens if you instead change the Line->Level = InitialLevel; statement
from below to before this line? That seems like the more intuitively correct
fix.
Comment at: lib/Format/UnwrappedLineParser.cpp:2378
ScopedLineState BlockState(*this, SwitchToPreprocessorLines);
+ if (InitialLevel) Line->Level = *InitialLevel;
// Comments stored before the preprocessor directive need to be output
LLVM style requires a line break.
Comment at: unittests/Format/FormatTestComments.cpp:848
+"}",
+format("int f(int i) {\n"
+ " if (true) {\n"
Generally, mess up the code in some way to ensure that it is actually being
formatted.
https://reviews.llvm.org/D35485
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
