euhlmann updated this revision to Diff 111606.
euhlmann marked an inline comment as done.
euhlmann edited the summary of this revision.
euhlmann added a comment.
Eliminated redundancy in tests by fixing test::messUp which was incorrectly
merging lines.
Fixing the issue Mark brought up regarding comments before indented
preprocessor lines appears to be a major change and I believe it's too much
complexity for this patch.
https://reviews.llvm.org/D35955
Files:
docs/ClangFormatStyleOptions.rst
include/clang/Format/Format.h
lib/Format/ContinuationIndenter.cpp
lib/Format/Format.cpp
lib/Format/TokenAnnotator.cpp
lib/Format/UnwrappedLineFormatter.cpp
lib/Format/UnwrappedLineParser.cpp
lib/Format/UnwrappedLineParser.h
unittests/Format/FormatTest.cpp
unittests/Format/FormatTestUtils.h
Index: unittests/Format/FormatTestUtils.h
===================================================================
--- unittests/Format/FormatTestUtils.h
+++ unittests/Format/FormatTestUtils.h
@@ -25,19 +25,21 @@
bool InComment = false;
bool InPreprocessorDirective = false;
bool JustReplacedNewline = false;
+ bool JustPassedNewline = false;
for (unsigned i = 0, e = MessedUp.size() - 1; i != e; ++i) {
if (MessedUp[i] == '/' && MessedUp[i + 1] == '/') {
if (JustReplacedNewline)
MessedUp[i - 1] = '\n';
InComment = true;
- } else if (MessedUp[i] == '#' && (JustReplacedNewline || i == 0)) {
+ } else if (MessedUp[i] == '#' && (JustPassedNewline || i == 0)) {
if (i != 0)
MessedUp[i - 1] = '\n';
InPreprocessorDirective = true;
} else if (MessedUp[i] == '\\' && MessedUp[i + 1] == '\n') {
MessedUp[i] = ' ';
MessedUp[i + 1] = ' ';
} else if (MessedUp[i] == '\n') {
+ JustPassedNewline = true;
if (InComment) {
InComment = false;
} else if (InPreprocessorDirective) {
@@ -48,6 +50,7 @@
}
} else if (MessedUp[i] != ' ') {
JustReplacedNewline = false;
+ JustPassedNewline = false;
}
}
std::string WithoutWhitespace;
Index: unittests/Format/FormatTest.cpp
===================================================================
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -2286,8 +2286,176 @@
getLLVMStyleWithColumns(11));
}
-TEST_F(FormatTest, IndentPreprocessorDirectivesAtZero) {
- EXPECT_EQ("{\n {\n#define A\n }\n}", format("{{\n#define A\n}}"));
+TEST_F(FormatTest, IndentPreprocessorDirectives) {
+ FormatStyle Style = getLLVMStyle();
+ Style.IndentPPDirectives = FormatStyle::PPDIS_None;
+ verifyFormat("#ifdef _WIN32\n"
+ "#define A 0\n"
+ "#ifdef VAR2\n"
+ "#define B 1\n"
+ "#include <someheader.h>\n"
+ "#define MACRO "
+ " \\\n"
+ " some_very_long_func_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
+ "aaaaaaaaaaaaa();\n"
+ "#endif\n"
+ "#else\n"
+ "#define A 1\n"
+ "#endif",
+ Style);
+
+ Style.IndentPPDirectives = FormatStyle::PPDIS_AfterHash;
+ verifyFormat("#ifdef _WIN32\n"
+ "# define A 0\n"
+ "# ifdef VAR2\n"
+ "# define B 1\n"
+ "# include <someheader.h>\n"
+ "# define MACRO "
+ " \\\n"
+ " some_very_long_func_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
+ "aaaaaaaaaaaaaaaaa();\n"
+ "# endif\n"
+ "#else\n"
+ "# define A 1\n"
+ "#endif",
+ Style);
+ // Comments before include guard.
+ verifyFormat("// file comment\n"
+ "// file comment\n"
+ "#ifndef HEADER_H\n"
+ "#define HEADER_H\n"
+ "code();\n"
+ "#endif",
+ Style);
+ // Test with include guards.
+ // EXPECT_EQ is used because verifyFormat() calls messUp() which incorrectly
+ // merges lines.
+ verifyFormat("#ifndef HEADER_H\n"
+ "#define HEADER_H\n"
+ "code();\n"
+ "#endif",
+ Style);
+ // Include guards must have a #define with the same variable immediately
+ // after #ifndef.
+ verifyFormat("#ifndef NOT_GUARD\n"
+ "# define FOO\n"
+ "code();\n"
+ "#endif",
+ Style);
+
+ // Include guards must cover the entire file.
+ verifyFormat("code();\n"
+ "code();\n"
+ "#ifndef NOT_GUARD\n"
+ "# define NOT_GUARD\n"
+ "code();\n"
+ "#endif",
+ Style);
+ verifyFormat("#ifndef NOT_GUARD\n"
+ "# define NOT_GUARD\n"
+ "code();\n"
+ "#endif\n"
+ "code();",
+ Style);
+ // Test with trailing blank lines.
+ verifyFormat("#ifndef HEADER_H\n"
+ "#define HEADER_H\n"
+ "code();\n"
+ "#endif\n",
+ Style);
+ // Include guards don't have #else.
+ verifyFormat("#ifndef NOT_GUARD\n"
+ "# define NOT_GUARD\n"
+ "code();\n"
+ "#else\n"
+ "#endif",
+ Style);
+ verifyFormat("#ifndef NOT_GUARD\n"
+ "# define NOT_GUARD\n"
+ "code();\n"
+ "#elif FOO\n"
+ "#endif",
+ Style);
+ // Defect: This doesn't handle the case where there's code between the
+ // #ifndef and #define but all other conditions hold. This is because when
+ // the #define line is parsed, UnwrappedLineParser::Lines doesn't hold the
+ // previous code line yet, so we can't detect it.
+ EXPECT_EQ("#ifndef NOT_GUARD\n"
+ "code();\n"
+ "#define NOT_GUARD\n"
+ "code();\n"
+ "#endif",
+ format("#ifndef NOT_GUARD\n"
+ "code();\n"
+ "# define NOT_GUARD\n"
+ "code();\n"
+ "#endif",
+ Style));
+ // Defect: This doesn't handle cases where legitimate preprocessor lines may
+ // be outside an include guard. Examples are #pragma once and
+ // #pragma GCC diagnostic, or anything else that does not change the meaning
+ // of the file if it's included multiple times.
+ EXPECT_EQ("#ifdef WIN32\n"
+ "# pragma once\n"
+ "#endif\n"
+ "#ifndef HEADER_H\n"
+ "# define HEADER_H\n"
+ "code();\n"
+ "#endif",
+ format("#ifdef WIN32\n"
+ "# pragma once\n"
+ "#endif\n"
+ "#ifndef HEADER_H\n"
+ "#define HEADER_H\n"
+ "code();\n"
+ "#endif",
+ Style));
+ // Defect: This does not detect when there is a single non-preprocessor line
+ // in front of an include-guard-like structure where other conditions hold
+ // because ScopedLineState hides the line.
+ EXPECT_EQ("code();\n"
+ "#ifndef HEADER_H\n"
+ "#define HEADER_H\n"
+ "code();\n"
+ "#endif",
+ format("code();\n"
+ "#ifndef HEADER_H\n"
+ "# define HEADER_H\n"
+ "code();\n"
+ "#endif",
+ Style));
+ // Defect: 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));
+ // Test with tabs.
+ Style.UseTab = FormatStyle::UT_Always;
+ Style.IndentWidth = 8;
+ Style.TabWidth = 8;
+ verifyFormat("#ifdef _WIN32\n"
+ "#\tdefine A 0\n"
+ "#\tifdef VAR2\n"
+ "#\t\tdefine B 1\n"
+ "#\t\tinclude <someheader.h>\n"
+ "#\t\tdefine MACRO \\\n"
+ "\t\t\tsome_very_long_func_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
+ "aaaaaaaaaaaaaaaaa();\n"
+ "#\tendif\n"
+ "#else\n"
+ "#\tdefine A 1\n"
+ "#endif",
+ Style);
}
TEST_F(FormatTest, FormatHashIfNotAtStartOfLine) {
Index: lib/Format/UnwrappedLineParser.h
===================================================================
--- lib/Format/UnwrappedLineParser.h
+++ lib/Format/UnwrappedLineParser.h
@@ -246,6 +246,9 @@
// sequence.
std::stack<int> PPChainBranchIndex;
+ FormatToken *PPMaybeIncludeGuard;
+ bool FoundIncludeGuardStart;
+
friend class ScopedLineState;
friend class CompoundStatementIndenter;
};
Index: lib/Format/UnwrappedLineParser.cpp
===================================================================
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -231,10 +231,13 @@
: Line(new UnwrappedLine), MustBreakBeforeNextToken(false),
CurrentLines(&Lines), Style(Style), Keywords(Keywords),
CommentPragmasRegex(Style.CommentPragmas), Tokens(nullptr),
- Callback(Callback), AllTokens(Tokens), PPBranchLevel(-1) {}
+ Callback(Callback), AllTokens(Tokens), PPBranchLevel(-1),
+ PPMaybeIncludeGuard(nullptr), FoundIncludeGuardStart(false) {}
void UnwrappedLineParser::reset() {
PPBranchLevel = -1;
+ PPMaybeIncludeGuard = nullptr;
+ FoundIncludeGuardStart = false;
Line.reset(new UnwrappedLine);
CommentsBeforeNextToken.clear();
FormatTok = nullptr;
@@ -674,7 +677,7 @@
}
}
// Guard against #endif's without #if.
- if (PPBranchLevel > 0)
+ if (PPBranchLevel > -1)
--PPBranchLevel;
if (!PPChainBranchIndex.empty())
PPChainBranchIndex.pop();
@@ -691,19 +694,50 @@
if (IfDef && !IfNDef && FormatTok->TokenText == "SWIG")
Unreachable = true;
conditionalCompilationStart(Unreachable);
+ FormatToken *IfCondition = FormatTok;
+ // If there's a #ifndef on the first line, and the only lines before it are
+ // comments, it could be an include guard.
+ bool MaybeIncludeGuard = IfNDef;
+ for (auto& Line : Lines) {
+ if (!Line.Tokens.front().Tok->is(tok::comment)) {
+ MaybeIncludeGuard = false;
+ break;
+ }
+ }
+ --PPBranchLevel;
parsePPUnknown();
+ ++PPBranchLevel;
+ if (MaybeIncludeGuard)
+ PPMaybeIncludeGuard = IfCondition;
}
void UnwrappedLineParser::parsePPElse() {
+ // If a potential include guard has an #else, it's not an include guard.
+ if (FoundIncludeGuardStart && PPBranchLevel == 0)
+ FoundIncludeGuardStart = false;
conditionalCompilationAlternative();
+ if (PPBranchLevel > -1)
+ --PPBranchLevel;
parsePPUnknown();
+ ++PPBranchLevel;
}
void UnwrappedLineParser::parsePPElIf() { parsePPElse(); }
void UnwrappedLineParser::parsePPEndIf() {
conditionalCompilationEnd();
parsePPUnknown();
+ // If the #endif of a potential include guard is the last thing in the file,
+ // then we count it as a real include guard and subtract one from every
+ // preprocessor indent.
+ unsigned TokenPosition = Tokens->getPosition();
+ FormatToken *PeekNext = AllTokens[TokenPosition];
+ if (FoundIncludeGuardStart && PPBranchLevel == -1 && PeekNext->is(tok::eof)) {
+ for (auto& Line : Lines) {
+ if (Line.InPPDirective && Line.Level > 0)
+ --Line.Level;
+ }
+ }
}
void UnwrappedLineParser::parsePPDefine() {
@@ -713,14 +747,27 @@
parsePPUnknown();
return;
}
+ if (PPMaybeIncludeGuard &&
+ PPMaybeIncludeGuard->TokenText == FormatTok->TokenText) {
+ FoundIncludeGuardStart = true;
+ for (auto& Line : Lines) {
+ if (!Line.Tokens.front().Tok->isOneOf(tok::comment, tok::hash)) {
+ FoundIncludeGuardStart = false;
+ break;
+ }
+ }
+ }
+ PPMaybeIncludeGuard = nullptr;
nextToken();
if (FormatTok->Tok.getKind() == tok::l_paren &&
FormatTok->WhitespaceRange.getBegin() ==
FormatTok->WhitespaceRange.getEnd()) {
parseParens();
}
+ if (Style.IndentPPDirectives == FormatStyle::PPDIS_AfterHash)
+ Line->Level += PPBranchLevel + 1;
addUnwrappedLine();
- Line->Level = 1;
+ ++Line->Level;
// Errors during a preprocessor directive can only affect the layout of the
// preprocessor directive, and thus we ignore them. An alternative approach
@@ -734,7 +781,10 @@
do {
nextToken();
} while (!eof());
+ if (Style.IndentPPDirectives == FormatStyle::PPDIS_AfterHash)
+ Line->Level += PPBranchLevel + 1;
addUnwrappedLine();
+ PPMaybeIncludeGuard = nullptr;
}
// Here we blacklist certain tokens that are not usually the first token in an
Index: lib/Format/UnwrappedLineFormatter.cpp
===================================================================
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -1037,6 +1037,10 @@
if (RootToken.IsFirst && !RootToken.HasUnescapedNewline)
Newlines = 0;
+ // Preprocessor directives get indented after the hash, if indented.
+ if (Line.Type == LT_PreprocessorDirective || Line.Type == LT_ImportStatement)
+ Indent = 0;
+
// Remove empty lines after "{".
if (!Style.KeepEmptyLinesAtTheStartOfBlocks && PreviousLine &&
PreviousLine->Last->is(tok::l_brace) &&
Index: lib/Format/TokenAnnotator.cpp
===================================================================
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -1710,7 +1710,7 @@
bool AlignedWithNextLine =
NextNonCommentLine->First->NewlinesBefore <= 1 &&
NextNonCommentLine->First->OriginalColumn ==
- (*I)->First->OriginalColumn;
+ (*I)->First->OriginalColumn;
if (AlignedWithNextLine)
(*I)->Level = NextNonCommentLine->Level;
} else {
Index: lib/Format/Format.cpp
===================================================================
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -133,6 +133,14 @@
}
};
+template <>
+struct ScalarEnumerationTraits<FormatStyle::PPDirectiveIndentStyle> {
+ static void enumeration(IO &IO, FormatStyle::PPDirectiveIndentStyle &Value) {
+ IO.enumCase(Value, "None", FormatStyle::PPDIS_None);
+ IO.enumCase(Value, "AfterHash", FormatStyle::PPDIS_AfterHash);
+ }
+};
+
template <>
struct ScalarEnumerationTraits<FormatStyle::ReturnTypeBreakingStyle> {
static void enumeration(IO &IO, FormatStyle::ReturnTypeBreakingStyle &Value) {
@@ -350,6 +358,7 @@
IO.mapOptional("IncludeCategories", Style.IncludeCategories);
IO.mapOptional("IncludeIsMainRegex", Style.IncludeIsMainRegex);
IO.mapOptional("IndentCaseLabels", Style.IndentCaseLabels);
+ IO.mapOptional("IndentPPDirectives", Style.IndentPPDirectives);
IO.mapOptional("IndentWidth", Style.IndentWidth);
IO.mapOptional("IndentWrappedFunctionNames",
Style.IndentWrappedFunctionNames);
@@ -589,6 +598,7 @@
{".*", 1}};
LLVMStyle.IncludeIsMainRegex = "(Test)?$";
LLVMStyle.IndentCaseLabels = false;
+ LLVMStyle.IndentPPDirectives = FormatStyle::PPDIS_None;
LLVMStyle.IndentWrappedFunctionNames = false;
LLVMStyle.IndentWidth = 2;
LLVMStyle.JavaScriptQuotes = FormatStyle::JSQS_Leave;
Index: lib/Format/ContinuationIndenter.cpp
===================================================================
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -376,6 +376,19 @@
unsigned Spaces = Current.SpacesRequiredBefore + ExtraSpaces;
+ // Indent preprocessor directives after the hash if required.
+ if (Style.IndentPPDirectives == FormatStyle::PPDIS_AfterHash &&
+ (State.Line->Type == LT_PreprocessorDirective ||
+ State.Line->Type == LT_ImportStatement) &&
+ Current.Previous->is(tok::hash) && State.FirstIndent > 0) {
+ Spaces += State.FirstIndent;
+ // For preprocessor indent with tabs, State.Column will be 1 because of the
+ // hash. This causes second-level indents onward to have an extra space
+ // after the tabs. We set the state to column 0 to avoid this misalignment.
+ if (Style.UseTab != FormatStyle::UT_Never)
+ --State.Column;
+ }
+
if (!DryRun)
Whitespaces.replaceWhitespace(Current, /*Newlines=*/0, Spaces,
State.Column + Spaces);
Index: include/clang/Format/Format.h
===================================================================
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -1024,6 +1024,31 @@
/// \endcode
bool IndentCaseLabels;
+ /// \brief Options for indenting preprocessor directives.
+ enum PPDirectiveIndentStyle {
+ /// Does not indent any directives.
+ /// \code
+ /// #if FOO
+ /// #if BAR
+ /// #include <foo>
+ /// #endif
+ /// #endif
+ /// \endcode
+ PPDIS_None,
+ /// Indents directives after the hash.
+ /// \code
+ /// #if FOO
+ /// # if BAR
+ /// # include <foo>
+ /// # endif
+ /// #endif
+ /// \endcode
+ PPDIS_AfterHash
+ };
+
+ /// \brief The preprocessor directive indenting style to use.
+ PPDirectiveIndentStyle IndentPPDirectives;
+
/// \brief The number of columns to use for indentation.
/// \code
/// IndentWidth: 3
@@ -1514,6 +1539,7 @@
ForEachMacros == R.ForEachMacros &&
IncludeCategories == R.IncludeCategories &&
IndentCaseLabels == R.IndentCaseLabels &&
+ IndentPPDirectives == R.IndentPPDirectives &&
IndentWidth == R.IndentWidth && Language == R.Language &&
IndentWrappedFunctionNames == R.IndentWrappedFunctionNames &&
JavaScriptQuotes == R.JavaScriptQuotes &&
Index: docs/ClangFormatStyleOptions.rst
===================================================================
--- docs/ClangFormatStyleOptions.rst
+++ docs/ClangFormatStyleOptions.rst
@@ -1179,6 +1179,33 @@
plop(); plop();
} }
+**IndentPPDirectives** (``PPDirectiveIndentStyle``)
+ Indent preprocessor directives on conditionals.
+
+ Possible values:
+
+ * ``PPDIS_None`` (in configuration: ``None``)
+ Does not indent any directives.
+
+ .. code-block:: c++
+
+ #if FOO
+ #if BAR
+ #include <foo>
+ #endif
+ #endif
+
+ * ``PPDIS_AfterHash`` (in configuration: ``AfterHash``)
+ Indents directives after the hash.
+
+ .. code-block:: c++
+
+ #if FOO
+ # if BAR
+ # include <foo>
+ # endif
+ #endif
+
**IndentWidth** (``unsigned``)
The number of columns to use for indentation.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
