mzeren-vmw created this revision.
mzeren-vmw added reviewers: euhlmann, krasimir, klimek.
Herald added a subscriber: cfe-commits.
r312125, which introduced preprocessor indentation, shipped with a known
issue where "indentation of comments immediately before indented
preprocessor lines is toggled on each run". For example these two forms
toggle:
#ifndef HEADER_H
#define HEADER_H
#if 1
// comment
# define A 0
#endif
#endif
#ifndef HEADER_H
#define HEADER_H
#if 1
// comment
# define A 0
#endif
#endif
This happens because we check vertical alignment against the "#" yet
indent to the level of the "define". This patch resolves this issue by
checking vertical alignment against the "define", and by tracking a
"LevelOffset" (0 or 1) in each AnnotatedLine to account for the
off-by-one indentation of preprocessor lines.
Repository:
rC Clang
https://reviews.llvm.org/D42036
Files:
lib/Format/TokenAnnotator.cpp
lib/Format/TokenAnnotator.h
lib/Format/UnwrappedLineFormatter.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===================================================================
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -2497,7 +2497,6 @@
"code();\n"
"#endif",
Style);
-
// Include guards must cover the entire file.
verifyFormat("code();\n"
"code();\n"
@@ -2593,21 +2592,34 @@
"code();\n"
"#endif",
Style));
- // FIXME: The comment indent corrector in TokenAnnotator gets thrown off by
- // preprocessor indentation.
- EXPECT_EQ("#if 1\n"
- " // comment\n"
- "# define A 0\n"
- "// comment\n"
- "# define B 0\n"
- "#endif",
- format("#if 1\n"
- "// comment\n"
- "# define A 0\n"
- " // comment\n"
- "# define B 0\n"
- "#endif",
- Style));
+ // Comments aligned to macros stay aligned. This test is incompatible with
+ // verifyFormat() because messUp() removes the alignment.
+ {
+ const char *Expected = "// Level 0 unaligned comment\n"
+ "#ifndef HEADER_H\n"
+ "// Level 0 aligned comment\n"
+ "#define HEADER_H\n"
+ "#if 1\n"
+ " // aligned comment\n"
+ "# define A 0\n"
+ "// un-aligned comment\n"
+ "# define B 0\n"
+ "#endif\n"
+ "#endif";
+ const char *ToFormat = " // Level 0 unaligned comment\n"
+ "#ifndef HEADER_H\n"
+ "// Level 0 aligned comment\n"
+ "#define HEADER_H\n"
+ "#if 1\n"
+ " // aligned comment\n"
+ "# define A 0\n"
+ " // un-aligned comment\n"
+ "# define B 0\n"
+ "#endif\n"
+ "#endif";
+ EXPECT_EQ(Expected, format(ToFormat, Style));
+ EXPECT_EQ(Expected, format(Expected, Style));
+ }
// Test with tabs.
Style.UseTab = FormatStyle::UT_Always;
Style.IndentWidth = 8;
Index: lib/Format/UnwrappedLineFormatter.cpp
===================================================================
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -52,6 +52,7 @@
/// next.
void nextLine(const AnnotatedLine &Line) {
Offset = getIndentOffset(*Line.First);
+ Offset += Line.LevelOffset;
// Update the indent level cache size so that we can rely on it
// having the right size in adjustToUnmodifiedline.
while (IndentForLevel.size() <= Line.Level)
Index: lib/Format/TokenAnnotator.h
===================================================================
--- lib/Format/TokenAnnotator.h
+++ lib/Format/TokenAnnotator.h
@@ -38,7 +38,7 @@
class AnnotatedLine {
public:
AnnotatedLine(const UnwrappedLine &Line)
- : First(Line.Tokens.front().Tok), Level(Line.Level),
+ : First(Line.Tokens.front().Tok), Level(Line.Level), LevelOffset(0),
MatchingOpeningBlockLineIndex(Line.MatchingOpeningBlockLineIndex),
InPPDirective(Line.InPPDirective),
MustBeDeclaration(Line.MustBeDeclaration), MightBeFunctionDecl(false),
@@ -111,6 +111,12 @@
LineType Type;
unsigned Level;
+
+ /// Adjustment to Level based indent. When comments are aligned to the next
+ /// preprocessor line they must use the same offset as the directive,
+ /// typically 1 due to the leading #.
+ unsigned LevelOffset;
+
size_t MatchingOpeningBlockLineIndex;
bool InPPDirective;
bool MustBeDeclaration;
Index: lib/Format/TokenAnnotator.cpp
===================================================================
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -1707,6 +1707,28 @@
FormatToken *Current;
};
+enum CommentAlignment { CA_None, CA_Code, CA_Preprocessor };
+
+CommentAlignment getCommentAlignment(const AnnotatedLine &Comment,
+ const AnnotatedLine &NextNonComment) {
+ if (NextNonComment.First->NewlinesBefore > 1)
+ return CA_None;
+ // If the next line is an indented preprocessor line look at the directive,
+ // not the #.
+ if ((NextNonComment.Type == LT_PreprocessorDirective ||
+ NextNonComment.Type == LT_ImportStatement) &&
+ NextNonComment.Level > 0 && !Comment.InPPDirective)
+ return NextNonComment.First->Next &&
+ (Comment.First->OriginalColumn ==
+ NextNonComment.First->Next->OriginalColumn)
+ ? CA_Preprocessor
+ : CA_None;
+ else
+ return Comment.First->OriginalColumn == NextNonComment.First->OriginalColumn
+ ? CA_Code
+ : CA_None;
+}
+
} // end anonymous namespace
void TokenAnnotator::setCommentLineLevels(
@@ -1723,15 +1745,15 @@
}
}
- if (NextNonCommentLine && CommentLine) {
- // If the comment is currently aligned with the line immediately following
- // it, that's probably intentional and we should keep it.
- bool AlignedWithNextLine =
- NextNonCommentLine->First->NewlinesBefore <= 1 &&
- NextNonCommentLine->First->OriginalColumn ==
- (*I)->First->OriginalColumn;
- if (AlignedWithNextLine)
+ // If the comment is aligned with the line immediately following it, that's
+ // probably intentional and we should keep it.
+ if (CommentLine && NextNonCommentLine) {
+ CommentAlignment Alignment =
+ getCommentAlignment(**I, *NextNonCommentLine);
+ if (Alignment != CA_None)
(*I)->Level = NextNonCommentLine->Level;
+ if (Alignment == CA_Preprocessor)
+ (*I)->LevelOffset = 1;
} else {
NextNonCommentLine = (*I)->First->isNot(tok::r_brace) ? (*I) : nullptr;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
