[clang] f10d271 - [clang][dataflow] Handle null pointers of type std::nullptr_t
Author: Eric Li
Date: 2022-07-05T13:49:26Z
New Revision: f10d271ae27f35cd56535ff7e832a358732c8fcd
URL:
https://github.com/llvm/llvm-project/commit/f10d271ae27f35cd56535ff7e832a358732c8fcd
DIFF:
https://github.com/llvm/llvm-project/commit/f10d271ae27f35cd56535ff7e832a358732c8fcd.diff
LOG: [clang][dataflow] Handle null pointers of type std::nullptr_t
Treat `std::nullptr_t` as a regular scalar type to avoid tripping
assertions when analyzing code that uses `std::nullptr_t`.
Differential Revision: https://reviews.llvm.org/D129097
Added:
Modified:
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
Removed:
diff --git
a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
index c1100d8474aa4..d87b9cc37b996 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
@@ -155,6 +155,7 @@ class DataflowAnalysisContext {
/// Returns a pointer value that represents a null pointer. Calls with
/// `PointeeType` that are canonically equivalent will return the same
result.
+ /// A null `PointeeType` can be used for the pointee of `std::nullptr_t`.
PointerValue &getOrCreateNullPointerValue(QualType PointeeType);
/// Returns a symbolic boolean value that models a boolean literal equal to
@@ -251,6 +252,17 @@ class DataflowAnalysisContext {
bool equivalentBoolValues(BoolValue &Val1, BoolValue &Val2);
private:
+ struct NullableQualTypeDenseMapInfo : private llvm::DenseMapInfo {
+static QualType getEmptyKey() {
+ // Allow a NULL `QualType` by using a
diff erent value as the empty key.
+ return QualType::getFromOpaquePtr(reinterpret_cast(1));
+}
+
+using DenseMapInfo::getHashValue;
+using DenseMapInfo::getTombstoneKey;
+using DenseMapInfo::isEqual;
+ };
+
/// Adds all constraints of the flow condition identified by `Token` and all
/// of its transitive dependencies to `Constraints`. `VisitedTokens` is used
/// to track tokens of flow conditions that were already visited by recursive
@@ -311,7 +323,8 @@ class DataflowAnalysisContext {
// required to initialize the `PointeeLoc` field in `PointerValue`. Consider
// creating a type-independent `NullPointerValue` without a `PointeeLoc`
// field.
- llvm::DenseMap NullPointerVals;
+ llvm::DenseMap
+ NullPointerVals;
AtomicBoolValue &TrueVal;
AtomicBoolValue &FalseVal;
diff --git a/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
b/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
index e08fc71c51dc7..cd87e87a6acab 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
@@ -24,8 +24,8 @@ namespace dataflow {
StorageLocation &
DataflowAnalysisContext::getStableStorageLocation(QualType Type) {
- assert(!Type.isNull());
- if (Type->isStructureOrClassType() || Type->isUnionType()) {
+ if (!Type.isNull() &&
+ (Type->isStructureOrClassType() || Type->isUnionType())) {
// FIXME: Explore options to avoid eager initialization of fields as some
of
// them might not be needed for a particular analysis.
llvm::DenseMap FieldLocs;
@@ -57,8 +57,8 @@ DataflowAnalysisContext::getStableStorageLocation(const Expr
&E) {
PointerValue &
DataflowAnalysisContext::getOrCreateNullPointerValue(QualType PointeeType) {
- assert(!PointeeType.isNull());
- auto CanonicalPointeeType = PointeeType.getCanonicalType();
+ auto CanonicalPointeeType =
+ PointeeType.isNull() ? PointeeType : PointeeType.getCanonicalType();
auto Res = NullPointerVals.try_emplace(CanonicalPointeeType, nullptr);
if (Res.second) {
auto &PointeeLoc = getStableStorageLocation(CanonicalPointeeType);
diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index 04710b1795ef4..c4a42061edd90 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -2213,12 +2213,14 @@ TEST(TransferTest, IntegralToBooleanCastFromBool) {
TEST(TransferTest, NullToPointerCast) {
std::string Code = R"(
+using my_nullptr_t = decltype(nullptr);
struct Baz {};
void target() {
int *FooX = nullptr;
int *FooY = nullptr;
bool **Bar = nullptr;
Baz *Baz = nullptr;
+ my_nullptr_t Null = 0;
// [[p]]
}
)";
@@ -2242,6 +2244,9 @@ TEST(TransferTest, NullToPointerCast) {
const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
ASSERT_THAT(BazDecl, NotNull());
[clang] a29fd4d - [libTooling] Fix indentation. NFC.
Author: Eric Li
Date: 2022-03-28T18:34:45Z
New Revision: a29fd4d4dad3f4c62dbcb31bc097535620455208
URL:
https://github.com/llvm/llvm-project/commit/a29fd4d4dad3f4c62dbcb31bc097535620455208
DIFF:
https://github.com/llvm/llvm-project/commit/a29fd4d4dad3f4c62dbcb31bc097535620455208.diff
LOG: [libTooling] Fix indentation. NFC.
Added:
Modified:
clang/unittests/Tooling/TransformerTest.cpp
Removed:
diff --git a/clang/unittests/Tooling/TransformerTest.cpp
b/clang/unittests/Tooling/TransformerTest.cpp
index 75ba9b919bbf9..9bc372b97d807 100644
--- a/clang/unittests/Tooling/TransformerTest.cpp
+++ b/clang/unittests/Tooling/TransformerTest.cpp
@@ -82,7 +82,7 @@ static std::string format(StringRef Code) {
}
static void compareSnippets(StringRef Expected,
- const llvm::Optional &MaybeActual) {
+const llvm::Optional &MaybeActual) {
ASSERT_TRUE(MaybeActual) << "Rewrite failed. Expecting: " << Expected;
auto Actual = *MaybeActual;
std::string HL = "#include \"header.h\"\n";
@@ -1265,31 +1265,29 @@ void testIt()
auto RewriteOutput =
CodePrefix + RangeLoop + LoopBody + RangeLoop + LoopBody + CodeSuffix;
-auto MatchedLoop = forStmt(
-has(declStmt(
-hasSingleDecl(varDecl(hasInitializer(integerLiteral(equals(0
- .bind("loopVar",
-has(binaryOperator(hasOperatorName("!="),
- hasLHS(ignoringImplicit(declRefExpr(
- to(varDecl(equalsBoundNode("loopVar")),
- hasRHS(expr().bind("upperBoundExpr",
-has(unaryOperator(hasOperatorName("++"),
- hasUnaryOperand(declRefExpr(
- to(varDecl(equalsBoundNode("loopVar"))
-.bind("incrementOp")));
+ auto MatchedLoop = forStmt(
+ has(declStmt(hasSingleDecl(
+
varDecl(hasInitializer(integerLiteral(equals(0.bind("loopVar",
+ has(binaryOperator(hasOperatorName("!="),
+ hasLHS(ignoringImplicit(declRefExpr(
+ to(varDecl(equalsBoundNode("loopVar")),
+ hasRHS(expr().bind("upperBoundExpr",
+ has(unaryOperator(hasOperatorName("++"),
+hasUnaryOperand(declRefExpr(
+to(varDecl(equalsBoundNode("loopVar"))
+ .bind("incrementOp")));
-auto RewriteRule =
-changeTo(transformer::enclose(node("loopVar"), node("incrementOp")),
- cat("auto ", name("loopVar"), " : boost::irange(",
- node("upperBoundExpr"), ")"));
-
-testRule(makeRule(traverse(TK_IgnoreUnlessSpelledInSource, MatchedLoop),
- RewriteRule),
- RewriteInput, RewriteOutput);
+ auto RewriteRule =
+ changeTo(transformer::enclose(node("loopVar"), node("incrementOp")),
+ cat("auto ", name("loopVar"), " : boost::irange(",
+ node("upperBoundExpr"), ")"));
-testRuleFailure(makeRule(traverse(TK_AsIs, MatchedLoop), RewriteRule),
-RewriteInput);
+ testRule(makeRule(traverse(TK_IgnoreUnlessSpelledInSource, MatchedLoop),
+RewriteRule),
+ RewriteInput, RewriteOutput);
+ testRuleFailure(makeRule(traverse(TK_AsIs, MatchedLoop), RewriteRule),
+ RewriteInput);
}
TEST_F(TransformerTest, ImplicitNodes_ForStmt2) {
@@ -1338,31 +1336,29 @@ void testIt()
auto RewriteOutput =
CodePrefix + RangeLoop + LoopBody + RangeLoop + LoopBody + CodeSuffix;
-auto MatchedLoop = forStmt(
-hasLoopInit(declStmt(
-hasSingleDecl(varDecl(hasInitializer(integerLiteral(equals(0
- .bind("loopVar",
-hasCondition(binaryOperator(hasOperatorName("!="),
-hasLHS(ignoringImplicit(declRefExpr(to(
-
varDecl(equalsBoundNode("loopVar")),
-hasRHS(expr().bind("upperBoundExpr",
-hasIncrement(unaryOperator(hasOperatorName("++"),
- hasUnaryOperand(declRefExpr(to(
- varDecl(equalsBoundNode("loopVar"))
- .bind("incrementOp")));
-
-auto RewriteRule =
-changeTo(transformer::enclose(node("loopVar"), node("incrementOp")),
- cat("auto ", name("loopVar"), " : boost::irange(",
- node("upperBoundExpr"), ")"));
-
-testRule(makeRule(traverse(TK_IgnoreUnlessSpelledInSource, MatchedLoop),
- RewriteRule),
- RewriteInput, RewriteOutput);
-
-testRuleFailure(makeRule(traverse(TK_AsIs, MatchedLoop), RewriteRule),
-RewriteInput);
+ a
[clang] 5e28923 - [clang][dataflow][NFC] Remove last use of deprecated ctor
Author: Eric Li
Date: 2022-07-27T14:23:35-04:00
New Revision: 5e28923e332f2e738d17d35f1978df3391ee10af
URL:
https://github.com/llvm/llvm-project/commit/5e28923e332f2e738d17d35f1978df3391ee10af
DIFF:
https://github.com/llvm/llvm-project/commit/5e28923e332f2e738d17d35f1978df3391ee10af.diff
LOG: [clang][dataflow][NFC] Remove last use of deprecated ctor
Use a delegating constructor to remove the last use of the deprecated
ctor of `TypeErasedDataflowAnalysis`, and then delete it.
Differential Revision: https://reviews.llvm.org/D130653
Added:
Modified:
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
Removed:
diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
index ef8f7a51496c9..a8785c554eb2f 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
@@ -66,7 +66,8 @@ class DataflowAnalysis : public TypeErasedDataflowAnalysis {
/// Deprecated. Use the `DataflowAnalysisOptions` constructor instead.
explicit DataflowAnalysis(ASTContext &Context, bool ApplyBuiltinTransfer)
- : TypeErasedDataflowAnalysis(ApplyBuiltinTransfer), Context(Context) {}
+ : DataflowAnalysis(Context, DataflowAnalysisOptions{ApplyBuiltinTransfer,
+ TransferOptions{}})
{}
explicit DataflowAnalysis(ASTContext &Context,
DataflowAnalysisOptions Options)
diff --git
a/clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
b/clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
index 92700f164e7bd..3a108402ab159 100644
--- a/clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
+++ b/clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
@@ -59,10 +59,6 @@ class TypeErasedDataflowAnalysis : public
Environment::ValueModel {
public:
TypeErasedDataflowAnalysis() : Options({}) {}
- /// Deprecated. Use the `DataflowAnalysisOptions` constructor instead.
- TypeErasedDataflowAnalysis(bool ApplyBuiltinTransfer)
- : Options({ApplyBuiltinTransfer, TransferOptions{}}) {}
-
TypeErasedDataflowAnalysis(DataflowAnalysisOptions Options)
: Options(Options) {}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 54d24ea - [clang][dataflow][NFC] Fix outdated comment on getStableStorageLocation
Author: Eric Li
Date: 2022-08-04T11:15:14-04:00
New Revision: 54d24eae98726e37867bc3a683cd58af6ec128df
URL:
https://github.com/llvm/llvm-project/commit/54d24eae98726e37867bc3a683cd58af6ec128df
DIFF:
https://github.com/llvm/llvm-project/commit/54d24eae98726e37867bc3a683cd58af6ec128df.diff
LOG: [clang][dataflow][NFC] Fix outdated comment on getStableStorageLocation
Follow-up to D129097.
It is no longer a requirement that the `QualType` passed to to
`DataflowAnalysisContext::getStableStorageLocation()` is not null. A
null type pass as an argument is only applicable as the pointee type
of a `std::nullptr_t` pointer.
Differential Revision: https://reviews.llvm.org/D131109
Added:
Modified:
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
Removed:
diff --git
a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
index a31e08fd16c48..ab60d313f242c 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
@@ -93,9 +93,7 @@ class DataflowAnalysisContext {
/// Returns a new storage location appropriate for `Type`.
///
- /// Requirements:
- ///
- /// `Type` must not be null.
+ /// A null `Type` is interpreted as the pointee type of `std::nullptr_t`.
StorageLocation &createStorageLocation(QualType Type);
/// Returns a stable storage location for `D`.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 5659908 - [clang][dataflow][NFC] Resize vector directly with ctor
Author: Eric Li Date: 2022-08-04T13:12:37-04:00 New Revision: 5659908f4c6d32d3f7d1ebbe014fbd9419cdfb9c URL: https://github.com/llvm/llvm-project/commit/5659908f4c6d32d3f7d1ebbe014fbd9419cdfb9c DIFF: https://github.com/llvm/llvm-project/commit/5659908f4c6d32d3f7d1ebbe014fbd9419cdfb9c.diff LOG: [clang][dataflow][NFC] Resize vector directly with ctor Differential Revision: https://reviews.llvm.org/D131177 Added: Modified: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp Removed: diff --git a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp index 32fb48470b14..fe650200e4e3 100644 --- a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp +++ b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp @@ -339,8 +339,8 @@ runTypeErasedDataflowAnalysis( PostOrderCFGView POV(&CFCtx.getCFG()); ForwardDataflowWorklist Worklist(CFCtx.getCFG(), &POV); - std::vector> BlockStates; - BlockStates.resize(CFCtx.getCFG().size(), llvm::None); + std::vector> BlockStates( + CFCtx.getCFG().size(), llvm::None); // The entry basic block doesn't contain statements so it can be skipped. const CFGBlock &Entry = CFCtx.getCFG().getEntry(); ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 18034ae - [clang][dataflow][NFC] Convert mutable vector references to ArrayRef
Author: Eric Li
Date: 2022-08-04T13:12:37-04:00
New Revision: 18034aee63eeac673496a88d9e90c8dd73d15927
URL:
https://github.com/llvm/llvm-project/commit/18034aee63eeac673496a88d9e90c8dd73d15927
DIFF:
https://github.com/llvm/llvm-project/commit/18034aee63eeac673496a88d9e90c8dd73d15927.diff
LOG: [clang][dataflow][NFC] Convert mutable vector references to ArrayRef
`transferBlock` and `computeBlockInputState` only read the
`BlockStates` vector for the predecessor block(s), and do not need to
mutate any of the contents. Only `runTypeErasedDataflowAnalysis`
writes into the `vector`, so simply down to an `ArrayRef`.
Added:
Modified:
clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
Removed:
diff --git
a/clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
b/clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
index c563a2cd076b0..3bd1c8e12e9b2 100644
--- a/clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
+++ b/clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
@@ -121,7 +121,7 @@ struct TypeErasedDataflowAnalysisState {
/// `llvm::None` represent basic blocks that are not evaluated yet.
TypeErasedDataflowAnalysisState transferBlock(
const ControlFlowContext &CFCtx,
-std::vector> &BlockStates,
+llvm::ArrayRef>
BlockStates,
const CFGBlock &Block, const Environment &InitEnv,
TypeErasedDataflowAnalysis &Analysis,
std::function {
/// `llvm::None` represent basic blocks that are not evaluated yet.
static TypeErasedDataflowAnalysisState computeBlockInputState(
const ControlFlowContext &CFCtx,
-std::vector> &BlockStates,
+llvm::ArrayRef>
BlockStates,
const CFGBlock &Block, const Environment &InitEnv,
TypeErasedDataflowAnalysis &Analysis) {
llvm::DenseSet Preds;
@@ -303,7 +303,7 @@ static void transferCFGInitializer(const CFGInitializer
&CfgInit,
TypeErasedDataflowAnalysisState transferBlock(
const ControlFlowContext &CFCtx,
-std::vector> &BlockStates,
+llvm::ArrayRef>
BlockStates,
const CFGBlock &Block, const Environment &InitEnv,
TypeErasedDataflowAnalysis &Analysis,
std::functionhttps://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 62b2a47 - [clang][dataflow] Only skip ExprWithCleanups when visiting terminators
Author: Eric Li
Date: 2022-05-04T15:31:49Z
New Revision: 62b2a47a9f15ed2f1dc4b39c924341c7b9bd7cf8
URL:
https://github.com/llvm/llvm-project/commit/62b2a47a9f15ed2f1dc4b39c924341c7b9bd7cf8
DIFF:
https://github.com/llvm/llvm-project/commit/62b2a47a9f15ed2f1dc4b39c924341c7b9bd7cf8.diff
LOG: [clang][dataflow] Only skip ExprWithCleanups when visiting terminators
`IgnoreParenImpCasts` will remove implicit casts to bool
(e.g. `PointerToBoolean`), such that the resulting expression may not
be of the `bool` type. The `cast_or_null` in
`extendFlowCondition` will then trigger an assert, as the pointer
expression will not have a `BoolValue`.
Instead, we only skip `ExprWithCleanups` and `ParenExpr` nodes, as the
CFG does not emit them.
Differential Revision: https://reviews.llvm.org/D124807
Added:
Modified:
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
clang/include/clang/Analysis/FlowSensitive/Transfer.h
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
clang/lib/Analysis/FlowSensitive/Transfer.cpp
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
Removed:
diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
index 0a2c75f804c2a..019d07c9b7f72 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -172,6 +172,10 @@ class Environment {
/// Creates a storage location for `E`. Does not assign the returned storage
/// location to `E` in the environment. Does not assign a value to the
/// returned storage location in the environment.
+ ///
+ /// Requirements:
+ ///
+ /// `E` must not be a `ExprWithCleanups`.
StorageLocation &createStorageLocation(const Expr &E);
/// Assigns `Loc` as the storage location of `D` in the environment.
@@ -191,11 +195,16 @@ class Environment {
/// Requirements:
///
/// `E` must not be assigned a storage location in the environment.
+ /// `E` must not be a `ExprWithCleanups`.
void setStorageLocation(const Expr &E, StorageLocation &Loc);
/// Returns the storage location assigned to `E` in the environment, applying
/// the `SP` policy for skipping past indirections, or null if `E` isn't
/// assigned a storage location in the environment.
+ ///
+ /// Requirements:
+ ///
+ /// `E` must not be a `ExprWithCleanups`.
StorageLocation *getStorageLocation(const Expr &E, SkipPast SP) const;
/// Returns the storage location assigned to the `this` pointee in the
@@ -226,6 +235,12 @@ class Environment {
/// Equivalent to `getValue(getStorageLocation(E, SP), SkipPast::None)` if
`E`
/// is assigned a storage location in the environment, otherwise returns
null.
+ ///
+ /// Requirements:
+ ///
+ /// `E` must not be a `ExprWithCleanups`.
+ ///
+ /// FIXME: `Environment` should ignore any `ExprWithCleanups` it sees.
Value *getValue(const Expr &E, SkipPast SP) const;
/// Transfers ownership of `Loc` to the analysis context and returns a
diff --git a/clang/include/clang/Analysis/FlowSensitive/Transfer.h
b/clang/include/clang/Analysis/FlowSensitive/Transfer.h
index a6b663b997fd6..c5b1086c451fd 100644
--- a/clang/include/clang/Analysis/FlowSensitive/Transfer.h
+++ b/clang/include/clang/Analysis/FlowSensitive/Transfer.h
@@ -35,9 +35,19 @@ class StmtToEnvMap {
///
/// Requirements:
///
-/// The type of `S` must not be `ParenExpr`.
+/// `S` must not be `ParenExpr` or `ExprWithCleanups`.
void transfer(const StmtToEnvMap &StmtToEnv, const Stmt &S, Environment &Env);
+/// Skip past a `ExprWithCleanups` which might surround `E`. Returns null if
`E`
+/// is null.
+///
+/// The CFG omits `ExprWithCleanups` nodes (as it does with `ParenExpr`), and
so
+/// the transfer function doesn't accept them as valid input. Manual traversal
+/// of the AST should skip and unwrap any `ExprWithCleanups` it might expect to
+/// see. They are safe to skip, as the CFG will emit calls to destructors as
+/// appropriate.
+const Expr *ignoreExprWithCleanups(const Expr *E);
+
} // namespace dataflow
} // namespace clang
diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 469696643d319..ad917f18b2570 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -15,6 +15,7 @@
#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
#include "clang/AST/Decl.h"
#include "clang/AST/DeclCXX.h"
+#include "clang/AST/ExprCXX.h"
#include "clang/AST/Type.h"
#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
#include "clang/Analysis/FlowSensitive/StorageLocat
[clang] 58abe36 - [clang][dataflow] Add flowConditionIsTautology function
Author: Eric Li
Date: 2022-05-05T03:57:43Z
New Revision: 58abe36ae7654987f5af793e3e261ac0b43c870b
URL:
https://github.com/llvm/llvm-project/commit/58abe36ae7654987f5af793e3e261ac0b43c870b
DIFF:
https://github.com/llvm/llvm-project/commit/58abe36ae7654987f5af793e3e261ac0b43c870b.diff
LOG: [clang][dataflow] Add flowConditionIsTautology function
Provide a way for users to check if a flow condition is
unconditionally true.
Differential Revision: https://reviews.llvm.org/D124943
Added:
Modified:
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
clang/unittests/Analysis/FlowSensitive/DataflowAnalysisContextTest.cpp
Removed:
diff --git
a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
index 5cf681e4e489c..a762cb9de48bd 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
@@ -173,6 +173,10 @@ class DataflowAnalysisContext {
/// identified by `Token` imply that `Val` is true.
bool flowConditionImplies(AtomicBoolValue &Token, BoolValue &Val);
+ /// Returns true if and only if the constraints of the flow condition
+ /// identified by `Token` are always true.
+ bool flowConditionIsTautology(AtomicBoolValue &Token);
+
private:
/// Adds all constraints of the flow condition identified by `Token` and all
/// of its transitive dependencies to `Constraints`. `VisitedTokens` is used
diff --git a/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
b/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
index 4274fca4e3c14..b29983eed79f9 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
@@ -117,6 +117,19 @@ bool
DataflowAnalysisContext::flowConditionImplies(AtomicBoolValue &Token,
return S->solve(std::move(Constraints)) == Solver::Result::Unsatisfiable;
}
+bool DataflowAnalysisContext::flowConditionIsTautology(AtomicBoolValue &Token)
{
+ // Returns true if and only if we cannot prove that the flow condition can
+ // ever be false.
+ llvm::DenseSet Constraints = {
+ &getBoolLiteralValue(true),
+ &getOrCreateNegationValue(getBoolLiteralValue(false)),
+ &getOrCreateNegationValue(Token),
+ };
+ llvm::DenseSet VisitedTokens;
+ addTransitiveFlowConditionConstraints(Token, Constraints, VisitedTokens);
+ return S->solve(std::move(Constraints)) == Solver::Result::Unsatisfiable;
+}
+
void DataflowAnalysisContext::addTransitiveFlowConditionConstraints(
AtomicBoolValue &Token, llvm::DenseSet &Constraints,
llvm::DenseSet &VisitedTokens) const {
diff --git
a/clang/unittests/Analysis/FlowSensitive/DataflowAnalysisContextTest.cpp
b/clang/unittests/Analysis/FlowSensitive/DataflowAnalysisContextTest.cpp
index b4b9b169f440e..b8a9ef52504af 100644
--- a/clang/unittests/Analysis/FlowSensitive/DataflowAnalysisContextTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/DataflowAnalysisContextTest.cpp
@@ -140,4 +140,33 @@ TEST_F(DataflowAnalysisContextTest, JoinFlowConditions) {
EXPECT_TRUE(Context.flowConditionImplies(FC3, C3));
}
+TEST_F(DataflowAnalysisContextTest, FlowConditionTautologies) {
+ // Fresh flow condition with empty/no constraints is always true.
+ auto &FC1 = Context.makeFlowConditionToken();
+ EXPECT_TRUE(Context.flowConditionIsTautology(FC1));
+
+ // Literal `true` is always true.
+ auto &FC2 = Context.makeFlowConditionToken();
+ Context.addFlowConditionConstraint(FC2, Context.getBoolLiteralValue(true));
+ EXPECT_TRUE(Context.flowConditionIsTautology(FC2));
+
+ // Literal `false` is never true.
+ auto &FC3 = Context.makeFlowConditionToken();
+ Context.addFlowConditionConstraint(FC3, Context.getBoolLiteralValue(false));
+ EXPECT_FALSE(Context.flowConditionIsTautology(FC3));
+
+ // We can't prove that an arbitrary bool A is always true...
+ auto &C1 = Context.createAtomicBoolValue();
+ auto &FC4 = Context.makeFlowConditionToken();
+ Context.addFlowConditionConstraint(FC4, C1);
+ EXPECT_FALSE(Context.flowConditionIsTautology(FC4));
+
+ // ... but we can prove A || !A is true.
+ auto &FC5 = Context.makeFlowConditionToken();
+ Context.addFlowConditionConstraint(
+ FC5, Context.getOrCreateDisjunctionValue(
+ C1, Context.getOrCreateNegationValue(C1)));
+ EXPECT_TRUE(Context.flowConditionIsTautology(FC5));
+}
+
} // namespace
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 45643cf - [clang][dataflow] Centralize expression skipping logic
Author: Eric Li
Date: 2022-05-05T20:28:11Z
New Revision: 45643cfcc12ea6152fb9516ed24f5adf7469eb4c
URL:
https://github.com/llvm/llvm-project/commit/45643cfcc12ea6152fb9516ed24f5adf7469eb4c
DIFF:
https://github.com/llvm/llvm-project/commit/45643cfcc12ea6152fb9516ed24f5adf7469eb4c.diff
LOG: [clang][dataflow] Centralize expression skipping logic
A follow-up to 62b2a47 to centralize the logic that skips expressions
that the CFG does not emit. This allows client code to avoid
sprinkling this logic everywhere.
Add redirects in the transfer function to similarly skip such
expressions by forwarding the visit to the sub-expression.
Differential Revision: https://reviews.llvm.org/D124965
Added:
Modified:
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
clang/include/clang/Analysis/FlowSensitive/Transfer.h
clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
clang/lib/Analysis/FlowSensitive/Transfer.cpp
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
Removed:
diff --git
a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
index a762cb9de48bd..e2820fcb55655 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
@@ -31,6 +31,19 @@
namespace clang {
namespace dataflow {
+/// Skip past nodes that the CFG does not emit. These nodes are invisible to
+/// flow-sensitive analysis, and should be ignored as they will effectively not
+/// exist.
+///
+/// * `ParenExpr` - The CFG takes the operator precedence into account, but
+/// otherwise omits the node afterwards.
+///
+/// * `ExprWithCleanups` - The CFG will generate the appropriate calls to
+/// destructors and then omit the node.
+///
+const Expr &ignoreCFGOmittedNodes(const Expr &E);
+const Stmt &ignoreCFGOmittedNodes(const Stmt &S);
+
/// Owns objects that encompass the state of a program and stores context that
/// is used during dataflow analysis.
class DataflowAnalysisContext {
@@ -95,14 +108,15 @@ class DataflowAnalysisContext {
///
/// `E` must not be assigned a storage location.
void setStorageLocation(const Expr &E, StorageLocation &Loc) {
-assert(ExprToLoc.find(&E) == ExprToLoc.end());
-ExprToLoc[&E] = &Loc;
+const Expr &CanonE = ignoreCFGOmittedNodes(E);
+assert(ExprToLoc.find(&CanonE) == ExprToLoc.end());
+ExprToLoc[&CanonE] = &Loc;
}
/// Returns the storage location assigned to `E` or null if `E` has no
/// assigned storage location.
StorageLocation *getStorageLocation(const Expr &E) const {
-auto It = ExprToLoc.find(&E);
+auto It = ExprToLoc.find(&ignoreCFGOmittedNodes(E));
return It == ExprToLoc.end() ? nullptr : It->second;
}
diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
index 019d07c9b7f72..0a2c75f804c2a 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -172,10 +172,6 @@ class Environment {
/// Creates a storage location for `E`. Does not assign the returned storage
/// location to `E` in the environment. Does not assign a value to the
/// returned storage location in the environment.
- ///
- /// Requirements:
- ///
- /// `E` must not be a `ExprWithCleanups`.
StorageLocation &createStorageLocation(const Expr &E);
/// Assigns `Loc` as the storage location of `D` in the environment.
@@ -195,16 +191,11 @@ class Environment {
/// Requirements:
///
/// `E` must not be assigned a storage location in the environment.
- /// `E` must not be a `ExprWithCleanups`.
void setStorageLocation(const Expr &E, StorageLocation &Loc);
/// Returns the storage location assigned to `E` in the environment, applying
/// the `SP` policy for skipping past indirections, or null if `E` isn't
/// assigned a storage location in the environment.
- ///
- /// Requirements:
- ///
- /// `E` must not be a `ExprWithCleanups`.
StorageLocation *getStorageLocation(const Expr &E, SkipPast SP) const;
/// Returns the storage location assigned to the `this` pointee in the
@@ -235,12 +226,6 @@ class Environment {
/// Equivalent to `getValue(getStorageLocation(E, SP), SkipPast::None)` if
`E`
/// is assigned a storage location in the environment, otherwise returns
null.
- ///
- /// Requirements:
- ///
- /// `E` must not be a `ExprWithCleanups`.
- ///
- /// FIXME: `Environment` should ignore any `ExprWithCleanups` it sees.
Value *getValue(const Expr &E, SkipPast SP
[clang] 29d35ec - [clang][dataflow] Fix MapLattice::insert() to not drop return value
Author: Eric Li
Date: 2022-07-25T14:24:33-04:00
New Revision: 29d35ece8249f2d8a51437a5c008e6bf63da9874
URL:
https://github.com/llvm/llvm-project/commit/29d35ece8249f2d8a51437a5c008e6bf63da9874
DIFF:
https://github.com/llvm/llvm-project/commit/29d35ece8249f2d8a51437a5c008e6bf63da9874.diff
LOG: [clang][dataflow] Fix MapLattice::insert() to not drop return value
Fix `MapLattice` API to return `std::pair`,
allowing users to detect when an element has been inserted without
performing a redundant map lookup.
Differential Revision: https://reviews.llvm.org/D130497
Added:
Modified:
clang/include/clang/Analysis/FlowSensitive/MapLattice.h
clang/unittests/Analysis/FlowSensitive/MapLatticeTest.cpp
Removed:
diff --git a/clang/include/clang/Analysis/FlowSensitive/MapLattice.h
b/clang/include/clang/Analysis/FlowSensitive/MapLattice.h
index 014cd60841ee7..16b0c978779a7 100644
--- a/clang/include/clang/Analysis/FlowSensitive/MapLattice.h
+++ b/clang/include/clang/Analysis/FlowSensitive/MapLattice.h
@@ -54,10 +54,13 @@ template class
MapLattice {
// The `bottom` element is the empty map.
static MapLattice bottom() { return MapLattice(); }
- void insert(const std::pair &P) { C.insert(P); }
+ std::pair
+ insert(const std::pair &P) {
+return C.insert(P);
+ }
- void insert(std::pair &&P) {
-C.insert(std::move(P));
+ std::pair insert(std::pair &&P)
{
+return C.insert(std::move(P));
}
unsigned size() const { return C.size(); }
diff --git a/clang/unittests/Analysis/FlowSensitive/MapLatticeTest.cpp
b/clang/unittests/Analysis/FlowSensitive/MapLatticeTest.cpp
index d3436e8f9496b..f6fe81e0dfc5f 100644
--- a/clang/unittests/Analysis/FlowSensitive/MapLatticeTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/MapLatticeTest.cpp
@@ -50,13 +50,18 @@ static constexpr int Key1 = 0;
static constexpr int Key2 = 1;
namespace {
+using ::testing::_;
using ::testing::Pair;
using ::testing::UnorderedElementsAre;
TEST(MapLatticeTest, InsertWorks) {
MapLattice Lattice;
- Lattice.insert({Key1, BooleanLattice(false)});
- Lattice.insert({Key2, BooleanLattice(false)});
+ EXPECT_THAT(Lattice.insert({Key1, BooleanLattice(false)}), Pair(_, true));
+ EXPECT_THAT(Lattice.insert({Key2, BooleanLattice(false)}), Pair(_, true));
+
+ // Insertion fails on collision.
+ EXPECT_THAT(Lattice.insert({Key1, BooleanLattice(false)}), Pair(_, false));
+ EXPECT_THAT(Lattice.insert({Key2, BooleanLattice(false)}), Pair(_, false));
EXPECT_THAT(Lattice, UnorderedElementsAre(Pair(Key1, BooleanLattice(false)),
Pair(Key2,
BooleanLattice(false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 854c273 - [clang][dataflow] Weaken guard to only check for storage location
Author: Eric Li
Date: 2022-05-17T18:58:07Z
New Revision: 854c273cbb7ec6745dc63cfe1951b51e9092e8ee
URL:
https://github.com/llvm/llvm-project/commit/854c273cbb7ec6745dc63cfe1951b51e9092e8ee
DIFF:
https://github.com/llvm/llvm-project/commit/854c273cbb7ec6745dc63cfe1951b51e9092e8ee.diff
LOG: [clang][dataflow] Weaken guard to only check for storage location
Weaken the guard for whether a sub-expression has been evaluated to
only check for the storage location, instead of checking for the
value. It should be sufficient to check for the storage location, as
we don't necessarily guarantee that a value will be set for the
location (although this is currently true right now).
Differential Revision: https://reviews.llvm.org/D125823
Added:
Modified:
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
Removed:
diff --git a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
index a8d739c155ab..aebc9dd6f6fa 100644
--- a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -104,7 +104,7 @@ class TerminatorVisitor : public
ConstStmtVisitor {
private:
void extendFlowCondition(const Expr &Cond) {
// The terminator sub-expression might not be evaluated.
-if (Env.getValue(Cond, SkipPast::None) == nullptr)
+if (Env.getStorageLocation(Cond, SkipPast::None) == nullptr)
transfer(StmtToEnv, Cond, Env);
// FIXME: The flow condition must be an r-value, so `SkipPast::None` should
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 5bbef2e - [clang][dataflow] Fix double visitation of nested logical operators
Author: Eric Li
Date: 2022-05-17T20:28:48Z
New Revision: 5bbef2e3fff123293ed9c2037e2662e352bf37af
URL:
https://github.com/llvm/llvm-project/commit/5bbef2e3fff123293ed9c2037e2662e352bf37af
DIFF:
https://github.com/llvm/llvm-project/commit/5bbef2e3fff123293ed9c2037e2662e352bf37af.diff
LOG: [clang][dataflow] Fix double visitation of nested logical operators
Sub-expressions that are logical operators are not spelled out
separately in basic blocks, so we need to manually visit them when we
encounter them. We do this in both the `TerminatorVisitor`
(conditionally) and the `TransferVisitor` (unconditionally), which can
cause cause an expression to be visited twice when the binary
operators are nested 2+ times.
This changes the visit in `TransferVisitor` to check if it has been
evaluated before trying to visit the sub-expression.
Differential Revision: https://reviews.llvm.org/D125821
Added:
Modified:
clang/lib/Analysis/FlowSensitive/Transfer.cpp
clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
Removed:
diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index 3a94434b7aeb7..f88406595d728 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -571,12 +571,15 @@ class TransferVisitor : public
ConstStmtVisitor {
return *Val;
}
-// Sub-expressions that are logic operators are not added in basic blocks
-// (e.g. see CFG for `bool d = a && (b || c);`). If `SubExpr` is a logic
-// operator, it isn't evaluated and assigned a value yet. In that case, we
-// need to first visit `SubExpr` and then try to get the value that gets
-// assigned to it.
-Visit(&SubExpr);
+if (Env.getStorageLocation(SubExpr, SkipPast::None) == nullptr) {
+ // Sub-expressions that are logic operators are not added in basic blocks
+ // (e.g. see CFG for `bool d = a && (b || c);`). If `SubExpr` is a logic
+ // operator, it may not have been evaluated and assigned a value yet. In
+ // that case, we need to first visit `SubExpr` and then try to get the
+ // value that gets assigned to it.
+ Visit(&SubExpr);
+}
+
if (auto *Val = dyn_cast_or_null(
Env.getValue(SubExpr, SkipPast::Reference)))
return *Val;
diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index db1f1cff311c7..9819409291040 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -2467,6 +2467,67 @@ TEST_F(TransferTest, AssignFromCompositeBoolExpression) {
EXPECT_EQ(&BazVal->getRightSubValue(), BarVal);
});
}
+
+ {
+std::string Code = R"(
+ void target(bool A, bool B, bool C, bool D) {
+bool Foo = ((A && B) && C) && D;
+// [[p]]
+ }
+)";
+runDataflow(
+Code, [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+ const Environment &Env = Results[0].second.Env;
+
+ const ValueDecl *ADecl = findValueDecl(ASTCtx, "A");
+ ASSERT_THAT(ADecl, NotNull());
+
+ const auto *AVal =
+ dyn_cast_or_null(Env.getValue(*ADecl,
SkipPast::None));
+ ASSERT_THAT(AVal, NotNull());
+
+ const ValueDecl *BDecl = findValueDecl(ASTCtx, "B");
+ ASSERT_THAT(BDecl, NotNull());
+
+ const auto *BVal =
+ dyn_cast_or_null(Env.getValue(*BDecl,
SkipPast::None));
+ ASSERT_THAT(BVal, NotNull());
+
+ const ValueDecl *CDecl = findValueDecl(ASTCtx, "C");
+ ASSERT_THAT(CDecl, NotNull());
+
+ const auto *CVal =
+ dyn_cast_or_null(Env.getValue(*CDecl,
SkipPast::None));
+ ASSERT_THAT(CVal, NotNull());
+
+ const ValueDecl *DDecl = findValueDecl(ASTCtx, "D");
+ ASSERT_THAT(DDecl, NotNull());
+
+ const auto *DVal =
+ dyn_cast_or_null(Env.getValue(*DDecl,
SkipPast::None));
+ ASSERT_THAT(DVal, NotNull());
+
+ const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ ASSERT_THAT(FooDecl, NotNull());
+
+ const auto *FooVal = dyn_cast_or_null(
+ Env.getValue(*FooDecl, SkipPast::None));
+ ASSERT_THAT(FooVal, NotNull());
+
+ const auto &FooLeftSubVal =
+ cast(FooVal->getLeftSubValue());
+ const auto &FooLeftLeftSubVal =
+ cast(FooLeftSubVal.getLeftSubValue());
+ EXPECT_EQ(&FooLeftLeftSubVal.getLeftSubValue(), AVal);
+ EXPECT_EQ(&FooLeftLeftSubVal.getRightSubValue(), BVal);
+ EXPECT_EQ(&FooLeftSubVal.getRightSubValue(), CVal);
+
[clang] [clang][dataflow] Fix crash when analyzing a coroutine (PR #85957)
https://github.com/tJener created
https://github.com/llvm/llvm-project/pull/85957
A coroutine function body (`CoroutineBodyStmt`) may have null children, which
causes `isa` to segfault.
>From 386e7dea15739df65ad9ecb3dd7f7dcd23b6acae Mon Sep 17 00:00:00 2001
From: Eric Li
Date: Wed, 20 Mar 2024 12:15:45 -0400
Subject: [PATCH] [clang][dataflow] Fix crash when analyzing a coroutine
A coroutine function body (`CoroutineBodyStmt`) may have null
children, which causes `isa` to segfault.
---
.../lib/Analysis/FlowSensitive/AdornedCFG.cpp | 2 +-
.../Analysis/FlowSensitive/TransferTest.cpp | 53 ++-
2 files changed, 52 insertions(+), 3 deletions(-)
diff --git a/clang/lib/Analysis/FlowSensitive/AdornedCFG.cpp
b/clang/lib/Analysis/FlowSensitive/AdornedCFG.cpp
index 3813b8c3ee8a23..daa73bed1bd9f5 100644
--- a/clang/lib/Analysis/FlowSensitive/AdornedCFG.cpp
+++ b/clang/lib/Analysis/FlowSensitive/AdornedCFG.cpp
@@ -103,7 +103,7 @@ buildContainsExprConsumedInDifferentBlock(
auto CheckChildExprs = [&Result, &StmtToBlock](const Stmt *S,
const CFGBlock *Block) {
for (const Stmt *Child : S->children()) {
- if (!isa(Child))
+ if (!isa_and_nonnull(Child))
continue;
const CFGBlock *ChildBlock = StmtToBlock.lookup(Child);
if (ChildBlock != Block)
diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index a243535d387257..2dc01a655264f3 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -37,6 +37,40 @@ using ::testing::Ne;
using ::testing::NotNull;
using ::testing::UnorderedElementsAre;
+// Declares a minimal coroutine library.
+constexpr llvm::StringRef CoroutineLibrary = R"cc(
+struct promise;
+struct task;
+
+namespace std {
+template
+struct coroutine_traits {};
+template <>
+struct coroutine_traits {
+using promise_type = promise;
+};
+
+template
+struct coroutine_handle {
+static constexpr coroutine_handle from_address(void *addr) { return {}; }
+};
+} // namespace std
+
+struct awaitable {
+bool await_ready() const noexcept;
+void await_suspend(std::coroutine_handle) const noexcept;
+void await_resume() const noexcept;
+};
+struct task {};
+struct promise {
+task get_return_object();
+awaitable initial_suspend();
+awaitable final_suspend() noexcept;
+void unhandled_exception();
+void return_void();
+};
+)cc";
+
void runDataflow(
llvm::StringRef Code,
std::function<
@@ -4607,7 +4641,7 @@ TEST(TransferTest, LoopCanProveInvariantForBoolean) {
}
TEST(TransferTest, DoesNotCrashOnUnionThisExpr) {
- std::string Code = R"(
+ std::string Code = R"cc(
union Union {
int A;
float B;
@@ -4618,7 +4652,7 @@ TEST(TransferTest, DoesNotCrashOnUnionThisExpr) {
Union B;
A = B;
}
- )";
+ )cc";
// This is a crash regression test when calling the transfer function on a
// `CXXThisExpr` that refers to a union.
runDataflow(
@@ -4628,6 +4662,21 @@ TEST(TransferTest, DoesNotCrashOnUnionThisExpr) {
LangStandard::lang_cxx17, /*ApplyBuiltinTransfer=*/true, "operator=");
}
+TEST(TransferTest, DoesNotCrashOnNullChildren) {
+ std::string Code = (CoroutineLibrary + R"cc(
+task foo() noexcept {
+ co_return;
+}
+ )cc").str();
+ // This is a crash regression test when calling `AdornedCFG::build` on a
+ // statement (in this case, the `CoroutineBodyStmt`) with null children.
+ runDataflow(
+ Code,
+ [](const llvm::StringMap> &,
+ ASTContext &) {},
+ LangStandard::lang_cxx20, /*ApplyBuiltinTransfer=*/true, "foo");
+}
+
TEST(TransferTest, StructuredBindingAssignFromStructIntMembersToRefs) {
std::string Code = R"(
struct A {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Fix crash when analyzing a coroutine (PR #85957)
https://github.com/tJener updated
https://github.com/llvm/llvm-project/pull/85957
>From 386e7dea15739df65ad9ecb3dd7f7dcd23b6acae Mon Sep 17 00:00:00 2001
From: Eric Li
Date: Wed, 20 Mar 2024 12:15:45 -0400
Subject: [PATCH 1/2] [clang][dataflow] Fix crash when analyzing a coroutine
A coroutine function body (`CoroutineBodyStmt`) may have null
children, which causes `isa` to segfault.
---
.../lib/Analysis/FlowSensitive/AdornedCFG.cpp | 2 +-
.../Analysis/FlowSensitive/TransferTest.cpp | 53 ++-
2 files changed, 52 insertions(+), 3 deletions(-)
diff --git a/clang/lib/Analysis/FlowSensitive/AdornedCFG.cpp
b/clang/lib/Analysis/FlowSensitive/AdornedCFG.cpp
index 3813b8c3ee8a23..daa73bed1bd9f5 100644
--- a/clang/lib/Analysis/FlowSensitive/AdornedCFG.cpp
+++ b/clang/lib/Analysis/FlowSensitive/AdornedCFG.cpp
@@ -103,7 +103,7 @@ buildContainsExprConsumedInDifferentBlock(
auto CheckChildExprs = [&Result, &StmtToBlock](const Stmt *S,
const CFGBlock *Block) {
for (const Stmt *Child : S->children()) {
- if (!isa(Child))
+ if (!isa_and_nonnull(Child))
continue;
const CFGBlock *ChildBlock = StmtToBlock.lookup(Child);
if (ChildBlock != Block)
diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index a243535d387257..2dc01a655264f3 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -37,6 +37,40 @@ using ::testing::Ne;
using ::testing::NotNull;
using ::testing::UnorderedElementsAre;
+// Declares a minimal coroutine library.
+constexpr llvm::StringRef CoroutineLibrary = R"cc(
+struct promise;
+struct task;
+
+namespace std {
+template
+struct coroutine_traits {};
+template <>
+struct coroutine_traits {
+using promise_type = promise;
+};
+
+template
+struct coroutine_handle {
+static constexpr coroutine_handle from_address(void *addr) { return {}; }
+};
+} // namespace std
+
+struct awaitable {
+bool await_ready() const noexcept;
+void await_suspend(std::coroutine_handle) const noexcept;
+void await_resume() const noexcept;
+};
+struct task {};
+struct promise {
+task get_return_object();
+awaitable initial_suspend();
+awaitable final_suspend() noexcept;
+void unhandled_exception();
+void return_void();
+};
+)cc";
+
void runDataflow(
llvm::StringRef Code,
std::function<
@@ -4607,7 +4641,7 @@ TEST(TransferTest, LoopCanProveInvariantForBoolean) {
}
TEST(TransferTest, DoesNotCrashOnUnionThisExpr) {
- std::string Code = R"(
+ std::string Code = R"cc(
union Union {
int A;
float B;
@@ -4618,7 +4652,7 @@ TEST(TransferTest, DoesNotCrashOnUnionThisExpr) {
Union B;
A = B;
}
- )";
+ )cc";
// This is a crash regression test when calling the transfer function on a
// `CXXThisExpr` that refers to a union.
runDataflow(
@@ -4628,6 +4662,21 @@ TEST(TransferTest, DoesNotCrashOnUnionThisExpr) {
LangStandard::lang_cxx17, /*ApplyBuiltinTransfer=*/true, "operator=");
}
+TEST(TransferTest, DoesNotCrashOnNullChildren) {
+ std::string Code = (CoroutineLibrary + R"cc(
+task foo() noexcept {
+ co_return;
+}
+ )cc").str();
+ // This is a crash regression test when calling `AdornedCFG::build` on a
+ // statement (in this case, the `CoroutineBodyStmt`) with null children.
+ runDataflow(
+ Code,
+ [](const llvm::StringMap> &,
+ ASTContext &) {},
+ LangStandard::lang_cxx20, /*ApplyBuiltinTransfer=*/true, "foo");
+}
+
TEST(TransferTest, StructuredBindingAssignFromStructIntMembersToRefs) {
std::string Code = R"(
struct A {
>From 71a0ade85a22597d74dbd9507115ee74f6d3a2d4 Mon Sep 17 00:00:00 2001
From: Eric Li
Date: Wed, 20 Mar 2024 12:32:42 -0400
Subject: [PATCH 2/2] fixup! [clang][dataflow] Fix crash when analyzing a
coroutine
---
clang/unittests/Analysis/FlowSensitive/TransferTest.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index 2dc01a655264f3..b8f8f1ada2fb98 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -4667,7 +4667,8 @@ TEST(TransferTest, DoesNotCrashOnNullChildren) {
task foo() noexcept {
co_return;
}
- )cc").str();
+ )cc")
+ .str();
// This is a crash regression test when calling `AdornedCFG::build` on a
// statement (in this case, the `CoroutineBodyStmt`) with null children.
runDataflow(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Fix crash when analyzing a coroutine (PR #85957)
https://github.com/tJener updated
https://github.com/llvm/llvm-project/pull/85957
>From 386e7dea15739df65ad9ecb3dd7f7dcd23b6acae Mon Sep 17 00:00:00 2001
From: Eric Li
Date: Wed, 20 Mar 2024 12:15:45 -0400
Subject: [PATCH 1/3] [clang][dataflow] Fix crash when analyzing a coroutine
A coroutine function body (`CoroutineBodyStmt`) may have null
children, which causes `isa` to segfault.
---
.../lib/Analysis/FlowSensitive/AdornedCFG.cpp | 2 +-
.../Analysis/FlowSensitive/TransferTest.cpp | 53 ++-
2 files changed, 52 insertions(+), 3 deletions(-)
diff --git a/clang/lib/Analysis/FlowSensitive/AdornedCFG.cpp
b/clang/lib/Analysis/FlowSensitive/AdornedCFG.cpp
index 3813b8c3ee8a23..daa73bed1bd9f5 100644
--- a/clang/lib/Analysis/FlowSensitive/AdornedCFG.cpp
+++ b/clang/lib/Analysis/FlowSensitive/AdornedCFG.cpp
@@ -103,7 +103,7 @@ buildContainsExprConsumedInDifferentBlock(
auto CheckChildExprs = [&Result, &StmtToBlock](const Stmt *S,
const CFGBlock *Block) {
for (const Stmt *Child : S->children()) {
- if (!isa(Child))
+ if (!isa_and_nonnull(Child))
continue;
const CFGBlock *ChildBlock = StmtToBlock.lookup(Child);
if (ChildBlock != Block)
diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index a243535d387257..2dc01a655264f3 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -37,6 +37,40 @@ using ::testing::Ne;
using ::testing::NotNull;
using ::testing::UnorderedElementsAre;
+// Declares a minimal coroutine library.
+constexpr llvm::StringRef CoroutineLibrary = R"cc(
+struct promise;
+struct task;
+
+namespace std {
+template
+struct coroutine_traits {};
+template <>
+struct coroutine_traits {
+using promise_type = promise;
+};
+
+template
+struct coroutine_handle {
+static constexpr coroutine_handle from_address(void *addr) { return {}; }
+};
+} // namespace std
+
+struct awaitable {
+bool await_ready() const noexcept;
+void await_suspend(std::coroutine_handle) const noexcept;
+void await_resume() const noexcept;
+};
+struct task {};
+struct promise {
+task get_return_object();
+awaitable initial_suspend();
+awaitable final_suspend() noexcept;
+void unhandled_exception();
+void return_void();
+};
+)cc";
+
void runDataflow(
llvm::StringRef Code,
std::function<
@@ -4607,7 +4641,7 @@ TEST(TransferTest, LoopCanProveInvariantForBoolean) {
}
TEST(TransferTest, DoesNotCrashOnUnionThisExpr) {
- std::string Code = R"(
+ std::string Code = R"cc(
union Union {
int A;
float B;
@@ -4618,7 +4652,7 @@ TEST(TransferTest, DoesNotCrashOnUnionThisExpr) {
Union B;
A = B;
}
- )";
+ )cc";
// This is a crash regression test when calling the transfer function on a
// `CXXThisExpr` that refers to a union.
runDataflow(
@@ -4628,6 +4662,21 @@ TEST(TransferTest, DoesNotCrashOnUnionThisExpr) {
LangStandard::lang_cxx17, /*ApplyBuiltinTransfer=*/true, "operator=");
}
+TEST(TransferTest, DoesNotCrashOnNullChildren) {
+ std::string Code = (CoroutineLibrary + R"cc(
+task foo() noexcept {
+ co_return;
+}
+ )cc").str();
+ // This is a crash regression test when calling `AdornedCFG::build` on a
+ // statement (in this case, the `CoroutineBodyStmt`) with null children.
+ runDataflow(
+ Code,
+ [](const llvm::StringMap> &,
+ ASTContext &) {},
+ LangStandard::lang_cxx20, /*ApplyBuiltinTransfer=*/true, "foo");
+}
+
TEST(TransferTest, StructuredBindingAssignFromStructIntMembersToRefs) {
std::string Code = R"(
struct A {
>From 71a0ade85a22597d74dbd9507115ee74f6d3a2d4 Mon Sep 17 00:00:00 2001
From: Eric Li
Date: Wed, 20 Mar 2024 12:32:42 -0400
Subject: [PATCH 2/3] fixup! [clang][dataflow] Fix crash when analyzing a
coroutine
---
clang/unittests/Analysis/FlowSensitive/TransferTest.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index 2dc01a655264f3..b8f8f1ada2fb98 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -4667,7 +4667,8 @@ TEST(TransferTest, DoesNotCrashOnNullChildren) {
task foo() noexcept {
co_return;
}
- )cc").str();
+ )cc")
+ .str();
// This is a crash regression test when calling `AdornedCFG::build` on a
// statement (in this case, the `CoroutineBodyStmt`) with null children.
runDataflow(
>From a08e5818f5db6e4d4e08f5e4863454c06a731acf Mon Sep 17 00:00:00 2001
From: Eric Li
Date: Wed, 20 Mar 2024 12:38:18 -0400
Subject: [PATCH 3/3] fixup! [clang][dataflow] Fix crash when analyzing a
coroutine
---
clang/unittests/Analysis/Flow
[clang] [clang][dataflow] Fix crash when analyzing a coroutine (PR #85957)
@@ -4628,6 +4662,21 @@ TEST(TransferTest, DoesNotCrashOnUnionThisExpr) {
LangStandard::lang_cxx17, /*ApplyBuiltinTransfer=*/true, "operator=");
}
+TEST(TransferTest, DoesNotCrashOnNullChildren) {
+ std::string Code = (CoroutineLibrary + R"cc(
+task foo() noexcept {
+ co_return;
+}
+ )cc").str();
+ // This is a crash regression test when calling `AdornedCFG::build` on a
+ // statement (in this case, the `CoroutineBodyStmt`) with null children.
+ runDataflow(
+ Code,
+ [](const llvm::StringMap> &,
+ ASTContext &) {},
+ LangStandard::lang_cxx20, /*ApplyBuiltinTransfer=*/true, "foo");
tJener wrote:
Done.
https://github.com/llvm/llvm-project/pull/85957
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Fix crash when analyzing a coroutine (PR #85957)
https://github.com/tJener closed https://github.com/llvm/llvm-project/pull/85957 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 576283c - [clang] Support `constexpr` for some `ASTNodeKind` member functions
Author: Eric Li
Date: 2022-10-13T13:00:48-04:00
New Revision: 576283c3a8ef5078b3ec12fa442c14f3a1b5fea2
URL:
https://github.com/llvm/llvm-project/commit/576283c3a8ef5078b3ec12fa442c14f3a1b5fea2
DIFF:
https://github.com/llvm/llvm-project/commit/576283c3a8ef5078b3ec12fa442c14f3a1b5fea2.diff
LOG: [clang] Support `constexpr` for some `ASTNodeKind` member functions
Add `constexpr` support for:
* The `getFromNodeKind` factory function
* `isSame`
* `isNone`
* `hasPointerIdentity`
This enables these functions to be used in SFINAE context for AST node
types.
Differential Revision: https://reviews.llvm.org/D135816
Added:
Modified:
clang/include/clang/AST/ASTTypeTraits.h
clang/unittests/AST/ASTTypeTraitsTest.cpp
Removed:
diff --git a/clang/include/clang/AST/ASTTypeTraits.h
b/clang/include/clang/AST/ASTTypeTraits.h
index cd6b5143bf79..8713221a7378 100644
--- a/clang/include/clang/AST/ASTTypeTraits.h
+++ b/clang/include/clang/AST/ASTTypeTraits.h
@@ -51,11 +51,10 @@ enum TraversalKind {
class ASTNodeKind {
public:
/// Empty identifier. It matches nothing.
- ASTNodeKind() : KindId(NKI_None) {}
+ constexpr ASTNodeKind() : KindId(NKI_None) {}
/// Construct an identifier for T.
- template
- static ASTNodeKind getFromNodeKind() {
+ template static constexpr ASTNodeKind getFromNodeKind() {
return ASTNodeKind(KindToKindId::Id);
}
@@ -71,12 +70,12 @@ class ASTNodeKind {
/// \}
/// Returns \c true if \c this and \c Other represent the same kind.
- bool isSame(ASTNodeKind Other) const {
+ constexpr bool isSame(ASTNodeKind Other) const {
return KindId != NKI_None && KindId == Other.KindId;
}
/// Returns \c true only for the default \c ASTNodeKind()
- bool isNone() const { return KindId == NKI_None; }
+ constexpr bool isNone() const { return KindId == NKI_None; }
/// Returns \c true if \c this is a base kind of (or same as) \c Other.
/// \param Distance If non-null, used to return the distance between \c this
@@ -87,7 +86,7 @@ class ASTNodeKind {
StringRef asStringRef() const;
/// Strict weak ordering for ASTNodeKind.
- bool operator<(const ASTNodeKind &Other) const {
+ constexpr bool operator<(const ASTNodeKind &Other) const {
return KindId < Other.KindId;
}
@@ -121,7 +120,7 @@ class ASTNodeKind {
/// Check if the given ASTNodeKind identifies a type that offers pointer
/// identity. This is useful for the fast path in DynTypedNode.
- bool hasPointerIdentity() const {
+ constexpr bool hasPointerIdentity() const {
return KindId > NKI_LastKindWithoutPointerIdentity;
}
@@ -165,7 +164,7 @@ class ASTNodeKind {
};
/// Use getFromNodeKind() to construct the kind.
- ASTNodeKind(NodeKindId KindId) : KindId(KindId) {}
+ constexpr ASTNodeKind(NodeKindId KindId) : KindId(KindId) {}
/// Returns \c true if \c Base is a base kind of (or same as) \c
/// Derived.
diff --git a/clang/unittests/AST/ASTTypeTraitsTest.cpp
b/clang/unittests/AST/ASTTypeTraitsTest.cpp
index 9fcb00630209..dcea32d1f1c0 100644
--- a/clang/unittests/AST/ASTTypeTraitsTest.cpp
+++ b/clang/unittests/AST/ASTTypeTraitsTest.cpp
@@ -117,6 +117,47 @@ TEST(ASTNodeKind, UnknownKind) {
EXPECT_FALSE(DNT().isSame(DNT()));
}
+template
+constexpr bool HasPointerIdentity =
+ASTNodeKind::getFromNodeKind().hasPointerIdentity();
+
+TEST(ASTNodeKind, ConstexprHasPointerIdentity) {
+ EXPECT_TRUE(HasPointerIdentity);
+ EXPECT_TRUE(HasPointerIdentity);
+ EXPECT_FALSE(HasPointerIdentity);
+ EXPECT_FALSE(HasPointerIdentity);
+ EXPECT_FALSE(HasPointerIdentity);
+
+ constexpr bool DefaultConstructedHasPointerIdentity =
+ ASTNodeKind().hasPointerIdentity();
+ EXPECT_FALSE(DefaultConstructedHasPointerIdentity);
+}
+
+template
+constexpr bool NodeKindIsSame =
+
ASTNodeKind::getFromNodeKind().isSame(ASTNodeKind::getFromNodeKind());
+
+TEST(ASTNodeKind, ConstexprIsSame) {
+ EXPECT_TRUE((NodeKindIsSame));
+ EXPECT_FALSE((NodeKindIsSame));
+ EXPECT_FALSE((NodeKindIsSame));
+
+ constexpr bool DefaultConstructedIsSameToDefaultConstructed =
+ ASTNodeKind().isSame(ASTNodeKind());
+ EXPECT_FALSE(DefaultConstructedIsSameToDefaultConstructed);
+}
+
+template
+constexpr bool NodeKindIsNone = ASTNodeKind::getFromNodeKind().isNone();
+
+TEST(ASTNodeKind, ConstexprIsNone) {
+ EXPECT_FALSE(NodeKindIsNone);
+ EXPECT_TRUE(NodeKindIsNone);
+
+ constexpr bool DefaultConstructedIsNone = ASTNodeKind().isNone();
+ EXPECT_TRUE(DefaultConstructedIsNone);
+}
+
TEST(ASTNodeKind, Name) {
EXPECT_EQ("", ASTNodeKind().asStringRef());
#define VERIFY_NAME(Node) EXPECT_EQ(#Node, DNT().asStringRef());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 7d0cdbf - [libTooling] Add `getFileRange` as an alternative to `getRangeForEdit`
Author: Eric Li
Date: 2023-01-12T17:44:29-05:00
New Revision: 7d0cdbf69edf5b14101d9c11fe9ca4cf5228e81d
URL:
https://github.com/llvm/llvm-project/commit/7d0cdbf69edf5b14101d9c11fe9ca4cf5228e81d
DIFF:
https://github.com/llvm/llvm-project/commit/7d0cdbf69edf5b14101d9c11fe9ca4cf5228e81d.diff
LOG: [libTooling] Add `getFileRange` as an alternative to `getRangeForEdit`
Add a `getFileRange` function alongside the existing `getRangeForEdit`
as a way to get a contiguous range within a single file (similar to
`getRangeForEdit`) but without the restriction that it cannot be in a
system header.
This can be used where a tool may want to use the range to extract the
source text. In such cases, we don't want to restrict this from
pulling from system headers.
Differential Revision: https://reviews.llvm.org/D141634
Added:
Modified:
clang/include/clang/Tooling/Transformer/SourceCode.h
clang/lib/Tooling/Transformer/SourceCode.cpp
Removed:
diff --git a/clang/include/clang/Tooling/Transformer/SourceCode.h
b/clang/include/clang/Tooling/Transformer/SourceCode.h
index d95cb9b4ae187..266aae09d27d5 100644
--- a/clang/include/clang/Tooling/Transformer/SourceCode.h
+++ b/clang/include/clang/Tooling/Transformer/SourceCode.h
@@ -112,6 +112,27 @@ getRangeForEdit(const CharSourceRange &EditRange, const
ASTContext &Context,
return getRangeForEdit(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/SourceCode.cpp
b/clang/lib/Tooling/Transformer/SourceCode.cpp
index 008a83cde42c4..262ec3557f7be 100644
--- a/clang/lib/Tooling/Transformer/SourceCode.cpp
+++ b/clang/lib/Tooling/Transformer/SourceCode.cpp
@@ -50,8 +50,9 @@ CharSourceRange
clang::tooling::maybeExtendRange(CharSourceRange Range,
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 @@ llvm::Error clang::tooling::validateEditRange(const
CharSourceRange &Range,
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 @@ llvm::Error clang::tooling::validateEditRange(const
CharSourceRange &Range,
errc::invalid_argument, "Range begins and ends in
diff erent 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::validat
[clang] 2307029 - [libTooling] Rename `getRangeForEdit` as `getFileRangeForEdit`
Author: Eric Li
Date: 2023-01-18T13:58:26-05:00
New Revision: 2307029b1a43a86aa6614c33aa198addf82d486b
URL:
https://github.com/llvm/llvm-project/commit/2307029b1a43a86aa6614c33aa198addf82d486b
DIFF:
https://github.com/llvm/llvm-project/commit/2307029b1a43a86aa6614c33aa198addf82d486b.diff
LOG: [libTooling] Rename `getRangeForEdit` as `getFileRangeForEdit`
With the addition of `getFileRange`, we rename `getRangeForEdit` as
`getFileRangeForEdit` for consistency in the API.
Depends on D141634
Differential Revision: https://reviews.llvm.org/D141636
Added:
Modified:
clang/include/clang/Tooling/Transformer/SourceCode.h
clang/lib/Tooling/Transformer/RewriteRule.cpp
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 266aae09d27d5..44a4749db74c9 100644
--- a/clang/include/clang/Tooling/Transformer/SourceCode.h
+++ b/clang/include/clang/Tooling/Transformer/SourceCode.h
@@ -104,13 +104,14 @@ llvm::Error validateEditRange(const CharSourceRange
&Range,
/// 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
diff --git a/clang/lib/Tooling/Transformer/RewriteRule.cpp
b/clang/lib/Tooling/Transformer/RewriteRule.cpp
index afc281d5fa622..eefddc3494048 100644
--- a/clang/lib/Tooling/Transformer/RewriteRule.cpp
+++ b/clang/lib/Tooling/Transformer/RewriteRule.cpp
@@ -39,7 +39,7 @@ translateEdits(const MatchResult &Result, ArrayRef
ASTEdits) {
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
diff ers from
@@ -449,7 +449,7 @@ SourceLocation transformer::detail::getRuleMatchLoc(const
MatchResult &Result) {
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
index 262ec3557f7be..35edc261ef096 100644
--- a/clang/lib/Tooling/Transformer/SourceCode.cpp
+++ b/clang/lib/Tooling/Transformer/SourceCode.cpp
@@ -122,7 +122,7 @@ static CharSourceRange getRange(const CharSourceRange
&EditRange,
return Range;
}
-std::optional clang::tooling::getRangeForEdit(
+std::optional clang::tooling::getFileRangeForEdit(
const CharSourceRange &EditRange, const SourceManager &SM,
const LangOptions &LangOpts, bool IncludeMacroExpansion) {
CharSourceRange Range =
diff --git a/clang/unittests/Tooling/SourceCodeTest.cpp
b/clang/unittests/Tooling/SourceCodeTest.cpp
index 7a9bd329e8d46..3d1dbceb63a7f 100644
--- a/clang/unittests/Tooling/SourceCodeTest.cpp
+++ b/clang/unittests/Tooling/SourceCodeTest.cpp
@@ -25,7 +25,7 @@ using llvm::ValueIs;
using tooling::getAssociatedRange;
using tooling::getExtendedRange;
using tooling::getExtendedText;
-using tooling::getRangeForEdit;
+using tooling::getFileRangeForEdit;
using tooling::getText;
using tooling::maybeExtendRange;
using tooling::validateEditRange;
@@ -453,11 +453,11 @@ TEST(SourceCodeTest,
getAssociatedRangeInvalidForPartialExpansions) {
Visitor.runOver(Code);
}
-class GetRangeForEditTest : public testing::TestWithParam {};
-INSTANTIATE_TEST_SUITE_P(WithAndWithoutExpansions, GetRangeForEd
[clang] 33b598a - [clang][dataflow] Relax assert on existence of `this` pointee storage
Author: Eric Li
Date: 2022-05-25T20:58:02Z
New Revision: 33b598a808b9ef1a019e798f19855f203e09ad48
URL:
https://github.com/llvm/llvm-project/commit/33b598a808b9ef1a019e798f19855f203e09ad48
DIFF:
https://github.com/llvm/llvm-project/commit/33b598a808b9ef1a019e798f19855f203e09ad48.diff
LOG: [clang][dataflow] Relax assert on existence of `this` pointee storage
Support for unions is incomplete (per 99f7d55e) and the `this` pointee
storage location is not set for unions. The assert in
`VisitCXXThisExpr` is then guaranteed to trigger when analyzing member
functions of a union.
This commit changes the assert to an early-return. Any expression may
be undefined, and so having a value for the `CXXThisExpr` is not a
postcondition of the transfer function.
Differential Revision: https://reviews.llvm.org/D126405
Added:
Modified:
clang/lib/Analysis/FlowSensitive/Transfer.cpp
clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
Removed:
diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index f88406595d728..fc04e0b55ffc7 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -279,7 +279,10 @@ class TransferVisitor : public
ConstStmtVisitor {
void VisitCXXThisExpr(const CXXThisExpr *S) {
auto *ThisPointeeLoc = Env.getThisPointeeStorageLocation();
-assert(ThisPointeeLoc != nullptr);
+if (ThisPointeeLoc == nullptr)
+ // Unions are not supported yet, and will not have a location for the
+ // `this` expression's pointee.
+ return;
auto &Loc = Env.createStorageLocation(*S);
Env.setStorageLocation(*S, Loc);
diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index b6d2940a98daa..bb6e269969da7 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -42,10 +42,11 @@ class TransferTest : public ::testing::Test {
template
void runDataflow(llvm::StringRef Code, Matcher Match,
LangStandard::Kind Std = LangStandard::lang_cxx17,
- bool ApplyBuiltinTransfer = true) {
+ bool ApplyBuiltinTransfer = true,
+ llvm::StringRef TargetFun = "target") {
ASSERT_THAT_ERROR(
test::checkDataflow(
-Code, "target",
+Code, TargetFun,
[ApplyBuiltinTransfer](ASTContext &C, Environment &) {
return NoopAnalysis(C, ApplyBuiltinTransfer);
},
@@ -3175,4 +3176,27 @@ TEST_F(TransferTest,
LoopWithStructReferenceAssignmentConverges) {
});
}
+TEST_F(TransferTest, DoesNotCrashOnUnionThisExpr) {
+ std::string Code = R"(
+union Union {
+ int A;
+ float B;
+};
+
+void foo() {
+ Union A;
+ Union B;
+ A = B;
+}
+ )";
+ // This is a crash regression test when calling the transfer function on a
+ // `CXXThisExpr` that refers to a union.
+ runDataflow(
+ Code,
+ [](llvm::ArrayRef<
+ std::pair>>,
+ ASTContext &) {},
+ LangStandard::lang_cxx17, /*ApplyBuiltinTransfer=*/true, "operator=");
+}
+
} // namespace
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 5520c58 - [clang][dataflow] Fix incorrect CXXThisExpr pointee for lambdas
Author: Eric Li
Date: 2022-05-25T20:58:02Z
New Revision: 5520c5839016d1d98d8e01789eb17c910381bd6f
URL:
https://github.com/llvm/llvm-project/commit/5520c5839016d1d98d8e01789eb17c910381bd6f
DIFF:
https://github.com/llvm/llvm-project/commit/5520c5839016d1d98d8e01789eb17c910381bd6f.diff
LOG: [clang][dataflow] Fix incorrect CXXThisExpr pointee for lambdas
When constructing the `Environment`, the `this` pointee is established
for a `CXXMethodDecl` by looking at its parent. However, inside of
lambdas, a `CXXThisExpr` refers to the captured `this` coming from the
enclosing member function.
When establishing the `this` pointee for a function, we check whether
the function is a lambda, and check for an enclosing member function
to establish the `this` pointee storage location.
Differential Revision: https://reviews.llvm.org/D126413
Added:
Modified:
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
Removed:
diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index e555bee0e8bf..fe573bbf9f37 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -216,7 +216,12 @@ Environment::Environment(DataflowAnalysisContext &DACtx,
}
if (const auto *MethodDecl = dyn_cast(&DeclCtx)) {
-if (!MethodDecl->isStatic()) {
+auto *Parent = MethodDecl->getParent();
+assert(Parent != nullptr);
+if (Parent->isLambda())
+ MethodDecl = dyn_cast(Parent->getDeclContext());
+
+if (MethodDecl && !MethodDecl->isStatic()) {
QualType ThisPointeeType = MethodDecl->getThisObjectType();
// FIXME: Add support for union types.
if (!ThisPointeeType->isUnionType()) {
diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index bb6e269969da..bed6b975974c 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -1476,6 +1476,112 @@ TEST_F(TransferTest, ClassThisMember) {
});
}
+TEST_F(TransferTest, StructThisInLambda) {
+ std::string ThisCaptureCode = R"(
+struct A {
+ void frob() {
+[this]() {
+ int Foo = Bar;
+ // [[p1]]
+}();
+ }
+
+ int Bar;
+};
+ )";
+ runDataflow(
+ ThisCaptureCode,
+ [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p1", _)));
+const Environment &Env = Results[0].second.Env;
+
+const auto *ThisLoc = dyn_cast(
+Env.getThisPointeeStorageLocation());
+ASSERT_THAT(ThisLoc, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const auto *BarLoc =
+cast(&ThisLoc->getChild(*BarDecl));
+ASSERT_TRUE(isa_and_nonnull(BarLoc));
+
+const Value *BarVal = Env.getValue(*BarLoc);
+ASSERT_TRUE(isa_and_nonnull(BarVal));
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+EXPECT_EQ(Env.getValue(*FooDecl, SkipPast::None), BarVal);
+ },
+ LangStandard::lang_cxx17, /*ApplyBuiltinTransfer=*/true, "operator()");
+
+ std::string RefCaptureDefaultCode = R"(
+struct A {
+ void frob() {
+[&]() {
+ int Foo = Bar;
+ // [[p2]]
+}();
+ }
+
+ int Bar;
+};
+ )";
+ runDataflow(
+ RefCaptureDefaultCode,
+ [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p2", _)));
+const Environment &Env = Results[0].second.Env;
+
+const auto *ThisLoc = dyn_cast(
+Env.getThisPointeeStorageLocation());
+ASSERT_THAT(ThisLoc, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const auto *BarLoc =
+cast(&ThisLoc->getChild(*BarDecl));
+ASSERT_TRUE(isa_and_nonnull(BarLoc));
+
+const Value *BarVal = Env.getValue(*BarLoc);
+ASSERT_TRUE(isa_and_nonnull(BarVal));
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+EXPECT_EQ(Env.getValue(*FooDecl, SkipPast::None), BarVal);
+ },
+ LangStandard::lang_cxx17, /*ApplyBuiltinTransfer=*/true, "operator()");
+
+ std::string FreeFunctionLambdaCode = R"(
+void foo() {
+ int Bar;
+ [&]() {
+int Foo = Bar;
+// [[p3]]
+ }();
+}
+ )";
+ runDataflow(
+ FreeFunctionLambdaCode,
+ [](llvm::Ar
[clang] 3cb6ead - [clang][TypePrinter] Add option to skip over elaborated types
Author: Eric Li
Date: 2023-06-06T15:11:09-04:00
New Revision: 3cb6ead77c6668fcd1362763b2603752c6c595fa
URL:
https://github.com/llvm/llvm-project/commit/3cb6ead77c6668fcd1362763b2603752c6c595fa
DIFF:
https://github.com/llvm/llvm-project/commit/3cb6ead77c6668fcd1362763b2603752c6c595fa.diff
LOG: [clang][TypePrinter] Add option to skip over elaborated types
Elaborated types are sugar that represent how the type was spelled in
the original source. When printing a type outside of that original
context, the qualifiers as saved in the elaborated type will be
incorrect. Additionally, their existence also inhibits the use of
`PrintingCallbacks::isScopeVisible` as a customization point.
Differential Revision: https://reviews.llvm.org/D149677
Added:
Modified:
clang/include/clang/AST/PrettyPrinter.h
clang/lib/AST/TypePrinter.cpp
clang/unittests/AST/TypePrinterTest.cpp
Removed:
diff --git a/clang/include/clang/AST/PrettyPrinter.h
b/clang/include/clang/AST/PrettyPrinter.h
index 5aeaca7beda2f..8a0bc6dfb57b8 100644
--- a/clang/include/clang/AST/PrettyPrinter.h
+++ b/clang/include/clang/AST/PrettyPrinter.h
@@ -60,14 +60,15 @@ struct PrintingPolicy {
: Indentation(2), SuppressSpecifiers(false),
SuppressTagKeyword(LO.CPlusPlus), IncludeTagDefinition(false),
SuppressScope(false), SuppressUnwrittenScope(false),
-SuppressInlineNamespace(true), SuppressInitializers(false),
-ConstantArraySizeAsWritten(false), AnonymousTagLocations(true),
-SuppressStrongLifetime(false), SuppressLifetimeQualifiers(false),
+SuppressInlineNamespace(true), SuppressElaboration(false),
+SuppressInitializers(false), ConstantArraySizeAsWritten(false),
+AnonymousTagLocations(true), SuppressStrongLifetime(false),
+SuppressLifetimeQualifiers(false),
SuppressTemplateArgsInCXXConstructors(false),
SuppressDefaultTemplateArgs(true), Bool(LO.Bool),
Nullptr(LO.CPlusPlus11 || LO.C2x),
NullptrTypeInNamespace(LO.CPlusPlus),
-Restrict(LO.C99), Alignof(LO.CPlusPlus11),
-UnderscoreAlignof(LO.C11), UseVoidForZeroParams(!LO.CPlusPlus),
+Restrict(LO.C99), Alignof(LO.CPlusPlus11), UnderscoreAlignof(LO.C11),
+UseVoidForZeroParams(!LO.CPlusPlus),
SplitTemplateClosers(!LO.CPlusPlus11), TerseOutput(false),
PolishForDeclaration(false), Half(LO.Half),
MSWChar(LO.MicrosoftExt && !LO.WChar), IncludeNewlines(true),
@@ -139,6 +140,10 @@ struct PrintingPolicy {
/// removed.
unsigned SuppressInlineNamespace : 1;
+ /// Ignore qualifiers and tag keywords as specified by elaborated type sugar,
+ /// instead letting the underlying type print as normal.
+ unsigned SuppressElaboration : 1;
+
/// Suppress printing of variable initializers.
///
/// This flag is used when printing the loop variable in a for-range
diff --git a/clang/lib/AST/TypePrinter.cpp b/clang/lib/AST/TypePrinter.cpp
index 00f5fe0da8cdf..1b62f6630928c 100644
--- a/clang/lib/AST/TypePrinter.cpp
+++ b/clang/lib/AST/TypePrinter.cpp
@@ -1576,6 +1576,11 @@ void TypePrinter::printElaboratedBefore(const
ElaboratedType *T,
return;
}
+ if (Policy.SuppressElaboration) {
+printBefore(T->getNamedType(), OS);
+return;
+ }
+
// The tag definition will take care of these.
if (!Policy.IncludeTagDefinition)
{
@@ -1595,6 +1600,12 @@ void TypePrinter::printElaboratedAfter(const
ElaboratedType *T,
raw_ostream &OS) {
if (Policy.IncludeTagDefinition && T->getOwnedTagDecl())
return;
+
+ if (Policy.SuppressElaboration) {
+printAfter(T->getNamedType(), OS);
+return;
+ }
+
ElaboratedTypePolicyRAII PolicyRAII(Policy);
printAfter(T->getNamedType(), OS);
}
diff --git a/clang/unittests/AST/TypePrinterTest.cpp
b/clang/unittests/AST/TypePrinterTest.cpp
index 77ff442236686..f0a6eb7e9fd8c 100644
--- a/clang/unittests/AST/TypePrinterTest.cpp
+++ b/clang/unittests/AST/TypePrinterTest.cpp
@@ -97,6 +97,33 @@ TEST(TypePrinter, ParamsUglified) {
"const f *", Clean));
}
+TEST(TypePrinter, SuppressElaboration) {
+ llvm::StringLiteral Code = R"cpp(
+namespace shared {
+namespace a {
+template
+struct S {};
+} // namespace a
+namespace b {
+struct Foo {};
+} // namespace b
+using Alias = a::S;
+} // namespace shared
+ )cpp";
+
+ auto Matcher = typedefNameDecl(hasName("::shared::Alias"),
+ hasType(qualType().bind("id")));
+ ASSERT_TRUE(PrintedTypeMatches(
+ Code, {}, Matcher, "a::S",
+ [](PrintingPolicy &Policy) { Policy.FullyQualifiedName = true; }));
+ ASSERT_TRUE(PrintedTypeMatches(Code, {}, Matcher,
+ "shared::a::S",
+ [](PrintingPolicy &Policy) {
+
[clang] a78d4b5 - [libTooling] Add flag to getRangeForEdit to ignore macro expansions
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
getRangeForEdit(const CharSourceRange &EditRange, const SourceManager &SM,
-const LangOptions &LangOpts);
+const LangOptions &LangOpts, bool IncludeMacroExpansion =
true);
inline llvm::Optional
-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
-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 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) ||
+SpelledInMa
[clang] [libTooling] Fix `getFileRangeForEdit` for inner nested template types (PR #87673)
https://github.com/tJener updated
https://github.com/llvm/llvm-project/pull/87673
>From 697551dd155cd8ada0d93ce8aa2951a9f9f3d4b4 Mon Sep 17 00:00:00 2001
From: Eric Li
Date: Thu, 4 Apr 2024 14:04:25 -0400
Subject: [PATCH 1/2] [libTooling] Fix `getFileRangeForEdit` for inner nested
template types
When there is `>>` in source from the right brackets of a nested
template, the end location of the inner template points intto a
scratch space with a single `>` token. This prevents the lexer from
seeing the `>>` token in the original source.
However, this causes the range to appear to be partially in a macro,
and is problematic if we are trying to avoid ranges with any macro
expansions.
This change detects this odd case in token ranges, converting it to
the corresponding character range without the expansion.
---
clang/lib/Tooling/Transformer/SourceCode.cpp | 61 ++--
clang/unittests/Tooling/SourceCodeTest.cpp | 29 ++
2 files changed, 85 insertions(+), 5 deletions(-)
diff --git a/clang/lib/Tooling/Transformer/SourceCode.cpp
b/clang/lib/Tooling/Transformer/SourceCode.cpp
index 6aae834b0db563..114f6ba4833cb0 100644
--- a/clang/lib/Tooling/Transformer/SourceCode.cpp
+++ b/clang/lib/Tooling/Transformer/SourceCode.cpp
@@ -101,6 +101,56 @@ static bool spelledInMacroDefinition(SourceLocation Loc,
return false;
}
+// Returns the expansion loc of `Loc` if `Loc` is the location of the `>` token
+// of an inner nested template, where the inner and outer templates end
brackets
+// are spelled as `>>`.
+//
+// Clang handles the `>>` in nested templates by placing the `SourceLocation`
+// of the inner template end bracket in scratch space. This forces it to be a
+// separate token (otherwise it would be lexed as `>>`), but that means it also
+// looks like a macro.
+static std::optional getExpansionForNestedTemplateGreater(
+SourceLocation Loc, const SourceManager &SM, const LangOptions &LangOpts) {
+ if (Loc.isMacroID()) {
+auto SpellingLoc = SM.getSpellingLoc(Loc);
+auto ExpansionLoc = SM.getExpansionLoc(Loc);
+Token SpellingTok, ExpansionTok;
+if (Lexer::getRawToken(SpellingLoc, SpellingTok, SM, LangOpts,
+ /*IgnoreWhiteSpace=*/false)) {
+ return std::nullopt;
+}
+if (Lexer::getRawToken(ExpansionLoc, ExpansionTok, SM, LangOpts,
+ /*IgnoreWhiteSpace=*/false)) {
+ return std::nullopt;
+}
+if (SpellingTok.getKind() == tok::greater &&
+ExpansionTok.getKind() == tok::greatergreater) {
+ return ExpansionLoc;
+}
+ }
+ return std::nullopt;
+}
+
+// Returns `Range`, but adjusted to smooth over oddities introduced by Clang.
+static CharSourceRange
+cleanRangeForAvoidingMacros(CharSourceRange Range, const SourceManager &SM,
+const LangOptions &LangOpts) {
+ if (Range.isTokenRange()) {
+auto B =
+getExpansionForNestedTemplateGreater(Range.getBegin(), SM, LangOpts);
+auto E = getExpansionForNestedTemplateGreater(Range.getEnd(), SM,
LangOpts);
+if (E) {
+ // We can't use the expansion location with a token range, because that
+ // will lex as `>>`. So we instead convert to a char range.
+ return CharSourceRange::getCharRange(B.value_or(Range.getBegin()),
+ E->getLocWithOffset(1));
+} else if (B) {
+ return CharSourceRange::getTokenRange(*B, Range.getEnd());
+}
+ }
+ return Range;
+}
+
static CharSourceRange getRange(const CharSourceRange &EditRange,
const SourceManager &SM,
const LangOptions &LangOpts,
@@ -109,13 +159,14 @@ static CharSourceRange getRange(const CharSourceRange
&EditRange,
if (IncludeMacroExpansion) {
Range = Lexer::makeFileCharRange(EditRange, SM, LangOpts);
} else {
-if (spelledInMacroDefinition(EditRange.getBegin(), SM) ||
-spelledInMacroDefinition(EditRange.getEnd(), SM))
+auto AdjustedRange = cleanRangeForAvoidingMacros(EditRange, SM, LangOpts);
+if (spelledInMacroDefinition(AdjustedRange.getBegin(), SM) ||
+spelledInMacroDefinition(AdjustedRange.getEnd(), SM))
return {};
-auto B = SM.getSpellingLoc(EditRange.getBegin());
-auto E = SM.getSpellingLoc(EditRange.getEnd());
-if (EditRange.isTokenRange())
+auto B = SM.getSpellingLoc(AdjustedRange.getBegin());
+auto E = SM.getSpellingLoc(AdjustedRange.getEnd());
+if (AdjustedRange.isTokenRange())
E = Lexer::getLocForEndOfToken(E, 0, SM, LangOpts);
Range = CharSourceRange::getCharRange(B, E);
}
diff --git a/clang/unittests/Tooling/SourceCodeTest.cpp
b/clang/unittests/Tooling/SourceCodeTest.cpp
index 3641d2ee453f4a..def9701ccdf1e0 100644
--- a/clang/unittests/Tooling/SourceCodeTest.cpp
+++ b/clang/unittests/Tooling/SourceCodeTest.cpp
@@ -50,6 +50,15 @@ struct CallsVisitor : TestVisitor {
std::function OnCall;
};
+struct TypeLocVisitor :
[clang] [libTooling] Fix `getFileRangeForEdit` for inner nested template types (PR #87673)
https://github.com/tJener edited https://github.com/llvm/llvm-project/pull/87673 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [libTooling] Fix `getFileRangeForEdit` for inner nested template types (PR #87673)
https://github.com/tJener updated
https://github.com/llvm/llvm-project/pull/87673
>From 697551dd155cd8ada0d93ce8aa2951a9f9f3d4b4 Mon Sep 17 00:00:00 2001
From: Eric Li
Date: Thu, 4 Apr 2024 14:04:25 -0400
Subject: [PATCH 1/3] [libTooling] Fix `getFileRangeForEdit` for inner nested
template types
When there is `>>` in source from the right brackets of a nested
template, the end location of the inner template points intto a
scratch space with a single `>` token. This prevents the lexer from
seeing the `>>` token in the original source.
However, this causes the range to appear to be partially in a macro,
and is problematic if we are trying to avoid ranges with any macro
expansions.
This change detects this odd case in token ranges, converting it to
the corresponding character range without the expansion.
---
clang/lib/Tooling/Transformer/SourceCode.cpp | 61 ++--
clang/unittests/Tooling/SourceCodeTest.cpp | 29 ++
2 files changed, 85 insertions(+), 5 deletions(-)
diff --git a/clang/lib/Tooling/Transformer/SourceCode.cpp
b/clang/lib/Tooling/Transformer/SourceCode.cpp
index 6aae834b0db563..114f6ba4833cb0 100644
--- a/clang/lib/Tooling/Transformer/SourceCode.cpp
+++ b/clang/lib/Tooling/Transformer/SourceCode.cpp
@@ -101,6 +101,56 @@ static bool spelledInMacroDefinition(SourceLocation Loc,
return false;
}
+// Returns the expansion loc of `Loc` if `Loc` is the location of the `>` token
+// of an inner nested template, where the inner and outer templates end
brackets
+// are spelled as `>>`.
+//
+// Clang handles the `>>` in nested templates by placing the `SourceLocation`
+// of the inner template end bracket in scratch space. This forces it to be a
+// separate token (otherwise it would be lexed as `>>`), but that means it also
+// looks like a macro.
+static std::optional getExpansionForNestedTemplateGreater(
+SourceLocation Loc, const SourceManager &SM, const LangOptions &LangOpts) {
+ if (Loc.isMacroID()) {
+auto SpellingLoc = SM.getSpellingLoc(Loc);
+auto ExpansionLoc = SM.getExpansionLoc(Loc);
+Token SpellingTok, ExpansionTok;
+if (Lexer::getRawToken(SpellingLoc, SpellingTok, SM, LangOpts,
+ /*IgnoreWhiteSpace=*/false)) {
+ return std::nullopt;
+}
+if (Lexer::getRawToken(ExpansionLoc, ExpansionTok, SM, LangOpts,
+ /*IgnoreWhiteSpace=*/false)) {
+ return std::nullopt;
+}
+if (SpellingTok.getKind() == tok::greater &&
+ExpansionTok.getKind() == tok::greatergreater) {
+ return ExpansionLoc;
+}
+ }
+ return std::nullopt;
+}
+
+// Returns `Range`, but adjusted to smooth over oddities introduced by Clang.
+static CharSourceRange
+cleanRangeForAvoidingMacros(CharSourceRange Range, const SourceManager &SM,
+const LangOptions &LangOpts) {
+ if (Range.isTokenRange()) {
+auto B =
+getExpansionForNestedTemplateGreater(Range.getBegin(), SM, LangOpts);
+auto E = getExpansionForNestedTemplateGreater(Range.getEnd(), SM,
LangOpts);
+if (E) {
+ // We can't use the expansion location with a token range, because that
+ // will lex as `>>`. So we instead convert to a char range.
+ return CharSourceRange::getCharRange(B.value_or(Range.getBegin()),
+ E->getLocWithOffset(1));
+} else if (B) {
+ return CharSourceRange::getTokenRange(*B, Range.getEnd());
+}
+ }
+ return Range;
+}
+
static CharSourceRange getRange(const CharSourceRange &EditRange,
const SourceManager &SM,
const LangOptions &LangOpts,
@@ -109,13 +159,14 @@ static CharSourceRange getRange(const CharSourceRange
&EditRange,
if (IncludeMacroExpansion) {
Range = Lexer::makeFileCharRange(EditRange, SM, LangOpts);
} else {
-if (spelledInMacroDefinition(EditRange.getBegin(), SM) ||
-spelledInMacroDefinition(EditRange.getEnd(), SM))
+auto AdjustedRange = cleanRangeForAvoidingMacros(EditRange, SM, LangOpts);
+if (spelledInMacroDefinition(AdjustedRange.getBegin(), SM) ||
+spelledInMacroDefinition(AdjustedRange.getEnd(), SM))
return {};
-auto B = SM.getSpellingLoc(EditRange.getBegin());
-auto E = SM.getSpellingLoc(EditRange.getEnd());
-if (EditRange.isTokenRange())
+auto B = SM.getSpellingLoc(AdjustedRange.getBegin());
+auto E = SM.getSpellingLoc(AdjustedRange.getEnd());
+if (AdjustedRange.isTokenRange())
E = Lexer::getLocForEndOfToken(E, 0, SM, LangOpts);
Range = CharSourceRange::getCharRange(B, E);
}
diff --git a/clang/unittests/Tooling/SourceCodeTest.cpp
b/clang/unittests/Tooling/SourceCodeTest.cpp
index 3641d2ee453f4a..def9701ccdf1e0 100644
--- a/clang/unittests/Tooling/SourceCodeTest.cpp
+++ b/clang/unittests/Tooling/SourceCodeTest.cpp
@@ -50,6 +50,15 @@ struct CallsVisitor : TestVisitor {
std::function OnCall;
};
+struct TypeLocVisitor :
[clang] [libTooling] Fix `getFileRangeForEdit` for inner nested template types (PR #87673)
@@ -510,6 +519,26 @@ int c = M3(3);
Visitor.runOver(Code.code());
}
+TEST(SourceCodeTest, InnerNestedTemplate) {
tJener wrote:
Done.
https://github.com/llvm/llvm-project/pull/87673
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [libTooling] Fix `getFileRangeForEdit` for inner nested template types (PR #87673)
https://github.com/tJener closed https://github.com/llvm/llvm-project/pull/87673 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix `constructExprArgs` for direct-init and implicit construction (PR #139990)
https://github.com/tJener updated
https://github.com/llvm/llvm-project/pull/139990
>From c7f23d4280de89810606b5c34d7fb8d03d7a9c3f Mon Sep 17 00:00:00 2001
From: Eric Li
Date: Wed, 14 May 2025 21:21:07 -0400
Subject: [PATCH] [libTooling] Fix `constructExprArgs` for direct-init and
implicit construction
Use `getParenOrBraceRange()` to get the location of the opening paren
or braces instead of searching backwards from the first argument.
For implicit construction, get the range surrounding the first and
last arguments.
---
.../lib/Tooling/Transformer/RangeSelector.cpp | 73 ---
clang/unittests/Tooling/RangeSelectorTest.cpp | 43 +++
2 files changed, 89 insertions(+), 27 deletions(-)
diff --git a/clang/lib/Tooling/Transformer/RangeSelector.cpp
b/clang/lib/Tooling/Transformer/RangeSelector.cpp
index e84ddde74a707..73e6109da178e 100644
--- a/clang/lib/Tooling/Transformer/RangeSelector.cpp
+++ b/clang/lib/Tooling/Transformer/RangeSelector.cpp
@@ -281,49 +281,68 @@ RangeSelector transformer::statements(std::string ID) {
namespace {
-SourceLocation getRLoc(const CallExpr &E) { return E.getRParenLoc(); }
-
-SourceLocation getRLoc(const CXXConstructExpr &E) {
- return E.getParenOrBraceRange().getEnd();
-}
-
-tok::TokenKind getStartToken(const CallExpr &E) {
- return tok::TokenKind::l_paren;
-}
-
-tok::TokenKind getStartToken(const CXXConstructExpr &E) {
- return isa(E) ? tok::TokenKind::l_paren
-: tok::TokenKind::l_brace;
-}
-
-template
-SourceLocation findArgStartDelimiter(const ExprWithArgs &E, SourceLocation
RLoc,
+SourceLocation findArgStartDelimiter(const CallExpr &E, SourceLocation RLoc,
const SourceManager &SM,
const LangOptions &LangOpts) {
SourceLocation Loc = E.getNumArgs() == 0 ? RLoc : E.getArg(0)->getBeginLoc();
- return findPreviousTokenKind(Loc, SM, LangOpts, getStartToken(E));
+ return findPreviousTokenKind(Loc, SM, LangOpts, tok::TokenKind::l_paren);
}
-// Returns the range of the source between the call's or construct expr's
-// parentheses/braces.
-template
-CharSourceRange getArgumentsRange(const MatchResult &Result,
- const ExprWithArgs &CE) {
- const SourceLocation RLoc = getRLoc(CE);
+
+// Returns the location after the last argument of the construct expr. Returns
+// an invalid location if there are no arguments.
+SourceLocation findLastArgEnd(const CXXConstructExpr &CE,
+ const SourceManager &SM,
+ const LangOptions &LangOpts) {
+ for (int i = CE.getNumArgs() - 1; i >= 0; --i) {
+const Expr *Arg = CE.getArg(i);
+if (isa(Arg))
+ continue;
+return Lexer::getLocForEndOfToken(Arg->getEndLoc(), 0, SM, LangOpts);
+ }
+ return {};
+}
+
+// Returns the range of the source between the call's parentheses/braces.
+CharSourceRange getCallArgumentsRange(const MatchResult &Result,
+ const CallExpr &CE) {
+ const SourceLocation RLoc = CE.getRParenLoc();
return CharSourceRange::getCharRange(
findArgStartDelimiter(CE, RLoc, *Result.SourceManager,
Result.Context->getLangOpts())
.getLocWithOffset(1),
RLoc);
}
+
+// Returns the range of the source between the construct expr's
+// parentheses/braces.
+CharSourceRange getConstructArgumentsRange(const MatchResult &Result,
+ const CXXConstructExpr &CE) {
+ if (SourceRange R = CE.getParenOrBraceRange(); R.isValid()) {
+return CharSourceRange::getCharRange(
+Lexer::getLocForEndOfToken(R.getBegin(), 0, *Result.SourceManager,
+ Result.Context->getLangOpts()),
+R.getEnd());
+ }
+
+ if (CE.getNumArgs() > 0) {
+return CharSourceRange::getCharRange(
+CE.getArg(0)->getBeginLoc(),
+findLastArgEnd(CE, *Result.SourceManager,
+ Result.Context->getLangOpts()));
+ }
+
+ return {};
+}
+
} // namespace
RangeSelector transformer::callArgs(std::string ID) {
- return RelativeSelector>(std::move(ID));
+ return RelativeSelector(std::move(ID));
}
RangeSelector transformer::constructExprArgs(std::string ID) {
- return RelativeSelector>(std::move(ID));
+ return RelativeSelector(
+ std::move(ID));
}
namespace {
diff --git a/clang/unittests/Tooling/RangeSelectorTest.cpp
b/clang/unittests/Tooling/RangeSelectorTest.cpp
index 1bccf899f3f19..7abaa73b09e2c 100644
--- a/clang/unittests/Tooling/RangeSelectorTest.cpp
+++ b/clang/unittests/Tooling/RangeSelectorTest.cpp
@@ -693,6 +693,49 @@ TEST(RangeSelectorTest, ConstructExprNoArgs) {
EXPECT_THAT_EXPECTED(select(constructExprArgs(ID), Match), HasValue(""));
}
+TEST(RangeSelectorTest, ConstructExprArgsDirectInitialization) {
+ const StringRef Code = R"cc(
+struct C {
+ C(int, int);
+};
+void f() {
[clang] [libTooling] Fix `constructExprArgs` for direct-init and implicit construction (PR #139990)
https://github.com/tJener edited https://github.com/llvm/llvm-project/pull/139990 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix `constructExprArgs` for direct-init and implicit construction (PR #139990)
https://github.com/tJener updated
https://github.com/llvm/llvm-project/pull/139990
Rate limit · GitHub
body {
background-color: #f6f8fa;
color: #24292e;
font-family: -apple-system,BlinkMacSystemFont,Segoe
UI,Helvetica,Arial,sans-serif,Apple Color Emoji,Segoe UI Emoji,Segoe UI Symbol;
font-size: 14px;
line-height: 1.5;
margin: 0;
}
.container { margin: 50px auto; max-width: 600px; text-align: center;
padding: 0 24px; }
a { color: #0366d6; text-decoration: none; }
a:hover { text-decoration: underline; }
h1 { line-height: 60px; font-size: 48px; font-weight: 300; margin: 0px;
text-shadow: 0 1px 0 #fff; }
p { color: rgba(0, 0, 0, 0.5); margin: 20px 0 40px; }
ul { list-style: none; margin: 25px 0; padding: 0; }
li { display: table-cell; font-weight: bold; width: 1%; }
.logo { display: inline-block; margin-top: 35px; }
.logo-img-2x { display: none; }
@media
only screen and (-webkit-min-device-pixel-ratio: 2),
only screen and ( min--moz-device-pixel-ratio: 2),
only screen and ( -o-min-device-pixel-ratio: 2/1),
only screen and (min-device-pixel-ratio: 2),
only screen and (min-resolution: 192dpi),
only screen and (min-resolution: 2dppx) {
.logo-img-1x { display: none; }
.logo-img-2x { display: inline-block; }
}
#suggestions {
margin-top: 35px;
color: #ccc;
}
#suggestions a {
color: #66;
font-weight: 200;
font-size: 14px;
margin: 0 10px;
}
Whoa there!
You have exceeded a secondary rate limit.
Please wait a few minutes before you try again;
in some cases this may take up to an hour.
https://support.github.com/contact";>Contact Support —
https://githubstatus.com";>GitHub Status —
https://twitter.com/githubstatus";>@githubstatus
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [libTooling] Fix `constructExprArgs` for direct-init and implicit construction (PR #139990)
https://github.com/tJener updated
https://github.com/llvm/llvm-project/pull/139990
>From c7f23d4280de89810606b5c34d7fb8d03d7a9c3f Mon Sep 17 00:00:00 2001
From: Eric Li
Date: Wed, 14 May 2025 21:21:07 -0400
Subject: [PATCH 1/2] [libTooling] Fix `constructExprArgs` for direct-init and
implicit construction
Use `getParenOrBraceRange()` to get the location of the opening paren
or braces instead of searching backwards from the first argument.
For implicit construction, get the range surrounding the first and
last arguments.
---
.../lib/Tooling/Transformer/RangeSelector.cpp | 73 ---
clang/unittests/Tooling/RangeSelectorTest.cpp | 43 +++
2 files changed, 89 insertions(+), 27 deletions(-)
diff --git a/clang/lib/Tooling/Transformer/RangeSelector.cpp
b/clang/lib/Tooling/Transformer/RangeSelector.cpp
index e84ddde74a707..73e6109da178e 100644
--- a/clang/lib/Tooling/Transformer/RangeSelector.cpp
+++ b/clang/lib/Tooling/Transformer/RangeSelector.cpp
@@ -281,49 +281,68 @@ RangeSelector transformer::statements(std::string ID) {
namespace {
-SourceLocation getRLoc(const CallExpr &E) { return E.getRParenLoc(); }
-
-SourceLocation getRLoc(const CXXConstructExpr &E) {
- return E.getParenOrBraceRange().getEnd();
-}
-
-tok::TokenKind getStartToken(const CallExpr &E) {
- return tok::TokenKind::l_paren;
-}
-
-tok::TokenKind getStartToken(const CXXConstructExpr &E) {
- return isa(E) ? tok::TokenKind::l_paren
-: tok::TokenKind::l_brace;
-}
-
-template
-SourceLocation findArgStartDelimiter(const ExprWithArgs &E, SourceLocation
RLoc,
+SourceLocation findArgStartDelimiter(const CallExpr &E, SourceLocation RLoc,
const SourceManager &SM,
const LangOptions &LangOpts) {
SourceLocation Loc = E.getNumArgs() == 0 ? RLoc : E.getArg(0)->getBeginLoc();
- return findPreviousTokenKind(Loc, SM, LangOpts, getStartToken(E));
+ return findPreviousTokenKind(Loc, SM, LangOpts, tok::TokenKind::l_paren);
}
-// Returns the range of the source between the call's or construct expr's
-// parentheses/braces.
-template
-CharSourceRange getArgumentsRange(const MatchResult &Result,
- const ExprWithArgs &CE) {
- const SourceLocation RLoc = getRLoc(CE);
+
+// Returns the location after the last argument of the construct expr. Returns
+// an invalid location if there are no arguments.
+SourceLocation findLastArgEnd(const CXXConstructExpr &CE,
+ const SourceManager &SM,
+ const LangOptions &LangOpts) {
+ for (int i = CE.getNumArgs() - 1; i >= 0; --i) {
+const Expr *Arg = CE.getArg(i);
+if (isa(Arg))
+ continue;
+return Lexer::getLocForEndOfToken(Arg->getEndLoc(), 0, SM, LangOpts);
+ }
+ return {};
+}
+
+// Returns the range of the source between the call's parentheses/braces.
+CharSourceRange getCallArgumentsRange(const MatchResult &Result,
+ const CallExpr &CE) {
+ const SourceLocation RLoc = CE.getRParenLoc();
return CharSourceRange::getCharRange(
findArgStartDelimiter(CE, RLoc, *Result.SourceManager,
Result.Context->getLangOpts())
.getLocWithOffset(1),
RLoc);
}
+
+// Returns the range of the source between the construct expr's
+// parentheses/braces.
+CharSourceRange getConstructArgumentsRange(const MatchResult &Result,
+ const CXXConstructExpr &CE) {
+ if (SourceRange R = CE.getParenOrBraceRange(); R.isValid()) {
+return CharSourceRange::getCharRange(
+Lexer::getLocForEndOfToken(R.getBegin(), 0, *Result.SourceManager,
+ Result.Context->getLangOpts()),
+R.getEnd());
+ }
+
+ if (CE.getNumArgs() > 0) {
+return CharSourceRange::getCharRange(
+CE.getArg(0)->getBeginLoc(),
+findLastArgEnd(CE, *Result.SourceManager,
+ Result.Context->getLangOpts()));
+ }
+
+ return {};
+}
+
} // namespace
RangeSelector transformer::callArgs(std::string ID) {
- return RelativeSelector>(std::move(ID));
+ return RelativeSelector(std::move(ID));
}
RangeSelector transformer::constructExprArgs(std::string ID) {
- return RelativeSelector>(std::move(ID));
+ return RelativeSelector(
+ std::move(ID));
}
namespace {
diff --git a/clang/unittests/Tooling/RangeSelectorTest.cpp
b/clang/unittests/Tooling/RangeSelectorTest.cpp
index 1bccf899f3f19..7abaa73b09e2c 100644
--- a/clang/unittests/Tooling/RangeSelectorTest.cpp
+++ b/clang/unittests/Tooling/RangeSelectorTest.cpp
@@ -693,6 +693,49 @@ TEST(RangeSelectorTest, ConstructExprNoArgs) {
EXPECT_THAT_EXPECTED(select(constructExprArgs(ID), Match), HasValue(""));
}
+TEST(RangeSelectorTest, ConstructExprArgsDirectInitialization) {
+ const StringRef Code = R"cc(
+struct C {
+ C(int, int);
+};
+void f(
[clang] [libTooling] Fix `constructExprArgs` for direct-init and implicit construction (PR #139990)
https://github.com/tJener updated
https://github.com/llvm/llvm-project/pull/139990
>From c7f23d4280de89810606b5c34d7fb8d03d7a9c3f Mon Sep 17 00:00:00 2001
From: Eric Li
Date: Wed, 14 May 2025 21:21:07 -0400
Subject: [PATCH 1/3] [libTooling] Fix `constructExprArgs` for direct-init and
implicit construction
Use `getParenOrBraceRange()` to get the location of the opening paren
or braces instead of searching backwards from the first argument.
For implicit construction, get the range surrounding the first and
last arguments.
---
.../lib/Tooling/Transformer/RangeSelector.cpp | 73 ---
clang/unittests/Tooling/RangeSelectorTest.cpp | 43 +++
2 files changed, 89 insertions(+), 27 deletions(-)
diff --git a/clang/lib/Tooling/Transformer/RangeSelector.cpp
b/clang/lib/Tooling/Transformer/RangeSelector.cpp
index e84ddde74a707..73e6109da178e 100644
--- a/clang/lib/Tooling/Transformer/RangeSelector.cpp
+++ b/clang/lib/Tooling/Transformer/RangeSelector.cpp
@@ -281,49 +281,68 @@ RangeSelector transformer::statements(std::string ID) {
namespace {
-SourceLocation getRLoc(const CallExpr &E) { return E.getRParenLoc(); }
-
-SourceLocation getRLoc(const CXXConstructExpr &E) {
- return E.getParenOrBraceRange().getEnd();
-}
-
-tok::TokenKind getStartToken(const CallExpr &E) {
- return tok::TokenKind::l_paren;
-}
-
-tok::TokenKind getStartToken(const CXXConstructExpr &E) {
- return isa(E) ? tok::TokenKind::l_paren
-: tok::TokenKind::l_brace;
-}
-
-template
-SourceLocation findArgStartDelimiter(const ExprWithArgs &E, SourceLocation
RLoc,
+SourceLocation findArgStartDelimiter(const CallExpr &E, SourceLocation RLoc,
const SourceManager &SM,
const LangOptions &LangOpts) {
SourceLocation Loc = E.getNumArgs() == 0 ? RLoc : E.getArg(0)->getBeginLoc();
- return findPreviousTokenKind(Loc, SM, LangOpts, getStartToken(E));
+ return findPreviousTokenKind(Loc, SM, LangOpts, tok::TokenKind::l_paren);
}
-// Returns the range of the source between the call's or construct expr's
-// parentheses/braces.
-template
-CharSourceRange getArgumentsRange(const MatchResult &Result,
- const ExprWithArgs &CE) {
- const SourceLocation RLoc = getRLoc(CE);
+
+// Returns the location after the last argument of the construct expr. Returns
+// an invalid location if there are no arguments.
+SourceLocation findLastArgEnd(const CXXConstructExpr &CE,
+ const SourceManager &SM,
+ const LangOptions &LangOpts) {
+ for (int i = CE.getNumArgs() - 1; i >= 0; --i) {
+const Expr *Arg = CE.getArg(i);
+if (isa(Arg))
+ continue;
+return Lexer::getLocForEndOfToken(Arg->getEndLoc(), 0, SM, LangOpts);
+ }
+ return {};
+}
+
+// Returns the range of the source between the call's parentheses/braces.
+CharSourceRange getCallArgumentsRange(const MatchResult &Result,
+ const CallExpr &CE) {
+ const SourceLocation RLoc = CE.getRParenLoc();
return CharSourceRange::getCharRange(
findArgStartDelimiter(CE, RLoc, *Result.SourceManager,
Result.Context->getLangOpts())
.getLocWithOffset(1),
RLoc);
}
+
+// Returns the range of the source between the construct expr's
+// parentheses/braces.
+CharSourceRange getConstructArgumentsRange(const MatchResult &Result,
+ const CXXConstructExpr &CE) {
+ if (SourceRange R = CE.getParenOrBraceRange(); R.isValid()) {
+return CharSourceRange::getCharRange(
+Lexer::getLocForEndOfToken(R.getBegin(), 0, *Result.SourceManager,
+ Result.Context->getLangOpts()),
+R.getEnd());
+ }
+
+ if (CE.getNumArgs() > 0) {
+return CharSourceRange::getCharRange(
+CE.getArg(0)->getBeginLoc(),
+findLastArgEnd(CE, *Result.SourceManager,
+ Result.Context->getLangOpts()));
+ }
+
+ return {};
+}
+
} // namespace
RangeSelector transformer::callArgs(std::string ID) {
- return RelativeSelector>(std::move(ID));
+ return RelativeSelector(std::move(ID));
}
RangeSelector transformer::constructExprArgs(std::string ID) {
- return RelativeSelector>(std::move(ID));
+ return RelativeSelector(
+ std::move(ID));
}
namespace {
diff --git a/clang/unittests/Tooling/RangeSelectorTest.cpp
b/clang/unittests/Tooling/RangeSelectorTest.cpp
index 1bccf899f3f19..7abaa73b09e2c 100644
--- a/clang/unittests/Tooling/RangeSelectorTest.cpp
+++ b/clang/unittests/Tooling/RangeSelectorTest.cpp
@@ -693,6 +693,49 @@ TEST(RangeSelectorTest, ConstructExprNoArgs) {
EXPECT_THAT_EXPECTED(select(constructExprArgs(ID), Match), HasValue(""));
}
+TEST(RangeSelectorTest, ConstructExprArgsDirectInitialization) {
+ const StringRef Code = R"cc(
+struct C {
+ C(int, int);
+};
+void f(
[clang] [libTooling] Fix `constructExprArgs` for direct-init and implicit construction (PR #139990)
tJener wrote: 5c57e88 should fix the test failure. Forgot that they are sometimes run in pre-C++17, so there was an extra elidable `CXXConstructExpr`. https://github.com/llvm/llvm-project/pull/139990 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
