diff --git a/clang/lib/Tooling/Transformer/SourceCode.cpp b/clang/lib/Tooling/Transformer/SourceCode.cpp index 7adceda05ad4f..0ca5113035f7b 100644 --- a/clang/lib/Tooling/Transformer/SourceCode.cpp +++ b/clang/lib/Tooling/Transformer/SourceCode.cpp @@ -86,32 +86,35 @@ llvm::Error clang::tooling::validateEditRange(const CharSourceRange &Range, return validateRange(Range, SM, /*AllowSystemHeaders=*/false); } -// Returns the location of the top-level macro argument that is the spelling for -// the expansion `Loc` is from. If `Loc` is spelled in the macro definition, -// returns an invalid `SourceLocation`. -static SourceLocation getMacroArgumentSpellingLoc(SourceLocation Loc, - const SourceManager &SM) { +// Returns the full set of expansion locations of `Loc` from bottom to top-most +// macro, if `Loc` is spelled in a macro argument. If `Loc` is spelled in the +// macro definition, returns an empty vector. +static llvm::SmallVector +getMacroArgumentExpansionLocs(SourceLocation Loc, const SourceManager &SM) { assert(Loc.isMacroID() && "Location must be in a macro"); + llvm::SmallVector ArgLocs; while (Loc.isMacroID()) { const auto &Expansion = SM.getSLocEntry(SM.getFileID(Loc)).getExpansion(); if (Expansion.isMacroArgExpansion()) { // Check the spelling location of the macro arg, in case the arg itself is // in a macro expansion. Loc = Expansion.getSpellingLoc(); + ArgLocs.push_back(Expansion.getExpansionLocStart()); } else { return {}; } } - return Loc; + return ArgLocs; } static bool spelledInMacroDefinition(CharSourceRange Range, const SourceManager &SM) { if (Range.getBegin().isMacroID() && Range.getEnd().isMacroID()) { - // Check whether the range is entirely within a single macro argument. - auto B = getMacroArgumentSpellingLoc(Range.getBegin(), SM); - auto E = getMacroArgumentSpellingLoc(Range.getEnd(), SM); - return B.isInvalid() || B != E; + // Check whether the range is entirely within a single macro argument by + // checking if they are in the same macro argument at every level. + auto B = getMacroArgumentExpansionLocs(Range.getBegin(), SM); + auto E = getMacroArgumentExpansionLocs(Range.getEnd(), SM); + return B.empty() || B != E; } return Range.getBegin().isMacroID() || Range.getEnd().isMacroID(); diff --git a/clang/unittests/Tooling/SourceCodeTest.cpp b/clang/unittests/Tooling/SourceCodeTest.cpp index a998954a6e9ea..d0eba43b0bb1c 100644 --- a/clang/unittests/Tooling/SourceCodeTest.cpp +++ b/clang/unittests/Tooling/SourceCodeTest.cpp @@ -507,17 +507,21 @@ TEST(SourceCodeTest, EditInvolvingExpansionIgnoringExpansionShouldFail) { // If we specify to ignore macro expansions, none of these call expressions // should have an editable range. llvm::Annotations Code(R"cpp( +#define NOOP(x) x #define M1(x) x(1) -#define M2(x, y) x ## y +#define M2(x, y) NOOP(M2_IMPL(x, y)) +#define M2_IMPL(x, y) x ## y #define M3(x) foobar(x) -#define M4(x, y) x y +#define M4(x, y) NOOP(x y) #define M5(x) x +#define M6(...) M4(__VA_ARGS__) int foobar(int); int a = M1(foobar); int b = M2(foo, bar(2)); int c = M3(3); int d = M4(foobar, (4)); int e = M5(foobar) (5); +int f = M6(foobar, (6)); )cpp"); CallsVisitor Visitor; @@ -592,18 +596,39 @@ int c = BAR 3.0; } TEST_P(GetFileRangeForEditTest, EditWholeMacroArgShouldSucceed) { - llvm::Annotations Code(R"cpp( -#define FOO(a) a + 7.0; -int a = FOO($r[[10]]); -)cpp"); + { + llvm::Annotations Code(R"cpp( + #define FOO(a) a + 7.0; + int a = FOO($r[[10]]); + )cpp"); + + IntLitVisitor Visitor; + Visitor.OnIntLit = [&Code](IntegerLiteral *Expr, ASTContext *Context) { + auto Range = CharSourceRange::getTokenRange(Expr->getSourceRange()); + EXPECT_THAT( + getFileRangeForEdit(Range, *Context, GetParam()), + ValueIs(AsRange(Context->getSourceManager(), Code.range("r")))); + }; + Visitor.runOver(Code.code()); + } - IntLitVisitor Visitor; - Visitor.OnIntLit = [&Code](IntegerLiteral *Expr, ASTContext *Context) { - auto Range = CharSourceRange::getTokenRange(Expr->getSourceRange()); - EXPECT_THAT(getFileRangeForEdit(Range, *Context, GetParam()), - ValueIs(AsRange(Context->getSourceManager(), Code.range("r")))); - }; - Visitor.runOver(Code.code()); + { + llvm::Annotations Code(R"cpp( + #define FOO(a) a + 7.0; + #define DO_NOTHING(x) x + int Frob(int x); + int a = FOO($r[[Frob(DO_NOTHING(40))]]); + )cpp"); + + CallsVisitor Visitor; + Visitor.OnCall = [&Code](CallExpr *Expr, ASTContext *Context) { + auto Range = CharSourceRange::getTokenRange(Expr->getSourceRange()); + EXPECT_THAT( + getFileRangeForEdit(Range, *Context, GetParam()), + ValueIs(AsRange(Context->getSourceManager(), Code.range("r")))); + }; + Visitor.runOver(Code.code()); + } } TEST_P(GetFileRangeForEditTest, EditPartialMacroArgShouldSucceed) {