Skip to content

Commit ad47c90

Browse files
krasimirggkrasimirgg
committedAug 30, 2017
clang-format: Add preprocessor directive indentation
Summary: 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 (https://github.com/fmauch/clang/tree/preprocessor_indent) 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. This adds a new enum config option, `IndentPPDirectives`. Values are: * `PPDIS_None` (in config: `None`): ``` #if FOO #if BAR #include <foo> #endif #endif ``` * `PPDIS_AfterHash` (in config: `AfterHash`): ``` #if FOO # if BAR # include <foo> # endif #endif ``` This is meant to work whether spaces or tabs are used for indentation. Preprocessor indentation is independent of indentation for non-preprocessor lines. Preprocessor indentation also attempts to ignore include guards with the checks: 1. Include guards cover the entire file 2. Include guards don't have `#else` 3. Include guards begin with ``` #ifndef <var> #define <var> ``` This patch allows `UnwrappedLineParser::PPBranchLevel` to be decremented to -1 (the initial value is -1) so the variable can be used for indent tracking. Defects: * This patch does not 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. This is out of the scope of this patch. * This patch does not handle 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. * 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. * Preprocessor indentation throws off `TokenAnnotator::setCommentLineLevels` so the indentation of comments immediately before indented preprocessor lines is toggled on each run. Fixing this issue appears to be a major change and too much complexity for this patch. Contributed by @euhlmann! Reviewers: djasper, klimek, krasimir Reviewed By: djasper, krasimir Subscribers: krasimir, mzeren-vmw, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D35955 llvm-svn: 312125
1 parent 24aafa5 commit ad47c90

9 files changed

+332
-7
lines changed
 

‎clang/docs/ClangFormatStyleOptions.rst

+27
Original file line numberDiff line numberDiff line change
@@ -1179,6 +1179,33 @@ the configuration (without a prefix: ``Auto``).
11791179
plop(); plop();
11801180
} }
11811181

1182+
**IndentPPDirectives** (``PPDirectiveIndentStyle``)
1183+
Indent preprocessor directives on conditionals.
1184+
1185+
Possible values:
1186+
1187+
* ``PPDIS_None`` (in configuration: ``None``)
1188+
Does not indent any directives.
1189+
1190+
.. code-block:: c++
1191+
1192+
#if FOO
1193+
#if BAR
1194+
#include <foo>
1195+
#endif
1196+
#endif
1197+
1198+
* ``PPDIS_AfterHash`` (in configuration: ``AfterHash``)
1199+
Indents directives after the hash.
1200+
1201+
.. code-block:: c++
1202+
1203+
#if FOO
1204+
# if BAR
1205+
# include <foo>
1206+
# endif
1207+
#endif
1208+
11821209
**IndentWidth** (``unsigned``)
11831210
The number of columns to use for indentation.
11841211

‎clang/include/clang/Format/Format.h

