Index: lib/Format/UnwrappedLineParser.h =================================================================== --- lib/Format/UnwrappedLineParser.h +++ lib/Format/UnwrappedLineParser.h @@ -156,6 +156,11 @@ bool isOnNewLine(const FormatToken &FormatTok); + // Compute hash of the current preprocessor branch. + // This is used to identify the different branches, and thus track if block + // open and close in the same branch. + size_t computePPHash() const; + // FIXME: We are constantly running into bugs where Line.Level is incorrectly // subtracted from beyond 0. Introduce a method to subtract from Line.Level // and use that everywhere in the Parser. @@ -174,7 +179,7 @@ // Preprocessor directives are parsed out-of-order from other unwrapped lines. // Thus, we need to keep a list of preprocessor directives to be reported - // after an unwarpped line that has been started was finished. + // after an unwrapped line that has been started was finished. SmallVector PreprocessorDirectives; // New unwrapped lines are added via CurrentLines. @@ -207,8 +212,14 @@ PP_Unreachable // #if 0 or a conditional preprocessor block inside #if 0 }; + struct PPBranch { + PPBranch(PPBranchKind Kind, size_t Line) : Kind(Kind), Line(Line) {} + PPBranchKind Kind; + size_t Line; + }; + // Keeps a stack of currently active preprocessor branching directives. - SmallVector PPStack; + SmallVector PPStack; // The \c UnwrappedLineParser re-parses the code for each combination // of preprocessor branches that can be taken. Index: lib/Format/UnwrappedLineParser.cpp =================================================================== --- lib/Format/UnwrappedLineParser.cpp +++ lib/Format/UnwrappedLineParser.cpp @@ -452,6 +452,21 @@ FormatTok = Tokens->setPosition(StoredPosition); } +template +static inline void hash_combine(std::size_t &seed, const T &v) { + std::hash hasher; + seed ^= hasher(v) + 0x9e3779b9 + (seed << 6) + (seed >> 2); +} + +size_t UnwrappedLineParser::computePPHash() const { + size_t h = 0; + for (const auto &i : PPStack) { + hash_combine(h, size_t(i.Kind)); + hash_combine(h, i.Line); + } + return h; +} + void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, bool AddLevel, bool MunchSemi) { assert(FormatTok->isOneOf(tok::l_brace, TT_MacroBlockBegin) && @@ -459,16 +474,21 @@ const bool MacroBlock = FormatTok->is(TT_MacroBlockBegin); FormatTok->BlockKind = BK_Block; + size_t PPStartHash = computePPHash(); + unsigned InitialLevel = Line->Level; nextToken(); if (MacroBlock && FormatTok->is(tok::l_paren)) parseParens(); + size_t NbPreprocessorDirectives = + CurrentLines == &Lines ? PreprocessorDirectives.size() : 0; addUnwrappedLine(); - size_t OpeningLineIndex = CurrentLines->empty() - ? (UnwrappedLine::kInvalidIndex) - : (CurrentLines->size() - 1); + size_t OpeningLineIndex = + CurrentLines->empty() + ? (UnwrappedLine::kInvalidIndex) + : (CurrentLines->size() - 1 - NbPreprocessorDirectives); ScopedDeclarationState DeclarationState(*Line, DeclarationScopeStack, MustBeDeclaration); @@ -486,6 +506,7 @@ return; } + size_t PPEndHash = computePPHash(); nextToken(); // Munch the closing brace. if (MacroBlock && FormatTok->is(tok::l_paren)) @@ -494,11 +515,14 @@ if (MunchSemi && FormatTok->Tok.is(tok::semi)) nextToken(); Line->Level = InitialLevel; - Line->MatchingOpeningBlockLineIndex = OpeningLineIndex; - if (OpeningLineIndex != UnwrappedLine::kInvalidIndex) { - // Update the opening line to add the forward reference as well - (*CurrentLines)[OpeningLineIndex].MatchingOpeningBlockLineIndex = - CurrentLines->size() - 1; + + if (PPStartHash == PPEndHash) { + Line->MatchingOpeningBlockLineIndex = OpeningLineIndex; + if (OpeningLineIndex != UnwrappedLine::kInvalidIndex) { + // Update the opening line to add the forward reference as well + (*CurrentLines)[OpeningLineIndex].MatchingOpeningBlockLineIndex = + CurrentLines->size() - 1; + } } } @@ -606,10 +630,15 @@ } void UnwrappedLineParser::conditionalCompilationCondition(bool Unreachable) { - if (Unreachable || (!PPStack.empty() && PPStack.back() == PP_Unreachable)) - PPStack.push_back(PP_Unreachable); + size_t Line = CurrentLines->size(); + if (CurrentLines == &PreprocessorDirectives) + Line += Lines.size(); + + if (Unreachable || + (!PPStack.empty() && PPStack.back().Kind == PP_Unreachable)) + PPStack.push_back({PP_Unreachable, Line}); else - PPStack.push_back(PP_Conditional); + PPStack.push_back({PP_Conditional, Line}); } void UnwrappedLineParser::conditionalCompilationStart(bool Unreachable) { @@ -2395,7 +2424,7 @@ FormatTok->MustBreakBefore = true; } - if (!PPStack.empty() && (PPStack.back() == PP_Unreachable) && + if (!PPStack.empty() && (PPStack.back().Kind == PP_Unreachable) && !Line->InPPDirective) { continue; } Index: unittests/Format/NamespaceEndCommentsFixerTest.cpp =================================================================== --- unittests/Format/NamespaceEndCommentsFixerTest.cpp +++ unittests/Format/NamespaceEndCommentsFixerTest.cpp @@ -509,6 +509,134 @@ "}\n")); } +TEST_F(NamespaceEndCommentsFixerTest, AddEndCommentForNamespacesAroundMacros) { + // Conditional blocks around are fine + EXPECT_EQ("namespace A {\n" + "#if 1\n" + "int i;\n" + "#endif\n" + "}// namespace A", + fixNamespaceEndComments("namespace A {\n" + "#if 1\n" + "int i;\n" + "#endif\n" + "}")); + EXPECT_EQ("#if 1\n" + "#endif\n" + "namespace A {\n" + "int i;\n" + "int j;\n" + "}// namespace A", + fixNamespaceEndComments("#if 1\n" + "#endif\n" + "namespace A {\n" + "int i;\n" + "int j;\n" + "}")); + EXPECT_EQ("namespace A {\n" + "int i;\n" + "int j;\n" + "}// namespace A\n" + "#if 1\n" + "#endif", + fixNamespaceEndComments("namespace A {\n" + "int i;\n" + "int j;\n" + "}\n" + "#if 1\n" + "#endif")); + EXPECT_EQ("#if 1\n" + "namespace A {\n" + "int i;\n" + "int j;\n" + "}// namespace A\n" + "#endif", + fixNamespaceEndComments("#if 1\n" + "namespace A {\n" + "int i;\n" + "int j;\n" + "}\n" + "#endif")); + + // Macro definition has no impact + EXPECT_EQ("namespace A {\n" + "#define FOO\n" + "int i;\n" + "}// namespace A", + fixNamespaceEndComments("namespace A {\n" + "#define FOO\n" + "int i;\n" + "}")); + EXPECT_EQ("#define FOO\n" + "namespace A {\n" + "int i;\n" + "int j;\n" + "}// namespace A", + fixNamespaceEndComments("#define FOO\n" + "namespace A {\n" + "int i;\n" + "int j;\n" + "}")); + EXPECT_EQ("namespace A {\n" + "int i;\n" + "int j;\n" + "}// namespace A\n" + "#define FOO\n", + fixNamespaceEndComments("namespace A {\n" + "int i;\n" + "int j;\n" + "}\n" + "#define FOO\n")); + + // No replacement if open & close in different conditional blocks + EXPECT_EQ("#if 1\n" + "namespace A {\n" + "#endif\n" + "int i;\n" + "int j;\n" + "#if 1\n" + "}\n" + "#endif", + fixNamespaceEndComments("#if 1\n" + "namespace A {\n" + "#endif\n" + "int i;\n" + "int j;\n" + "#if 1\n" + "}\n" + "#endif")); + EXPECT_EQ("#ifdef A\n" + "namespace A {\n" + "#endif\n" + "int i;\n" + "int j;\n" + "#ifdef B\n" + "}\n" + "#endif", + fixNamespaceEndComments("#ifdef A\n" + "namespace A {\n" + "#endif\n" + "int i;\n" + "int j;\n" + "#ifdef B\n" + "}\n" + "#endif")); + + // No replacement inside unreachable conditional block + EXPECT_EQ("#if 0\n" + "namespace A {\n" + "int i;\n" + "int j;\n" + "}\n" + "#endif", + fixNamespaceEndComments("#if 0\n" + "namespace A {\n" + "int i;\n" + "int j;\n" + "}\n" + "#endif")); +} + TEST_F(NamespaceEndCommentsFixerTest, DoesNotAddEndCommentForNamespacesInMacroDeclarations) { EXPECT_EQ("#ifdef 1\n"