diff --git a/clang-tools-extra/clangd/unittests/tweaks/AddUsingTests.cpp b/clang-tools-extra/clangd/unittests/tweaks/AddUsingTests.cpp --- a/clang-tools-extra/clangd/unittests/tweaks/AddUsingTests.cpp +++ b/clang-tools-extra/clangd/unittests/tweaks/AddUsingTests.cpp @@ -71,6 +71,13 @@ // NestedNameSpecifier, but no namespace. EXPECT_UNAVAILABLE(Header + "class Foo {}; class F^oo foo;"); + // Nested macro case. + EXPECT_AVAILABLE(R"cpp( + #define ID2(X) X + #define ID(Y, X) Y;ID2(X) + namespace ns { struct Foo{}; } + ID(int xyz, ns::F^oo) f;)cpp"); + // Check that we do not trigger in header files. FileName = "test.h"; ExtraArgs.push_back("-xc++-header"); // .h file is treated a C by default. diff --git a/clang/lib/Tooling/Syntax/Tokens.cpp b/clang/lib/Tooling/Syntax/Tokens.cpp --- a/clang/lib/Tooling/Syntax/Tokens.cpp +++ b/clang/lib/Tooling/Syntax/Tokens.cpp @@ -103,66 +103,13 @@ // The token `a` is wrapped in 4 arg-expansions, we only want to unwrap 2. // We distinguish them by whether the macro expands into the target file. // Fortunately, the target file ones will always appear first. - auto &ExpMacro = - SM.getSLocEntry(SM.getFileID(ExpFirst.getExpansionLocStart())) - .getExpansion(); - if (ExpMacro.getExpansionLocStart().isMacroID()) + auto ExpFileID = SM.getFileID(ExpFirst.getExpansionLocStart()); + if (ExpFileID == TargetFile) break; // Replace each endpoint with its spelling inside the macro arg. // (This is getImmediateSpellingLoc without repeating lookups). First = ExpFirst.getSpellingLoc().getLocWithOffset(DecFirst.second); Last = ExpLast.getSpellingLoc().getLocWithOffset(DecLast.second); - - // Now: how do we adjust the previous/next bounds? Three cases: - // A) If they are also part of the same macro arg, we translate them too. - // This will ensure that we don't select any macros nested within the - // macro arg that cover extra tokens. Critical case: - // #define ID(X) X - // ID(prev target) // selecting 'target' succeeds - // #define LARGE ID(prev target) - // LARGE // selecting 'target' fails. - // B) They are not in the macro at all, then their expansion range is a - // sibling to it, and we can safely substitute that. - // #define PREV prev - // #define ID(X) X - // PREV ID(target) // selecting 'target' succeeds. - // #define LARGE PREV ID(target) - // LARGE // selecting 'target' fails. - // C) They are in a different arg of this macro, or the macro body. - // Now selecting the whole macro arg is fine, but the whole macro is not. - // Model this by setting using the edge of the macro call as the bound. - // #define ID2(X, Y) X Y - // ID2(prev, target) // selecting 'target' succeeds - // #define LARGE ID2(prev, target) - // LARGE // selecting 'target' fails - auto AdjustBound = [&](SourceLocation &Bound) { - if (Bound.isInvalid() || !Bound.isMacroID()) // Non-macro must be case B. - return; - auto DecBound = SM.getDecomposedLoc(Bound); - auto &ExpBound = SM.getSLocEntry(DecBound.first).getExpansion(); - if (ExpBound.isMacroArgExpansion() && - ExpBound.getExpansionLocStart() == ExpFirst.getExpansionLocStart()) { - // Case A: translate to (spelling) loc within the macro arg. - Bound = ExpBound.getSpellingLoc().getLocWithOffset(DecBound.second); - return; - } - while (Bound.isMacroID()) { - SourceRange Exp = SM.getImmediateExpansionRange(Bound).getAsRange(); - if (Exp.getBegin() == ExpMacro.getExpansionLocStart()) { - // Case B: bounds become the macro call itself. - Bound = (&Bound == &Prev) ? Exp.getBegin() : Exp.getEnd(); - return; - } - // Either case C, or expansion location will later find case B. - // We choose the upper bound for Prev and the lower one for Next: - // ID(prev) target ID(next) - // ^ ^ - // new-prev new-next - Bound = (&Bound == &Prev) ? Exp.getEnd() : Exp.getBegin(); - } - }; - AdjustBound(Prev); - AdjustBound(Next); } // In all remaining cases we need the full containing macros. @@ -170,9 +117,10 @@ SourceRange Candidate = SM.getExpansionRange(SourceRange(First, Last)).getAsRange(); auto DecFirst = SM.getDecomposedExpansionLoc(Candidate.getBegin()); - auto DecLast = SM.getDecomposedLoc(Candidate.getEnd()); + auto DecLast = SM.getDecomposedExpansionLoc(Candidate.getEnd()); // Can end up in the wrong file due to bad input or token-pasting shenanigans. - if (Candidate.isInvalid() || DecFirst.first != TargetFile || DecLast.first != TargetFile) + if (Candidate.isInvalid() || DecFirst.first != TargetFile || + DecLast.first != TargetFile) return SourceRange(); // Check bounds, which may still be inside macros. if (Prev.isValid()) { diff --git a/clang/unittests/Tooling/Syntax/TokensTest.cpp b/clang/unittests/Tooling/Syntax/TokensTest.cpp --- a/clang/unittests/Tooling/Syntax/TokensTest.cpp +++ b/clang/unittests/Tooling/Syntax/TokensTest.cpp @@ -769,12 +769,15 @@ // Critical cases for mapping of Prev/Next in spelledForExpandedSlow. recordTokens(R"cpp( #define ID(X) X - ID(prev ID(good)) + ID(prev good) + ID(prev ID(good2)) #define LARGE ID(prev ID(bad)) LARGE )cpp"); EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("good")), ValueIs(SameRange(findSpelled("good")))); + EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("good2")), + ValueIs(SameRange(findSpelled("good2")))); EXPECT_EQ(Buffer.spelledForExpanded(findExpanded("bad")), std::nullopt); recordTokens(R"cpp( @@ -785,19 +788,32 @@ LARGE )cpp"); EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("good")), - ValueIs(SameRange(findSpelled("good")))); + ValueIs(SameRange(findSpelled("good")))); EXPECT_EQ(Buffer.spelledForExpanded(findExpanded("bad")), std::nullopt); recordTokens(R"cpp( #define ID(X) X #define ID2(X, Y) X Y - ID2(prev, ID(good)) + ID2(prev, good) + ID2(prev, ID(good2)) #define LARGE ID2(prev, bad) LARGE )cpp"); EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("good")), - ValueIs(SameRange(findSpelled("good")))); + ValueIs(SameRange(findSpelled("good")))); + EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("good2")), + ValueIs(SameRange(findSpelled("good2")))); EXPECT_EQ(Buffer.spelledForExpanded(findExpanded("bad")), std::nullopt); + + // Prev from macro body. + recordTokens(R"cpp( + #define ID(X) X + #define ID2(X, Y) X prev ID(Y) + ID2(not_prev, good) + )cpp"); + EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("good")), + ValueIs(SameRange(findSpelled("good")))); + EXPECT_EQ(Buffer.spelledForExpanded(findExpanded("prev good")), std::nullopt); } TEST_F(TokenBufferTest, ExpandedTokensForRange) {