Index: cfe/trunk/docs/ClangFormatStyleOptions.rst =================================================================== --- cfe/trunk/docs/ClangFormatStyleOptions.rst +++ cfe/trunk/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 + #endif + #endif + + * ``PPDIS_AfterHash`` (in configuration: ``AfterHash``) + Indents directives after the hash. + + .. code-block:: c++ + + #if FOO + # if BAR + # include + # endif + #endif + **IndentWidth** (``unsigned``) The number of columns to use for indentation. Index: cfe/trunk/include/clang/Format/Format.h =================================================================== --- cfe/trunk/include/clang/Format/Format.h +++ cfe/trunk/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 + /// #endif + /// #endif + /// \endcode + PPDIS_None, + /// Indents directives after the hash. + /// \code + /// #if FOO + /// # if BAR + /// # include + /// # 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: cfe/trunk/lib/Format/ContinuationIndenter.cpp =================================================================== --- cfe/trunk/lib/Format/ContinuationIndenter.cpp +++ cfe/trunk/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: cfe/trunk/lib/Format/Format.cpp =================================================================== --- cfe/trunk/lib/Format/Format.cpp +++ cfe/trunk/lib/Format/Format.cpp @@ -134,6 +134,14 @@ }; template <> +struct ScalarEnumerationTraits { + 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 { 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: cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp =================================================================== --- cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp +++ cfe/trunk/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: cfe/trunk/lib/Format/UnwrappedLineParser.h =================================================================== --- cfe/trunk/lib/Format/UnwrappedLineParser.h +++ cfe/trunk/lib/Format/UnwrappedLineParser.h @@ -246,6 +246,11 @@ // sequence. std::stack PPChainBranchIndex; + // Contains the #ifndef condition for a potential include guard. + FormatToken *IfNdefCondition; + bool FoundIncludeGuardStart; + bool IncludeGuardRejected; + friend class ScopedLineState; friend class CompoundStatementIndenter; }; Index: cfe/trunk/lib/Format/UnwrappedLineParser.cpp =================================================================== --- cfe/trunk/lib/Format/UnwrappedLineParser.cpp +++ cfe/trunk/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,12 +701,35 @@ 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(); } @@ -709,6 +737,17 @@ 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: cfe/trunk/unittests/Format/FormatTest.cpp =================================================================== --- cfe/trunk/unittests/Format/FormatTest.cpp +++ cfe/trunk/unittests/Format/FormatTest.cpp @@ -2312,8 +2312,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 \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 \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 \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: cfe/trunk/unittests/Format/FormatTestUtils.h =================================================================== --- cfe/trunk/unittests/Format/FormatTestUtils.h +++ cfe/trunk/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;