euhlmann created this revision.
euhlmann added a project: clang.
This is an implementation for bug 17362
<https://bugs.llvm.org/attachment.cgi?bugid=17362> which adds support for
indenting preprocessor statements inside if/ifdef/endif. This takes previous
work from fmauch and makes it into a full feature.
The context of this patch is that I'm a VMware intern, and I implemented this
because VMware needs the feature. As such, some decisions were made based on
what VMware wants, and I would appreciate suggestions on expanding this if
necessary to use-cases other people may want.
Example:
#if FOO #if FOO
#define BAR -> # define BAR
#endif #endif
- It's controlled by a new bool config option `IndentPPDirectives`
- Indenting happens after the hash but the hash counts as a column. In other
words, one space is inserted for the first indent level and two spaces for
every other level.
#if FOO
# if BAR
# include <foo>
# endif
#endif
This suited VMware's needs, but if not counting the hash as a column is
significant, this could be configurable instead.
- Tabs are supported if enabled with `UseTab`
- There's a heuristic for detecting include guards and not indenting them. It
looks for the pattern
#ifndef <var>
immediately followed by
#define <var>
This was chosen because it's simple, but it will cause problems if that pattern
appears when it's not meant to be an include guard.
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
@@ -2215,8 +2215,55 @@
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 = false;
+ 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 = true;
+ 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);
+ // don't indent include guards
+ // verifyFormat() currently calls test::messUp() which incorrectly merges L2
+ // and L3, so call EXPECT_EQ manually
+ 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));
+
+ // make sure it works even with tabs
+ Style.UseTab = FormatStyle::UT_Always;
+ Style.IndentWidth = 8;
+ Style.TabWidth = 8;
+ verifyFormat("#ifdef _WIN32\n"
+ "#\tdefine C 1\n"
+ "#endif", Style);
}
TEST_F(FormatTest, FormatHashIfNotAtStartOfLine) {
Index: lib/Format/UnwrappedLineParser.h
===================================================================
--- lib/Format/UnwrappedLineParser.h
+++ lib/Format/UnwrappedLineParser.h
@@ -235,6 +235,9 @@
// sequence.
std::stack<int> PPChainBranchIndex;
+ unsigned PPIndentLevel;
+ FormatToken *PPMaybeIncludeGuard;
+
friend class ScopedLineState;
friend class CompoundStatementIndenter;
};
Index: lib/Format/UnwrappedLineParser.cpp
===================================================================
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -231,7 +231,8 @@
: 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),
+ PPIndentLevel(0), PPMaybeIncludeGuard(nullptr) {}
void UnwrappedLineParser::reset() {
PPBranchLevel = -1;
@@ -661,17 +662,29 @@
if (IfDef && !IfNDef && FormatTok->TokenText == "SWIG")
Unreachable = true;
conditionalCompilationStart(Unreachable);
+ FormatToken *IfCondition = FormatTok;
parsePPUnknown();
+ if (IfNDef) {
+ PPMaybeIncludeGuard = IfCondition;
+ }
+ ++PPIndentLevel;
}
void UnwrappedLineParser::parsePPElse() {
+ if (PPIndentLevel > 0) {
+ --PPIndentLevel;
+ }
conditionalCompilationAlternative();
parsePPUnknown();
+ ++PPIndentLevel;
}
void UnwrappedLineParser::parsePPElIf() { parsePPElse(); }
void UnwrappedLineParser::parsePPEndIf() {
+ if (PPIndentLevel > 0) {
+ --PPIndentLevel;
+ }
conditionalCompilationEnd();
parsePPUnknown();
}
@@ -683,14 +696,22 @@
parsePPUnknown();
return;
}
+ if (PPMaybeIncludeGuard != nullptr &&
+ PPMaybeIncludeGuard->TokenText == FormatTok->TokenText &&
+ PPIndentLevel > 0) {
+ --PPIndentLevel;
+ }
+ PPMaybeIncludeGuard = nullptr;
nextToken();
if (FormatTok->Tok.getKind() == tok::l_paren &&
FormatTok->WhitespaceRange.getBegin() ==
FormatTok->WhitespaceRange.getEnd()) {
parseParens();
}
+ if (Style.IndentPPDirectives)
+ Line->Level += PPIndentLevel;
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
@@ -704,7 +725,10 @@
do {
nextToken();
} while (!eof());
+ if (Style.IndentPPDirectives)
+ Line->Level += PPIndentLevel;
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
@@ -1018,6 +1018,12 @@
if (RootToken.IsFirst && !RootToken.HasUnescapedNewline)
Newlines = 0;
+ // Preprocessor directives get indented after the hash.
+ 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
@@ -350,6 +350,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 +590,7 @@
{".*", 1}};
LLVMStyle.IncludeIsMainRegex = "(Test)?$";
LLVMStyle.IndentCaseLabels = false;
+ LLVMStyle.IndentPPDirectives = false;
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 ((State.Line->Type == LT_PreprocessorDirective ||
+ State.Line->Type == LT_ImportStatement) &&
+ Current.Previous->is(tok::hash) && State.FirstIndent > 0) {
+ // subtract 1 so indent lines up with non-preprocessor code
+ Spaces += State.FirstIndent;
+ // when tabs are not enabled, subtract one so the indent still lines up with
+ // non-preprocessor code
+ if (Style.UseTab == FormatStyle::UT_Never && Spaces > 0) {
+ --Spaces;
+ }
+ }
+
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,19 @@
/// \endcode
bool IndentCaseLabels;
+ /// \brief Indent preprocessor directives.
+ /// \code
+ /// true:
+ /// #ifdef FOO
+ /// # define A 0
+ /// #endif
+ ///
+ /// false:
+ /// #ifdef FOO
+ /// #define A 0
+ /// #endif
+ bool IndentPPDirectives;
+
/// \brief The number of columns to use for indentation.
/// \code
/// IndentWidth: 3
@@ -1514,6 +1527,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,16 @@
plop(); plop();
} }
+**IndentPPDirectives** (``bool``)
+ Indent preprocessor directives on conditionals.
+
+ .. code-block:: c++
+
+ true: false:
+ #ifdef FOO vs. #ifdef FOO
+ # define A 0 #define A 0
+ #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
