diff --git a/clang/lib/Format/UnwrappedLineParser.h b/clang/lib/Format/UnwrappedLineParser.h --- a/clang/lib/Format/UnwrappedLineParser.h +++ b/clang/lib/Format/UnwrappedLineParser.h @@ -18,6 +18,7 @@ #include "FormatToken.h" #include "clang/Basic/IdentifierTable.h" #include "clang/Format/Format.h" +#include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/BitVector.h" #include "llvm/Support/Regex.h" #include @@ -314,11 +315,73 @@ // Contains the maximum number of branches at each nesting level. SmallVector PPLevelBranchCount; - // Contains the number of branches per nesting level we are currently - // in while parsing a preprocessor branch sequence. + // Contains the indices of the current if-section and branch in that section + // we are currently in while parsing a preprocessor branch sequence, with one + // entry per nesting level. // This is used to update PPLevelBranchCount at the end of a branch // sequence. - std::stack PPChainBranchIndex; + // For example: + // PPSectionsTop = 0 + // PPChainBranchIndex = [] + // #if 0 + // PPSectionsTop = 0 -- It gets reset to 0 when a new level is entered. + // PPChainBranchIndex = [{Section: 0, Branch: 0}] + // #elif 1 + // PPSectionsTop = 0 + // PPChainBranchIndex = [{Section: 0, Branch: 1}] + // #endif + // PPSectionsTop = 1 + // PPChainBranchindex = [] + // #if 2 + // PPSectionsTop = 0 + // PPChainBranchIndex = [{Section: 1, Branch: 0}] + // #if 3 + // PPSectionsTop = 0 + // PPChainBranchIndex = [{Section: 1, Branch: 0}, + // {Section: 0, Branch: 0}] + // #endif + // PPSectionsTop = 1 -- It gets incremented when a section ends. + // PPChainBranchIndex = [{Section: 1, Branch: 0}] + // #endif + // PPSectionsTop = 2 + // PPChainBranchIndex = [] + // After parsing all this, PPBranchStructure is: + // { + // Unreachable: true + // Sections: [ + // [ + // {Unreachable: false, Sections: []}, + // {Unreachable: false, Sections: []} + // ], + // [ + // { + // Unreachable: false, + // Sections: [[{Unreachable: false, Sections: []}]] + // } + // ] + // ] + // } + struct PPSectionGroupId { + int Section; + int Branch; + }; + SmallVector PPChainBranchIndex; + + // The number of sections seen at the current level. + int PPSectionsCurrent; + + // The structure of the #if branches. 'Sections' contains one entry for each + // section between #if and #endif regardless of how many #elsif there are in + // between. For each of the sections, there is one entry for each branch. + struct PPBranchStructureT { + std::vector> Sections; + bool Unreachable = false; + } PPBranchStructure; + + // Update the saved branch structure based on the current number of branches + // seen. Returns the structure addressed by the indices. + PPBranchStructureT *findPPBranchStructure(PPBranchStructureT *Structure, + ArrayRef Indices); // Include guard search state. Used to fixup preprocessor indent levels // so that include guards do not participate in indentation. diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp --- a/clang/lib/Format/UnwrappedLineParser.cpp +++ b/clang/lib/Format/UnwrappedLineParser.cpp @@ -346,6 +346,7 @@ NestedTooDeep.clear(); PPStack.clear(); Line->FirstStartColumn = FirstStartColumn; + PPSectionsCurrent = 0; } void UnwrappedLineParser::parse() { @@ -1125,11 +1126,48 @@ } void UnwrappedLineParser::conditionalCompilationCondition(bool Unreachable) { + bool Skip; + if (PPChainBranchIndex.empty()) { + // This condition occurs when there is an #elif without an #if. + Skip = false; + } else { + assert(PPBranchLevel >= 0); + const auto &CurrentIndex = PPChainBranchIndex.back(); + PPBranchStructureT *Structure = findPPBranchStructure( + &PPBranchStructure, makeArrayRef(PPChainBranchIndex).drop_back()); + findPPBranchStructure(Structure, + ArrayRef(PPChainBranchIndex.back())) + ->Unreachable = Unreachable; + // The default branch to be parsed when PPLevelBranchIndex calls for a + // nonexistent branch is the first branch when it is reachable, otherwise + // the second branch. + bool IsDefaultBranch = + CurrentIndex.Branch == 0 || + (CurrentIndex.Branch == 1 && + Structure->Sections[CurrentIndex.Section][0].Unreachable); + if (PPLevelBranchIndex[PPBranchLevel] == CurrentIndex.Branch) { + // The branch is parsed when called for. + Skip = false; + } else if (IsDefaultBranch && PPLevelBranchIndex[PPBranchLevel] == 0) { + // When the branch called for is the first one, but it is unreachable, the + // second one is parsed. + Skip = false; + } else if (IsDefaultBranch && + PPLevelBranchIndex[PPBranchLevel] >= + static_cast( + Structure->Sections[CurrentIndex.Section].size())) { + // When the branch called for doesn't exist, the default branch is parsed. + Skip = false; + } else { + Skip = true; + } + } + size_t Line = CurrentLines->size(); if (CurrentLines == &PreprocessorDirectives) Line += Lines.size(); - if (Unreachable || + if (Unreachable || Skip || (!PPStack.empty() && PPStack.back().Kind == PP_Unreachable)) { PPStack.push_back({PP_Unreachable, Line}); } else { @@ -1141,14 +1179,12 @@ ++PPBranchLevel; assert(PPBranchLevel >= 0 && PPBranchLevel <= (int)PPLevelBranchIndex.size()); if (PPBranchLevel == (int)PPLevelBranchIndex.size()) { - // If the first branch is unreachable, set the BranchIndex to 1. This way - // the next branch will be parsed if there is one. - PPLevelBranchIndex.push_back(Unreachable ? 1 : 0); + PPLevelBranchIndex.push_back(0); PPLevelBranchCount.push_back(0); } - PPChainBranchIndex.push(0); - bool Skip = PPLevelBranchIndex[PPBranchLevel] > 0; - conditionalCompilationCondition(Unreachable || Skip); + PPChainBranchIndex.push_back({/*Section=*/PPSectionsCurrent, /*Branch=*/0}); + PPSectionsCurrent = 0; + conditionalCompilationCondition(Unreachable); } void UnwrappedLineParser::conditionalCompilationAlternative() { @@ -1156,27 +1192,47 @@ PPStack.pop_back(); assert(PPBranchLevel < (int)PPLevelBranchIndex.size()); if (!PPChainBranchIndex.empty()) - ++PPChainBranchIndex.top(); - conditionalCompilationCondition( - PPBranchLevel >= 0 && !PPChainBranchIndex.empty() && - PPLevelBranchIndex[PPBranchLevel] != PPChainBranchIndex.top()); + ++PPChainBranchIndex.back().Branch; + conditionalCompilationCondition(/*Unreachable=*/false); } void UnwrappedLineParser::conditionalCompilationEnd() { assert(PPBranchLevel < (int)PPLevelBranchIndex.size()); if (PPBranchLevel >= 0 && !PPChainBranchIndex.empty()) { - if (PPChainBranchIndex.top() + 1 > PPLevelBranchCount[PPBranchLevel]) - PPLevelBranchCount[PPBranchLevel] = PPChainBranchIndex.top() + 1; + if (PPChainBranchIndex.back().Branch + 1 > + PPLevelBranchCount[PPBranchLevel]) { + PPLevelBranchCount[PPBranchLevel] = PPChainBranchIndex.back().Branch + 1; + } } // Guard against #endif's without #if. if (PPBranchLevel > -1) --PPBranchLevel; - if (!PPChainBranchIndex.empty()) - PPChainBranchIndex.pop(); + if (PPChainBranchIndex.empty()) { + PPSectionsCurrent = 0; + } else { + PPSectionsCurrent = PPChainBranchIndex.back().Section + 1; + PPChainBranchIndex.pop_back(); + } if (!PPStack.empty()) PPStack.pop_back(); } +UnwrappedLineParser::PPBranchStructureT * +UnwrappedLineParser::findPPBranchStructure(PPBranchStructureT *Structure, + ArrayRef Indices) { + if (Indices.empty()) + return Structure; + if (Indices[0].Section >= static_cast(Structure->Sections.size())) + Structure->Sections.resize(Indices[0].Section + 1); + if (Indices[0].Branch >= + static_cast(Structure->Sections[Indices[0].Section].size())) { + Structure->Sections[Indices[0].Section].resize(Indices[0].Branch + 1); + } + return findPPBranchStructure( + &Structure->Sections[Indices[0].Section][Indices[0].Branch], + Indices.drop_front()); +} + void UnwrappedLineParser::parsePPIf(bool IfDef) { bool IfNDef = FormatTok->is(tok::pp_ifndef); nextToken(); diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -6140,6 +6140,73 @@ "#endif\n" " x;\n" "}"); + // Verify that an `#if 0` at the start of a file doesn't affect other parts of + // the file. + verifyFormat("#if 0\n" + "#endif\n" + "#if X\n" + "int something_fairly_long;\n" + "if (1) {\n" + " int something_fairly_long;\n" + "}\n" + "#endif\n"); + // When sections have different numbers of branches, the extra branches should + // be parsed along with the first branches of the sections that have fewer + // branches if the condition of those first branches are not 0. + verifyFormat("#if 1\n" + "{\n" + "#endif\n" + "#if X\n" + " x;\n" + "#else\n" + " x;\n" + "#endif\n" + "}"); + // When the condition is 0, the default branch is the second branch. + verifyFormat("#if 0\n" + "#elif 1\n" + "{\n" + "#endif\n" + "#if X\n" + " x;\n" + "#elif X\n" + " x;\n" + "#else\n" + " x;\n" + "#endif"); + // Nested. + verifyFormat("#if X\n" + "#if X\n" + "#if X\n" + "{\n" + "#endif\n" + " x;\n" + "#endif\n" + " x;\n" + "#endif\n" + "#if X\n" + " x;\n" + "#else\n" + " x;\n" + "#endif\n" + "}"); + verifyFormat("#if X\n" + "#if X\n" + "#if X\n" + "{\n" + "#endif\n" + "#endif\n" + "#endif\n" + "#if X\n" + "#if X\n" + "#if X\n" + " x;\n" + "#else\n" + " x;\n" + "#endif\n" + "#endif\n" + "#endif\n" + "}"); } TEST_F(FormatTest, GraciouslyHandleIncorrectPreprocessorConditions) { @@ -18550,6 +18617,27 @@ Style); } +TEST_F(FormatTest, AlignComments) { + verifyFormat("int a; // Do\n" + "double b; // align comments."); + + FormatStyle Style = getLLVMStyle(); + Style.AlignTrailingComments = false; + verifyFormat("int a; // Do not\n" + "double b; // align comments.", + Style); + + // #58188 `#if 0` should not cause problems. + verifyFormat("#if X\n" + "int something_fairly_long; // Align here please\n" + "#endif // Should be aligned"); + verifyFormat("#if 0\n" + "#endif\n" + "#if X\n" + "int something_fairly_long; // Align here please\n" + "#endif // Should be aligned"); +} + TEST_F(FormatTest, LinuxBraceBreaking) { FormatStyle LinuxBraceStyle = getLLVMStyle(); LinuxBraceStyle.BreakBeforeBraces = FormatStyle::BS_Linux;