diff --git a/clang/include/clang/Tooling/Transformer/SourceCode.h b/clang/include/clang/Tooling/Transformer/SourceCode.h --- a/clang/include/clang/Tooling/Transformer/SourceCode.h +++ b/clang/include/clang/Tooling/Transformer/SourceCode.h @@ -104,14 +104,36 @@ /// will be rewritten to /// foo(6) std::optional -getRangeForEdit(const CharSourceRange &EditRange, const SourceManager &SM, - const LangOptions &LangOpts, bool IncludeMacroExpansion = true); +getFileRangeForEdit(const CharSourceRange &EditRange, const SourceManager &SM, + const LangOptions &LangOpts, + bool IncludeMacroExpansion = true); inline std::optional -getRangeForEdit(const CharSourceRange &EditRange, const ASTContext &Context, - bool IncludeMacroExpansion = true) { - return getRangeForEdit(EditRange, Context.getSourceManager(), - Context.getLangOpts(), IncludeMacroExpansion); +getFileRangeForEdit(const CharSourceRange &EditRange, const ASTContext &Context, + bool IncludeMacroExpansion = true) { + return getFileRangeForEdit(EditRange, Context.getSourceManager(), + Context.getLangOpts(), IncludeMacroExpansion); } + +/// Attempts to resolve the given range to one that starts and ends in a +/// particular file. +/// +/// If \c IncludeMacroExpansion is true, a limited set of cases involving source +/// locations in macro expansions is supported. For example, if we're looking to +/// get the range of the int literal 3, and we have the following definition: +/// #define DO_NOTHING(x) x +/// foo(DO_NOTHING(3)) +/// the returned range will hold the source text `DO_NOTHING(3)`. +std::optional getFileRange(const CharSourceRange &EditRange, + const SourceManager &SM, + const LangOptions &LangOpts, + bool IncludeMacroExpansion); +inline std::optional +getFileRange(const CharSourceRange &EditRange, const ASTContext &Context, + bool IncludeMacroExpansion) { + return getFileRange(EditRange, Context.getSourceManager(), + Context.getLangOpts(), IncludeMacroExpansion); +} + } // namespace tooling } // namespace clang #endif // LLVM_CLANG_TOOLING_TRANSFORMER_SOURCECODE_H diff --git a/clang/lib/Tooling/Transformer/RewriteRule.cpp b/clang/lib/Tooling/Transformer/RewriteRule.cpp --- a/clang/lib/Tooling/Transformer/RewriteRule.cpp +++ b/clang/lib/Tooling/Transformer/RewriteRule.cpp @@ -39,7 +39,7 @@ if (!Range) return Range.takeError(); std::optional EditRange = - tooling::getRangeForEdit(*Range, *Result.Context); + tooling::getFileRangeForEdit(*Range, *Result.Context); // FIXME: let user specify whether to treat this case as an error or ignore // it as is currently done. This behavior is problematic in that it hides // failures from bad ranges. Also, the behavior here differs from @@ -449,7 +449,7 @@ auto &NodesMap = Result.Nodes.getMap(); auto Root = NodesMap.find(RootID); assert(Root != NodesMap.end() && "Transformation failed: missing root node."); - std::optional RootRange = tooling::getRangeForEdit( + std::optional RootRange = tooling::getFileRangeForEdit( CharSourceRange::getTokenRange(Root->second.getSourceRange()), *Result.Context); if (RootRange) diff --git a/clang/lib/Tooling/Transformer/SourceCode.cpp b/clang/lib/Tooling/Transformer/SourceCode.cpp --- a/clang/lib/Tooling/Transformer/SourceCode.cpp +++ b/clang/lib/Tooling/Transformer/SourceCode.cpp @@ -50,8 +50,9 @@ return CharSourceRange::getTokenRange(Range.getBegin(), Tok.getLocation()); } -llvm::Error clang::tooling::validateEditRange(const CharSourceRange &Range, - const SourceManager &SM) { +static llvm::Error validateRange(const CharSourceRange &Range, + const SourceManager &SM, + bool AllowSystemHeaders) { if (Range.isInvalid()) return llvm::make_error(errc::invalid_argument, "Invalid range"); @@ -60,10 +61,12 @@ return llvm::make_error( errc::invalid_argument, "Range starts or ends in a macro expansion"); - if (SM.isInSystemHeader(Range.getBegin()) || - SM.isInSystemHeader(Range.getEnd())) - return llvm::make_error(errc::invalid_argument, - "Range is in system header"); + if (!AllowSystemHeaders) { + if (SM.isInSystemHeader(Range.getBegin()) || + SM.isInSystemHeader(Range.getEnd())) + return llvm::make_error(errc::invalid_argument, + "Range is in system header"); + } std::pair BeginInfo = SM.getDecomposedLoc(Range.getBegin()); std::pair EndInfo = SM.getDecomposedLoc(Range.getEnd()); @@ -72,13 +75,18 @@ errc::invalid_argument, "Range begins and ends in different files"); if (BeginInfo.second > EndInfo.second) - return llvm::make_error( - errc::invalid_argument, "Range's begin is past its end"); + return llvm::make_error(errc::invalid_argument, + "Range's begin is past its end"); return llvm::Error::success(); } -static bool SpelledInMacroDefinition(SourceLocation Loc, +llvm::Error clang::tooling::validateEditRange(const CharSourceRange &Range, + const SourceManager &SM) { + return validateRange(Range, SM, /*AllowSystemHeaders=*/false); +} + +static bool spelledInMacroDefinition(SourceLocation Loc, const SourceManager &SM) { while (Loc.isMacroID()) { const auto &Expansion = SM.getSLocEntry(SM.getFileID(Loc)).getExpansion(); @@ -93,16 +101,17 @@ return false; } -std::optional clang::tooling::getRangeForEdit( - const CharSourceRange &EditRange, const SourceManager &SM, - const LangOptions &LangOpts, bool IncludeMacroExpansion) { +static CharSourceRange getRange(const CharSourceRange &EditRange, + const SourceManager &SM, + const LangOptions &LangOpts, + bool IncludeMacroExpansion) { CharSourceRange Range; if (IncludeMacroExpansion) { Range = Lexer::makeFileCharRange(EditRange, SM, LangOpts); } else { - if (SpelledInMacroDefinition(EditRange.getBegin(), SM) || - SpelledInMacroDefinition(EditRange.getEnd(), SM)) - return std::nullopt; + if (spelledInMacroDefinition(EditRange.getBegin(), SM) || + spelledInMacroDefinition(EditRange.getEnd(), SM)) + return {}; auto B = SM.getSpellingLoc(EditRange.getBegin()); auto E = SM.getSpellingLoc(EditRange.getEnd()); @@ -110,13 +119,32 @@ E = Lexer::getLocForEndOfToken(E, 0, SM, LangOpts); Range = CharSourceRange::getCharRange(B, E); } + return Range; +} +std::optional clang::tooling::getFileRangeForEdit( + const CharSourceRange &EditRange, const SourceManager &SM, + const LangOptions &LangOpts, bool IncludeMacroExpansion) { + CharSourceRange Range = + getRange(EditRange, SM, LangOpts, IncludeMacroExpansion); bool IsInvalid = llvm::errorToBool(validateEditRange(Range, SM)); if (IsInvalid) return std::nullopt; return Range; } +std::optional clang::tooling::getFileRange( + const CharSourceRange &EditRange, const SourceManager &SM, + const LangOptions &LangOpts, bool IncludeMacroExpansion) { + CharSourceRange Range = + getRange(EditRange, SM, LangOpts, IncludeMacroExpansion); + bool IsInvalid = + llvm::errorToBool(validateRange(Range, SM, /*AllowSystemHeaders=*/true)); + if (IsInvalid) + return std::nullopt; + return Range; +} + static bool startsWithNewline(const SourceManager &SM, const Token &Tok) { return isVerticalWhitespace(SM.getCharacterData(Tok.getLocation())[0]); } diff --git a/clang/unittests/Tooling/SourceCodeTest.cpp b/clang/unittests/Tooling/SourceCodeTest.cpp --- a/clang/unittests/Tooling/SourceCodeTest.cpp +++ b/clang/unittests/Tooling/SourceCodeTest.cpp @@ -25,7 +25,7 @@ using tooling::getAssociatedRange; using tooling::getExtendedRange; using tooling::getExtendedText; -using tooling::getRangeForEdit; +using tooling::getFileRangeForEdit; using tooling::getText; using tooling::maybeExtendRange; using tooling::validateEditRange; @@ -469,7 +469,7 @@ CallsVisitor Visitor; Visitor.OnCall = [&Code](CallExpr *CE, ASTContext *Context) { auto Range = CharSourceRange::getTokenRange(CE->getSourceRange()); - EXPECT_THAT(getRangeForEdit(Range, *Context, GetParam()), + EXPECT_THAT(getFileRangeForEdit(Range, *Context, GetParam()), ValueIs(AsRange(Context->getSourceManager(), Code.range("r")))); }; Visitor.runOver(Code.code()); @@ -484,7 +484,7 @@ IntLitVisitor Visitor; Visitor.OnIntLit = [&Code](IntegerLiteral *Expr, ASTContext *Context) { auto Range = CharSourceRange::getTokenRange(Expr->getSourceRange()); - EXPECT_THAT(getRangeForEdit(Range, *Context), + EXPECT_THAT(getFileRangeForEdit(Range, *Context), ValueIs(AsRange(Context->getSourceManager(), Code.range("r")))); }; Visitor.runOver(Code.code()); @@ -507,7 +507,7 @@ Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) { auto Range = CharSourceRange::getTokenRange(CE->getSourceRange()); EXPECT_FALSE( - getRangeForEdit(Range, *Context, /*IncludeMacroExpansion=*/false)); + getFileRangeForEdit(Range, *Context, /*IncludeMacroExpansion=*/false)); }; Visitor.runOver(Code.code()); } @@ -521,7 +521,7 @@ IntLitVisitor Visitor; Visitor.OnIntLit = [](IntegerLiteral *Expr, ASTContext *Context) { auto Range = CharSourceRange::getTokenRange(Expr->getSourceRange()); - EXPECT_FALSE(getRangeForEdit(Range, *Context, GetParam())); + EXPECT_FALSE(getFileRangeForEdit(Range, *Context, GetParam())); }; Visitor.runOver(Code); } @@ -535,7 +535,7 @@ IntLitVisitor Visitor; Visitor.OnIntLit = [&Code](IntegerLiteral *Expr, ASTContext *Context) { auto Range = CharSourceRange::getTokenRange(Expr->getSourceRange()); - EXPECT_THAT(getRangeForEdit(Range, *Context, GetParam()), + EXPECT_THAT(getFileRangeForEdit(Range, *Context, GetParam()), ValueIs(AsRange(Context->getSourceManager(), Code.range("r")))); }; Visitor.runOver(Code.code()); @@ -550,7 +550,7 @@ IntLitVisitor Visitor; Visitor.OnIntLit = [&Code](IntegerLiteral *Expr, ASTContext *Context) { auto Range = CharSourceRange::getTokenRange(Expr->getSourceRange()); - EXPECT_THAT(getRangeForEdit(Range, *Context, GetParam()), + EXPECT_THAT(getFileRangeForEdit(Range, *Context, GetParam()), ValueIs(AsRange(Context->getSourceManager(), Code.range("r")))); }; Visitor.runOver(Code.code());