euhlmann updated this revision to Diff 110602.
euhlmann edited the summary of this revision.
euhlmann added a comment.
The patch now uses `PPBranchLevel` to track indent level. It allows
`PPBranchLevel` to go back down to -1. The existing `PPBranchLevel >= 0` checks
appear to prevent indexing into structures when `PPBranchLevel` is -1.
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
Index: unittests/Format/FormatTest.cpp
===================================================================
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -2286,8 +2286,158 @@
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);
+ // Test with include guards.
+ EXPECT_EQ("#ifndef _SOMEFILE_H\n"
+ "#define _SOMEFILE_H\n"
+ "code();\n"
+ "#endif",
+ format("#ifndef _SOMEFILE_H\n"
+ "#define _SOMEFILE_H\n"
+ "code();\n"
+ "#endif",
+ Style));
+ // Include guards must have a #define with the same variable immediately
+ // after #ifndef.
+ EXPECT_EQ("#ifndef NOT_GUARD\n"
+ "# define FOO\n"
+ "code();\n"
+ "#endif",
+ format("#ifndef NOT_GUARD\n"
+ "# define FOO\n"
+ "code();\n"
+ "#endif",
+ Style));
+
+ // Include guards must cover the entire file.
+ EXPECT_EQ("code();\n"
+ "#ifndef NOT_GUARD\n"
+ "# define NOT_GUARD\n"
+ "code();\n"
+ "#endif",
+ format("code();\n"
+ "#ifndef NOT_GUARD\n"
+ "# define NOT_GUARD\n"
+ "code();\n"
+ "#endif",
+ Style));
+ EXPECT_EQ("#ifndef NOT_GUARD\n"
+ "# define NOT_GUARD\n"
+ "code();\n"
+ "#endif\n"
+ "code();",
+ format("#ifndef NOT_GUARD\n"
+ "# define NOT_GUARD\n"
+ "code();\n"
+ "#endif\n"
+ "code();",
+ Style));
+ // Include guards don't have #else.
+ EXPECT_EQ("#ifndef NOT_GUARD\n"
+ "# define NOT_GUARD\n"
+ "code();\n"
+ "#else\n"
+ "#endif",
+ format("#ifndef NOT_GUARD\n"
+ "# define NOT_GUARD\n"
+ "code();\n"
+ "#else\n"
+ "#endif",
+ Style));
+ EXPECT_EQ("#ifndef NOT_GUARD\n"
+ "# define NOT_GUARD\n"
+ "code();\n"
+ "#elif FOO\n"
+ "#endif",
+ format("#ifndef NOT_GUARD\n"
+ "# define NOT_GUARD\n"
+ "code();\n"
+ "#elif FOO\n"
+ "#endif",
+ Style));
+ // Defect: We currently do not deal with 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_NE("#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: We currently do not deal with cases where legitimate 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_NE("#ifdef WIN32\n"
+ "# pragma once\n"
+ "#endif\n"
+ "#ifndef _SOMEFILE_H\n"
+ "#define _SOMEFILE_H\n"
+ "code();\n"
+ "#endif",
+ format("#ifdef WIN32\n"
+ "# pragma once\n"
+ "#endif\n"
+ "#ifndef _SOMEFILE_H\n"
+ "#define _SOMEFILE_H\n"
+ "code();\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,42 @@
if (IfDef && !IfNDef && FormatTok->TokenText == "SWIG")
Unreachable = true;
conditionalCompilationStart(Unreachable);
+ FormatToken *IfCondition = FormatTok;
+ --PPBranchLevel;
parsePPUnknown();
+ ++PPBranchLevel;
+ // If there's a #ifndef on the first line, it could be an include guard.
+ if (IfNDef && Lines.size() == 1)
+ PPMaybeIncludeGuard = IfCondition;
}
void UnwrappedLineParser::parsePPElse() {
+ // If a potential include guard has a #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 it = Lines.begin(); it != Lines.end(); ++it) {
+ if (it->InPPDirective && it->Level > 0)
+ --it->Level;
+ }
+ }
}
void UnwrappedLineParser::parsePPDefine() {
@@ -713,14 +739,22 @@
parsePPUnknown();
return;
}
+ if (PPMaybeIncludeGuard &&
+ PPMaybeIncludeGuard->TokenText == FormatTok->TokenText &&
+ Lines.size() == 1) {
+ FoundIncludeGuardStart = true;
+ }
+ 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 +768,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/Format.cpp
===================================================================
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -134,6 +134,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) {
IO.enumCase(Value, "None", FormatStyle::RTBS_None);
@@ -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, counting the hash as a column.
+ /// \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, counting the hash as a column.
+
+ .. 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
