diff --git a/clang/lib/Tooling/Transformer/RangeSelector.cpp b/clang/lib/Tooling/Transformer/RangeSelector.cpp --- a/clang/lib/Tooling/Transformer/RangeSelector.cpp +++ b/clang/lib/Tooling/Transformer/RangeSelector.cpp @@ -116,11 +116,24 @@ Expected SelectedRange = Selector(Result); if (!SelectedRange) return SelectedRange.takeError(); - if (SelectedRange->isCharRange()) - return CharSourceRange::getCharRange(SelectedRange->getEnd()); - return CharSourceRange::getCharRange(Lexer::getLocForEndOfToken( - SelectedRange->getEnd(), 0, Result.Context->getSourceManager(), - Result.Context->getLangOpts())); + SourceLocation End = SelectedRange->getEnd(); + if (SelectedRange->isTokenRange()) { + // We need to find the actual (exclusive) end location from which to + // create a new source range. However, that's not guaranteed to be valid, + // even if the token location itself is valid. So, we create a token range + // consisting only of the last token, then map that range back to the + // source file. If that succeeds, we have a valid location for the end of + // the generated range. + CharSourceRange Range = Lexer::makeFileCharRange( + CharSourceRange::getTokenRange(SelectedRange->getEnd()), + *Result.SourceManager, Result.Context->getLangOpts()); + if (Range.isInvalid()) + return invalidArgumentError( + "after: can't resolve sub-range to valid source range"); + End = Range.getEnd(); + } + + return CharSourceRange::getCharRange(End); }; } diff --git a/clang/unittests/Tooling/RangeSelectorTest.cpp b/clang/unittests/Tooling/RangeSelectorTest.cpp --- a/clang/unittests/Tooling/RangeSelectorTest.cpp +++ b/clang/unittests/Tooling/RangeSelectorTest.cpp @@ -193,6 +193,51 @@ HasValue(EqualsCharSourceRange(ExpectedAfter))); } +static void testAfterMacroArg(StringRef Code) { + const std::string Ref = "ref"; + TestMatch Match = + matchCode(Code, declRefExpr(to(namedDecl(hasName("y")))).bind(Ref)); + const auto *E = Match.Result.Nodes.getNodeAs(Ref); + assert(E != nullptr); + // `E` is the variable `y`, and so is one character wide. So advance by one, + // bringing us to the next character. + SourceLocation Next = + Match.Result.SourceManager->getSpellingLoc(E->getBeginLoc()) + .getLocWithOffset(1); + const auto ExpectedAfter = CharSourceRange::getCharRange(Next, Next); + + EXPECT_THAT_EXPECTED(after(node(Ref))(Match.Result), + HasValue(EqualsCharSourceRange(ExpectedAfter))); +} + +// Test with a range that is the entire macro arg, but does not end the +// expansion itself. +TEST(RangeSelectorTest, AfterOpInMacroArg) { + StringRef Code = R"cc( +#define ISNULL(x) x == nullptr + bool g() { int* y; return ISNULL(y); } + )cc"; + testAfterMacroArg(Code); +} + +// Test with a range that is the entire macro arg and ends the expansion itself. +TEST(RangeSelectorTest, AfterOpInMacroArgEndsExpansion) { + StringRef Code = R"cc( +#define ISNULL(x) nullptr == x + bool g() { int* y; return ISNULL(y); } + )cc"; + testAfterMacroArg(Code); +} + +TEST(RangeSelectorTest, AfterOpInPartOfMacroArg) { + StringRef Code = R"cc( +#define ISNULL(x) x == nullptr + int* f(int*); + bool g() { int* y; return ISNULL(f(y)); } + )cc"; + testAfterMacroArg(Code); +} + TEST(RangeSelectorTest, BetweenOp) { StringRef Code = R"cc( int f(int x, int y, int z) { return 3; }