+26
Original file line numberDiff line numberDiff line change
@@ -1024,6 +1024,31 @@ struct FormatStyle {
10241024
/// \endcode
10251025
bool IndentCaseLabels;
10261026

1027+
/// \brief Options for indenting preprocessor directives.
1028+
enum PPDirectiveIndentStyle {
1029+
/// Does not indent any directives.
1030+
/// \code
1031+
/// #if FOO
1032+
/// #if BAR
1033+
/// #include <foo>
1034+
/// #endif
1035+
/// #endif
1036+
/// \endcode
1037+
PPDIS_None,
1038+
/// Indents directives after the hash.
1039+
/// \code
1040+
/// #if FOO
1041+
/// # if BAR
1042+
/// # include <foo>
1043+
/// # endif
1044+
/// #endif
1045+
/// \endcode
1046+
PPDIS_AfterHash
1047+
};
1048+
1049+
/// \brief The preprocessor directive indenting style to use.
1050+
PPDirectiveIndentStyle IndentPPDirectives;
1051+
10271052
/// \brief The number of columns to use for indentation.
10281053
/// \code
10291054
/// IndentWidth: 3
@@ -1514,6 +1539,7 @@ struct FormatStyle {
15141539
ForEachMacros == R.ForEachMacros &&
15151540
IncludeCategories == R.IncludeCategories &&
15161541
IndentCaseLabels == R.IndentCaseLabels &&
1542+
IndentPPDirectives == R.IndentPPDirectives &&
15171543
IndentWidth == R.IndentWidth && Language == R.Language &&
15181544
IndentWrappedFunctionNames == R.IndentWrappedFunctionNames &&
15191545
JavaScriptQuotes == R.JavaScriptQuotes &&

‎clang/lib/Format/ContinuationIndenter.cpp

+24-1
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,13 @@ LineState ContinuationIndenter::getInitialState(unsigned FirstIndent,
9393
LineState State;
9494
State.FirstIndent = FirstIndent;
9595
State.Column = FirstIndent;
96+
// With preprocessor directive indentation, the line starts on column 0
97+
// since it's indented after the hash, but FirstIndent is set to the
98+
// preprocessor indent.
99+
if (Style.IndentPPDirectives == FormatStyle::PPDIS_AfterHash &&
100+
(Line->Type == LT_PreprocessorDirective ||
101+
Line->Type == LT_ImportStatement))
102+
State.Column = 0;
96103
State.Line = Line;
97104
State.NextToken = Line->First;
98105
State.Stack.push_back(ParenState(FirstIndent, FirstIndent,
@@ -376,9 +383,25 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
376383

377384
unsigned Spaces = Current.SpacesRequiredBefore + ExtraSpaces;
378385

386+
// Indent preprocessor directives after the hash if required.
387+
int PPColumnCorrection = 0;
388+
if (Style.IndentPPDirectives == FormatStyle::PPDIS_AfterHash &&
389+
Previous.is(tok::hash) && State.FirstIndent > 0 &&
390+
(State.Line->Type == LT_PreprocessorDirective ||
391+
State.Line->Type == LT_ImportStatement)) {
392+
Spaces += State.FirstIndent;
393+
394+
// For preprocessor indent with tabs, State.Column will be 1 because of the
395+
// hash. This causes second-level indents onward to have an extra space
396+
// after the tabs. We avoid this misalignment by subtracting 1 from the
397+
// column value passed to replaceWhitespace().
398+
if (Style.UseTab != FormatStyle::UT_Never)
399+
PPColumnCorrection = -1;
400+
}
401+
379402
if (!DryRun)
380403
Whitespaces.replaceWhitespace(Current, /*Newlines=*/0, Spaces,
381-
State.Column + Spaces);
404+
State.Column + Spaces + PPColumnCorrection);
382405

383406
// If "BreakBeforeInheritanceComma" mode, don't break within the inheritance
384407
// declaration unless there is multiple inheritance.

‎clang/lib/Format/Format.cpp

+10
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,14 @@ template <> struct ScalarEnumerationTraits<FormatStyle::BreakConstructorInitiali
133133
}
134134
};
135135

136+
template <>
137+
struct ScalarEnumerationTraits<FormatStyle::PPDirectiveIndentStyle> {
138+
static void enumeration(IO &IO, FormatStyle::PPDirectiveIndentStyle &Value) {
139+
IO.enumCase(Value, "None", FormatStyle::PPDIS_None);
140+
IO.enumCase(Value, "AfterHash", FormatStyle::PPDIS_AfterHash);
141+
}
142+
};
143+
136144
template <>
137145
struct ScalarEnumerationTraits<FormatStyle::ReturnTypeBreakingStyle> {
138146
static void enumeration(IO &IO, FormatStyle::ReturnTypeBreakingStyle &Value) {
@@ -350,6 +358,7 @@ template <> struct MappingTraits<FormatStyle> {
350358
IO.mapOptional("IncludeCategories", Style.IncludeCategories);
351359
IO.mapOptional("IncludeIsMainRegex", Style.IncludeIsMainRegex);
352360
IO.mapOptional("IndentCaseLabels", Style.IndentCaseLabels);
361+
IO.mapOptional("IndentPPDirectives", Style.IndentPPDirectives);
353362
IO.mapOptional("IndentWidth", Style.IndentWidth);
354363
IO.mapOptional("IndentWrappedFunctionNames",
355364
Style.IndentWrappedFunctionNames);
@@ -589,6 +598,7 @@ FormatStyle getLLVMStyle() {
589598
{".*", 1}};
590599
LLVMStyle.IncludeIsMainRegex = "(Test)?$";
591600
LLVMStyle.IndentCaseLabels = false;
601+
LLVMStyle.IndentPPDirectives = FormatStyle::PPDIS_None;
592602
LLVMStyle.IndentWrappedFunctionNames = false;
593603
LLVMStyle.IndentWidth = 2;
594604
LLVMStyle.JavaScriptQuotes = FormatStyle::JSQS_Leave;

‎clang/lib/Format/UnwrappedLineFormatter.cpp

+4
Original file line numberDiff line numberDiff line change
@@ -1037,6 +1037,10 @@ void UnwrappedLineFormatter::formatFirstToken(const AnnotatedLine &Line,
10371037
if (RootToken.IsFirst && !RootToken.HasUnescapedNewline)
10381038
Newlines = 0;
10391039

1040+
// Preprocessor directives get indented after the hash, if indented.
1041+
if (Line.Type == LT_PreprocessorDirective || Line.Type == LT_ImportStatement)
1042+
Indent = 0;
1043+
10401044
// Remove empty lines after "{".
10411045
if (!Style.KeepEmptyLinesAtTheStartOfBlocks && PreviousLine &&
10421046
PreviousLine->Last->is(tok::l_brace) &&

‎clang/lib/Format/UnwrappedLineParser.cpp

+57-3
Original file line numberDiff line numberDiff line change
@@ -231,10 +231,15 @@ UnwrappedLineParser::UnwrappedLineParser(const FormatStyle &Style,
231231
: Line(new UnwrappedLine), MustBreakBeforeNextToken(false),
232232
CurrentLines(&Lines), Style(Style), Keywords(Keywords),
233233
CommentPragmasRegex(Style.CommentPragmas), Tokens(nullptr),
234-
Callback(Callback), AllTokens(Tokens), PPBranchLevel(-1) {}
234+
Callback(Callback), AllTokens(Tokens), PPBranchLevel(-1),
235+
IfNdefCondition(nullptr), FoundIncludeGuardStart(false),
236+
IncludeGuardRejected(false) {}
235237

236238
void UnwrappedLineParser::reset() {
237239
PPBranchLevel = -1;
240+
IfNdefCondition = nullptr;
241+
FoundIncludeGuardStart = false;
242+
IncludeGuardRejected = false;
238243
Line.reset(new UnwrappedLine);
239244
CommentsBeforeNextToken.clear();
240245
FormatTok = nullptr;
@@ -679,7 +684,7 @@ void UnwrappedLineParser::conditionalCompilationEnd() {
679684
}
680685
}
681686
// Guard against #endif's without #if.
682-
if (PPBranchLevel > 0)
687+
if (PPBranchLevel > -1)
683688
--PPBranchLevel;
684689
if (!PPChainBranchIndex.empty())
685690
PPChainBranchIndex.pop();
@@ -696,19 +701,53 @@ void UnwrappedLineParser::parsePPIf(bool IfDef) {
696701
if (IfDef && !IfNDef && FormatTok->TokenText == "SWIG")
697702
Unreachable = true;
698703
conditionalCompilationStart(Unreachable);
704+
FormatToken *IfCondition = FormatTok;
705+
// If there's a #ifndef on the first line, and the only lines before it are
706+
// comments, it could be an include guard.
707+
bool MaybeIncludeGuard = IfNDef;
708+
if (!IncludeGuardRejected && !FoundIncludeGuardStart && MaybeIncludeGuard) {
709+
for (auto &Line : Lines) {
710+
if (!Line.Tokens.front().Tok->is(tok::comment)) {
711+
MaybeIncludeGuard = false;
712+
IncludeGuardRejected = true;
713+
break;
714+
}
715+
}
716+
}
717+
--PPBranchLevel;
699718
parsePPUnknown();
719+
++PPBranchLevel;
720+
if (!IncludeGuardRejected && !FoundIncludeGuardStart && MaybeIncludeGuard)
721+
IfNdefCondition = IfCondition;
700722
}
701723

702724
void UnwrappedLineParser::parsePPElse() {
725+
// If a potential include guard has an #else, it's not an include guard.
726+
if (FoundIncludeGuardStart && PPBranchLevel == 0)
727+
FoundIncludeGuardStart = false;
703728
conditionalCompilationAlternative();
729+
if (PPBranchLevel > -1)
730+
--PPBranchLevel;
704731
parsePPUnknown();
732+
++PPBranchLevel;
705733
}
706734

707735
void UnwrappedLineParser::parsePPElIf() { parsePPElse(); }
708736

709737
void UnwrappedLineParser::parsePPEndIf() {
710738
conditionalCompilationEnd();
711739
parsePPUnknown();
740+
// If the #endif of a potential include guard is the last thing in the file,
741+
// then we count it as a real include guard and subtract one from every
742+
// preprocessor indent.
743+
unsigned TokenPosition = Tokens->getPosition();
744+
FormatToken *PeekNext = AllTokens[TokenPosition];
745+
if (FoundIncludeGuardStart && PPBranchLevel == -1 && PeekNext->is(tok::eof)) {
746+
for (auto &Line : Lines) {
747+
if (Line.InPPDirective && Line.Level > 0)
748+
--Line.Level;
749+
}
750+
}
712751
}
713752

714753
void UnwrappedLineParser::parsePPDefine() {
@@ -718,14 +757,26 @@ void UnwrappedLineParser::parsePPDefine() {
718757
parsePPUnknown();
719758
return;
720759
}
760+
if (IfNdefCondition && IfNdefCondition->TokenText == FormatTok->TokenText) {
761+
FoundIncludeGuardStart = true;
762+
for (auto &Line : Lines) {
763+
if (!Line.Tokens.front().Tok->isOneOf(tok::comment, tok::hash)) {
764+
FoundIncludeGuardStart = false;
765+
break;
766+
}
767+
}
768+
}
769+
IfNdefCondition = nullptr;
721770
nextToken();
722771
if (FormatTok->Tok.getKind() == tok::l_paren &&
723772
FormatTok->WhitespaceRange.getBegin() ==
724773
FormatTok->WhitespaceRange.getEnd()) {
725774
parseParens();
726775
}
776+
if (Style.IndentPPDirectives == FormatStyle::PPDIS_AfterHash)
777+
Line->Level += PPBranchLevel + 1;
727778
addUnwrappedLine();
728-
Line->Level = 1;
779+
++Line->Level;
729780

730781
// Errors during a preprocessor directive can only affect the layout of the
731782
// preprocessor directive, and thus we ignore them. An alternative approach
@@ -739,7 +790,10 @@ void UnwrappedLineParser::parsePPUnknown() {
739790
do {
740791
nextToken();
741792
} while (!eof());
793+
if (Style.IndentPPDirectives == FormatStyle::PPDIS_AfterHash)
794+
Line->Level += PPBranchLevel + 1;
742795
addUnwrappedLine();
796+
IfNdefCondition = nullptr;
743797
}
744798

745799
// Here we blacklist certain tokens that are not usually the first token in an

‎clang/lib/Format/UnwrappedLineParser.h

+5
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,11 @@ class UnwrappedLineParser {
246246
// sequence.
247247
std::stack<int> PPChainBranchIndex;
248248

249+
// Contains the #ifndef condition for a potential include guard.
250+
FormatToken *IfNdefCondition;
251+
bool FoundIncludeGuardStart;
252+
bool IncludeGuardRejected;
253+
249254
friend class ScopedLineState;
250255
friend class CompoundStatementIndenter;
251256
};

0 commit comments

Comments
 (0)
Please sign in to comment.