euhlmann updated this revision to Diff 112432.
euhlmann marked 7 inline comments as done.
euhlmann added a comment.
- Swapped checks in ContinuationIndenter
- Renamed `PPMaybeIncludeGuard` to `IfNdefCondition` and added comment
- Added another bool member that tracks if include guard detection was rejected
once. If the bool gets set, the loop checking previous lines doesn't run again.
- Set lower ColumnLimit in tests
- Fixed incorrect column for `\` in multi-line macros
- Resolved comment in FormatTestUtils
https://reviews.llvm.org/D35955
Files:
docs/ClangFormatStyleOptions.rst
include/clang/Format/Format.h
lib/Format/ContinuationIndenter.cpp
lib/Format/Format.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
@@ -30,7 +30,8 @@
if (JustReplacedNewline)
MessedUp[i - 1] = '\n';
InComment = true;
- } else if (MessedUp[i] == '#' && (JustReplacedNewline || i == 0)) {
+ } else if (MessedUp[i] == '#' &&
+ (JustReplacedNewline || i == 0 || MessedUp[i - 1] == '\n')) {
if (i != 0)
MessedUp[i - 1] = '\n';
InPreprocessorDirective = true;
Index: unittests/Format/FormatTest.cpp
===================================================================
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -2286,8 +2286,183 @@
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;
+ Style.ColumnLimit = 40;
+ 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_aaaaaaaaaa();\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_aaaaaaaaaa();\n"
+ "# endif\n"
+ "#else\n"
+ "# define A 1\n"
+ "#endif",
+ Style);
+ verifyFormat("#if A\n"
+ "# define MACRO \\\n"
+ " void a(int x) { \\\n"
+ " b(); \\\n"
+ " c(); \\\n"
+ " d(); \\\n"
+ " e(); \\\n"
+ " f(); \\\n"
+ " }\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);
+ // FIXME: 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));
+ // FIXME: 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));
+ // FIXME: 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));
+ // 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));
+ // 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_aaaaaaaaaa();\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,11 @@
// sequence.
std::stack<int> PPChainBranchIndex;
+ // Contains the #ifndef condition for a potential include guard.
+ FormatToken *IfNdefCondition;
+ bool FoundIncludeGuardStart;
+ bool IncludeGuardRejected;
+
friend class ScopedLineState;
friend class CompoundStatementIndenter;
};
Index: lib/Format/UnwrappedLineParser.cpp
===================================================================
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -231,10 +231,15 @@
: 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),
+ IfNdefCondition(nullptr), FoundIncludeGuardStart(false),
+ IncludeGuardRejected(false) {}
void UnwrappedLineParser::reset() {
PPBranchLevel = -1;
+ IfNdefCondition = nullptr;
+ FoundIncludeGuardStart = false;
+ IncludeGuardRejected = false;
Line.reset(new UnwrappedLine);
CommentsBeforeNextToken.clear();
FormatTok = nullptr;
@@ -679,7 +684,7 @@
}
}
// Guard against #endif's without #if.
- if (PPBranchLevel > 0)
+ if (PPBranchLevel > -1)
--PPBranchLevel;
if (!PPChainBranchIndex.empty())
PPChainBranchIndex.pop();
@@ -696,19 +701,53 @@
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;
+ if (!IncludeGuardRejected && !FoundIncludeGuardStart && MaybeIncludeGuard) {
+ for (auto &Line : Lines) {
+ if (!Line.Tokens.front().Tok->is(tok::comment)) {
+ MaybeIncludeGuard = false;
+ IncludeGuardRejected = true;
+ break;
+ }
+ }
+ }
+ --PPBranchLevel;
parsePPUnknown();
+ ++PPBranchLevel;
+ if (!IncludeGuardRejected && !FoundIncludeGuardStart && MaybeIncludeGuard)
+ IfNdefCondition = 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() {
@@ -718,14 +757,26 @@
parsePPUnknown();
return;
}
+ if (IfNdefCondition && IfNdefCondition->TokenText == FormatTok->TokenText) {
+ FoundIncludeGuardStart = true;
+ for (auto &Line : Lines) {
+ if (!Line.Tokens.front().Tok->isOneOf(tok::comment, tok::hash)) {
+ FoundIncludeGuardStart = false;
+ break;
+ }
+ }
+ }
+ IfNdefCondition = 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
@@ -739,7 +790,10 @@
do {
nextToken();
} while (!eof());
+ if (Style.IndentPPDirectives == FormatStyle::PPDIS_AfterHash)
+ Line->Level += PPBranchLevel + 1;
addUnwrappedLine();
+ IfNdefCondition = 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/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
@@ -93,6 +93,13 @@
LineState State;
State.FirstIndent = FirstIndent;
State.Column = FirstIndent;
+ // With preprocessor directive indentation, the line starts on column 0
+ // since it's indented after the hash, but FirstIndent is set to the
+ // preprocessor indent.
+ if (Style.IndentPPDirectives == FormatStyle::PPDIS_AfterHash &&
+ (Line->Type == LT_PreprocessorDirective ||
+ Line->Type == LT_ImportStatement))
+ State.Column = 0;
State.Line = Line;
State.NextToken = Line->First;
State.Stack.push_back(ParenState(FirstIndent, FirstIndent,
@@ -376,9 +383,25 @@
unsigned Spaces = Current.SpacesRequiredBefore + ExtraSpaces;
+ // Indent preprocessor directives after the hash if required.
+ int PPColumnCorrection = 0;
+ if (Style.IndentPPDirectives == FormatStyle::PPDIS_AfterHash &&
+ Previous.is(tok::hash) && State.FirstIndent > 0 &&
+ (State.Line->Type == LT_PreprocessorDirective ||
+ State.Line->Type == LT_ImportStatement)) {
+ 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 avoid this misalignment by subtracting 1 from the
+ // column value passed to replaceWhitespace().
+ if (Style.UseTab != FormatStyle::UT_Never)
+ PPColumnCorrection = -1;
+ }
+
if (!DryRun)
Whitespaces.replaceWhitespace(Current, /*Newlines=*/0, Spaces,
- State.Column + Spaces);
+ State.Column + Spaces + PPColumnCorrection);
// If "BreakBeforeInheritanceComma" mode, don't break within the inheritance
// declaration unless there is multiple inheritance.
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
