Index: lib/Format/UnwrappedLineParser.h =================================================================== --- lib/Format/UnwrappedLineParser.h +++ lib/Format/UnwrappedLineParser.h @@ -155,6 +155,7 @@ void conditionalCompilationEnd(); bool isOnNewLine(const FormatToken &FormatTok); + 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 @@ -174,7 +175,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 +208,15 @@ PP_Unreachable // #if 0 or a conditional preprocessor block inside #if 0 }; + struct PPBranch { + PPBranch(PPBranchKind Kind, size_t Line) : Kind(Kind), Line(Line) {} + bool operator==(PPBranchKind Kind) const { return Kind == this->Kind; } + 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, 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,14 @@ } void UnwrappedLineParser::conditionalCompilationCondition(bool Unreachable) { + size_t Line = CurrentLines->size(); + if (CurrentLines == &PreprocessorDirectives) + Line += Lines.size(); + if (Unreachable || (!PPStack.empty() && PPStack.back() == PP_Unreachable)) - PPStack.push_back(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) { 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"