[clang] a78d4b5 - [libTooling] Add flag to getRangeForEdit to ignore macro expansions
Eric Li via cfe-commits
cfe-commits at lists.llvm.org
Thu Dec 8 19:41:05 PST 2022
Author: Eric Li
Date: 2022-12-08T22:40:10-05:00
New Revision: a78d4b5ba716d88a90b905c261f53e74e67a7367
URL: https://github.com/llvm/llvm-project/commit/a78d4b5ba716d88a90b905c261f53e74e67a7367
DIFF: https://github.com/llvm/llvm-project/commit/a78d4b5ba716d88a90b905c261f53e74e67a7367.diff
LOG: [libTooling] Add flag to getRangeForEdit to ignore macro expansions
This commit resolves the FIXME around the behavior of
`Lexer::makeFileCharRange` that `getRangeForEdit` inherits around
source locations in macro expansions.
We add a flag to `getRangeForEdit` that allows a caller to disable the
behavior, and instead uses the spelling location instead, with checks
to ensure that the source locations are not within a macro definition.
Differential Revision: https://reviews.llvm.org/D139676
Added:
Modified:
clang/include/clang/Tooling/Transformer/SourceCode.h
clang/lib/Tooling/Transformer/SourceCode.cpp
clang/unittests/Tooling/SourceCodeTest.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Tooling/Transformer/SourceCode.h b/clang/include/clang/Tooling/Transformer/SourceCode.h
index f3d119c86074..911c1bdb0f5d 100644
--- a/clang/include/clang/Tooling/Transformer/SourceCode.h
+++ b/clang/include/clang/Tooling/Transformer/SourceCode.h
@@ -91,16 +91,25 @@ llvm::Error validateEditRange(const CharSourceRange &Range,
const SourceManager &SM);
/// Attempts to resolve the given range to one that can be edited by a rewrite;
-/// generally, one that starts and ends within a particular file. It supports a
-/// limited set of cases involving source locations in macro expansions. If a
-/// value is returned, it satisfies \c validateEditRange.
+/// generally, one that starts and ends within a particular file. If a value is
+/// returned, it satisfies \c validateEditRange.
+///
+/// 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
+/// rewrite the int literal 3 to 6, and we have the following definition:
+/// #define DO_NOTHING(x) x
+/// then
+/// foo(DO_NOTHING(3))
+/// will be rewritten to
+/// foo(6)
llvm::Optional<CharSourceRange>
getRangeForEdit(const CharSourceRange &EditRange, const SourceManager &SM,
- const LangOptions &LangOpts);
+ const LangOptions &LangOpts, bool IncludeMacroExpansion = true);
inline llvm::Optional<CharSourceRange>
-getRangeForEdit(const CharSourceRange &EditRange, const ASTContext &Context) {
+getRangeForEdit(const CharSourceRange &EditRange, const ASTContext &Context,
+ bool IncludeMacroExpansion = true) {
return getRangeForEdit(EditRange, Context.getSourceManager(),
- Context.getLangOpts());
+ Context.getLangOpts(), IncludeMacroExpansion);
}
} // namespace tooling
} // namespace clang
diff --git a/clang/lib/Tooling/Transformer/SourceCode.cpp b/clang/lib/Tooling/Transformer/SourceCode.cpp
index b89d74a314bf..1aff94ac5077 100644
--- a/clang/lib/Tooling/Transformer/SourceCode.cpp
+++ b/clang/lib/Tooling/Transformer/SourceCode.cpp
@@ -78,27 +78,43 @@ llvm::Error clang::tooling::validateEditRange(const CharSourceRange &Range,
return llvm::Error::success();
}
-llvm::Optional<CharSourceRange>
-clang::tooling::getRangeForEdit(const CharSourceRange &EditRange,
- const SourceManager &SM,
- const LangOptions &LangOpts) {
- // FIXME: makeFileCharRange() has the disadvantage of stripping off "identity"
- // macros. For example, if we're looking to rewrite the int literal 3 to 6,
- // and we have the following definition:
- // #define DO_NOTHING(x) x
- // then
- // foo(DO_NOTHING(3))
- // will be rewritten to
- // foo(6)
- // rather than the arguably better
- // foo(DO_NOTHING(6))
- // Decide whether the current behavior is desirable and modify if not.
- CharSourceRange Range = Lexer::makeFileCharRange(EditRange, SM, LangOpts);
+static bool SpelledInMacroDefinition(SourceLocation Loc,
+ const SourceManager &SM) {
+ 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();
+ } else {
+ return true;
+ }
+ }
+ return false;
+}
+
+llvm::Optional<CharSourceRange> clang::tooling::getRangeForEdit(
+ 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;
+
+ auto B = SM.getSpellingLoc(EditRange.getBegin());
+ auto E = SM.getSpellingLoc(EditRange.getEnd());
+ if (EditRange.isTokenRange())
+ E = Lexer::getLocForEndOfToken(E, 0, SM, LangOpts);
+ Range = CharSourceRange::getCharRange(B, E);
+ }
+
bool IsInvalid = llvm::errorToBool(validateEditRange(Range, SM));
if (IsInvalid)
return std::nullopt;
return Range;
-
}
static bool startsWithNewline(const SourceManager &SM, const Token &Tok) {
diff --git a/clang/unittests/Tooling/SourceCodeTest.cpp b/clang/unittests/Tooling/SourceCodeTest.cpp
index bcb90a892218..90d0654bd5f7 100644
--- a/clang/unittests/Tooling/SourceCodeTest.cpp
+++ b/clang/unittests/Tooling/SourceCodeTest.cpp
@@ -453,7 +453,11 @@ TEST(SourceCodeTest, getAssociatedRangeInvalidForPartialExpansions) {
Visitor.runOver(Code);
}
-TEST(SourceCodeTest, EditRangeWithMacroExpansionsShouldSucceed) {
+class GetRangeForEditTest : public testing::TestWithParam<bool> {};
+INSTANTIATE_TEST_SUITE_P(WithAndWithoutExpansions, GetRangeForEditTest,
+ testing::Bool());
+
+TEST_P(GetRangeForEditTest, EditRangeWithMacroExpansionsShouldSucceed) {
// The call expression, whose range we are extracting, includes two macro
// expansions.
llvm::Annotations Code(R"cpp(
@@ -463,10 +467,9 @@ int a = $r[[foo(M(1), M(2))]];
)cpp");
CallsVisitor Visitor;
-
Visitor.OnCall = [&Code](CallExpr *CE, ASTContext *Context) {
auto Range = CharSourceRange::getTokenRange(CE->getSourceRange());
- EXPECT_THAT(getRangeForEdit(Range, *Context),
+ EXPECT_THAT(getRangeForEdit(Range, *Context, GetParam()),
ValueIs(AsRange(Context->getSourceManager(), Code.range("r"))));
};
Visitor.runOver(Code.code());
@@ -487,7 +490,29 @@ int a = $r[[FOO]];
Visitor.runOver(Code.code());
}
-TEST(SourceCodeTest, EditPartialMacroExpansionShouldFail) {
+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 M1(x) x(1)
+#define M2(x, y) x ## y
+#define M3(x) foobar(x)
+int foobar(int);
+int a = M1(foobar);
+int b = M2(foo, bar(2));
+int c = M3(3);
+)cpp");
+
+ CallsVisitor Visitor;
+ Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+ auto Range = CharSourceRange::getTokenRange(CE->getSourceRange());
+ EXPECT_FALSE(
+ getRangeForEdit(Range, *Context, /*IncludeMacroExpansion=*/false));
+ };
+ Visitor.runOver(Code.code());
+}
+
+TEST_P(GetRangeForEditTest, EditPartialMacroExpansionShouldFail) {
std::string Code = R"cpp(
#define BAR 10+
int c = BAR 3.0;
@@ -496,12 +521,12 @@ int c = BAR 3.0;
IntLitVisitor Visitor;
Visitor.OnIntLit = [](IntegerLiteral *Expr, ASTContext *Context) {
auto Range = CharSourceRange::getTokenRange(Expr->getSourceRange());
- EXPECT_FALSE(getRangeForEdit(Range, *Context));
+ EXPECT_FALSE(getRangeForEdit(Range, *Context, GetParam()));
};
Visitor.runOver(Code);
}
-TEST(SourceCodeTest, EditWholeMacroArgShouldSucceed) {
+TEST_P(GetRangeForEditTest, EditWholeMacroArgShouldSucceed) {
llvm::Annotations Code(R"cpp(
#define FOO(a) a + 7.0;
int a = FOO($r[[10]]);
@@ -510,13 +535,13 @@ int a = FOO($r[[10]]);
IntLitVisitor Visitor;
Visitor.OnIntLit = [&Code](IntegerLiteral *Expr, ASTContext *Context) {
auto Range = CharSourceRange::getTokenRange(Expr->getSourceRange());
- EXPECT_THAT(getRangeForEdit(Range, *Context),
+ EXPECT_THAT(getRangeForEdit(Range, *Context, GetParam()),
ValueIs(AsRange(Context->getSourceManager(), Code.range("r"))));
};
Visitor.runOver(Code.code());
}
-TEST(SourceCodeTest, EditPartialMacroArgShouldSucceed) {
+TEST_P(GetRangeForEditTest, EditPartialMacroArgShouldSucceed) {
llvm::Annotations Code(R"cpp(
#define FOO(a) a + 7.0;
int a = FOO($r[[10]] + 10.0);
@@ -525,7 +550,7 @@ int a = FOO($r[[10]] + 10.0);
IntLitVisitor Visitor;
Visitor.OnIntLit = [&Code](IntegerLiteral *Expr, ASTContext *Context) {
auto Range = CharSourceRange::getTokenRange(Expr->getSourceRange());
- EXPECT_THAT(getRangeForEdit(Range, *Context),
+ EXPECT_THAT(getRangeForEdit(Range, *Context, GetParam()),
ValueIs(AsRange(Context->getSourceManager(), Code.range("r"))));
};
Visitor.runOver(Code.code());
@@ -541,7 +566,6 @@ int a = foo(M(1), M(2));
)cpp";
CallsVisitor Visitor;
-
Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
auto Range = CharSourceRange::getTokenRange(CE->getSourceRange());
EXPECT_THAT_ERROR(validateEditRange(Range, Context->getSourceManager()),
More information about the cfe-commits
mailing list