comex created this revision.
comex added a reviewer: dblaikie.
Herald added subscribers: cfe-commits, dmgreen, kristof.beyls.
Herald added a project: clang.
Currently, `handleCall` takes a `CallExpr` as an argument and retrieves the
arguments from there. This changes it to take the argument list as a separate
argument of type `ArrayRef`.
The main motivation that I want `VisitCXXConstructExpr` to also be able to
invoke `handleCall`, even though `CXXConstructExpr` is not a subclass of
`CallExpr`. But actually adding that will wait for a future patch.
However, this also fixes a minor bug in `VisitCXXOperatorCallExpr`:
if (const auto *MCall = dyn_cast<CXXMemberCallExpr>(Call))
handleCall(MCall, MCall->getImplicitObjectArgument(), FunDecl);
else
handleCall(Call, Call->getArg(0), FunDecl);
`Call` here is a `CXXOperatorCallExpr`, and `CXXMemberCallExpr` is a sibling
class, so the cast will never succeed. CXXMemberCallExprs go through a
separate visitor, `VisitCXXMemberCallExpr`.
On the other hand, this logic may be intended to reflect the fact that C++
operators can be declared as either methods or free functions. The correct way
to differentiate these is shown at the beginning of handleCall:
unsigned Offset = 0;
if (isa<CXXOperatorCallExpr>(Call) && isa<CXXMethodDecl>(FunD))
Offset = 1; // first argument is 'this'
For `CXXOperatorCallExpr`s, if the decl is a `CXXMethodDecl`, the first
argument is `this`; otherwise there is no `this`.
Going back to the other first: currently, since the `dyn_cast` always fails, it
always passes `Call->getArg(0)` as `ObjArg` (i.e. the expression representing
`this`), even for operators declared as free functions. However, this is
harmless, because `ObjArg` is only used if the function is marked as one of
`set_typestate` or `test_typestate`, or `callable_when`, yet those attributes
are only allowed on methods. `Call->getArg(0)` will crash if there are zero
arguments, but the only kind of non-method operator function allowed to have
zero arguments is a user-defined literal, and those do not produce
`CXXOperatorCallExpr`s.
The bug could be fixed by changing the first snippet to check for
`CXXMethodDecl` like the second one, but this refactor makes things cleaner by
only having to check in one place.
Repository:
rC Clang
https://reviews.llvm.org/D67647
Files:
lib/Analysis/Consumed.cpp
Index: lib/Analysis/Consumed.cpp
===================================================================
--- lib/Analysis/Consumed.cpp
+++ lib/Analysis/Consumed.cpp
@@ -494,8 +494,8 @@
void checkCallability(const PropagationInfo &PInfo,
const FunctionDecl *FunDecl,
SourceLocation BlameLoc);
- bool handleCall(const CallExpr *Call, const Expr *ObjArg,
- const FunctionDecl *FunD);
+ bool handleCall(const Expr *Call, const Expr *ObjArg,
+ ArrayRef<const Expr *> Args, const FunctionDecl *FunD);
void VisitBinaryOperator(const BinaryOperator *BinOp);
void VisitCallExpr(const CallExpr *Call);
@@ -608,22 +608,21 @@
// Factors out common behavior for function, method, and operator calls.
// Check parameters and set parameter state if necessary.
// Returns true if the state of ObjArg is set, or false otherwise.
-bool ConsumedStmtVisitor::handleCall(const CallExpr *Call, const Expr *ObjArg,
+bool ConsumedStmtVisitor::handleCall(const Expr *Call,
+ const Expr *ObjArg,
+ ArrayRef<const Expr *> Args,
const FunctionDecl *FunD) {
- unsigned Offset = 0;
- if (isa<CXXOperatorCallExpr>(Call) && isa<CXXMethodDecl>(FunD))
- Offset = 1; // first argument is 'this'
-
// check explicit parameters
- for (unsigned Index = Offset; Index < Call->getNumArgs(); ++Index) {
+ unsigned Index = 0;
+ for (const Expr *Arg : Args) {
// Skip variable argument lists.
- if (Index - Offset >= FunD->getNumParams())
+ if (Index >= FunD->getNumParams())
break;
- const ParmVarDecl *Param = FunD->getParamDecl(Index - Offset);
+ const ParmVarDecl *Param = FunD->getParamDecl(Index++);
QualType ParamType = Param->getType();
- InfoEntry Entry = findInfo(Call->getArg(Index));
+ InfoEntry Entry = findInfo(Arg);
if (Entry == PropagationMap.end() || Entry->second.isTest())
continue;
@@ -636,7 +635,7 @@
if (ParamState != ExpectedState)
Analyzer.WarningsHandler.warnParamTypestateMismatch(
- Call->getArg(Index)->getExprLoc(),
+ Arg->getExprLoc(),
stateToString(ExpectedState), stateToString(ParamState));
}
@@ -749,7 +748,9 @@
return;
}
- handleCall(Call, nullptr, FunDecl);
+ handleCall(Call, nullptr,
+ llvm::makeArrayRef(Call->getArgs(), Call->getNumArgs()),
+ FunDecl);
propagateReturnType(Call, FunDecl);
}
@@ -805,7 +806,8 @@
if (!MD)
return;
- handleCall(Call, Call->getImplicitObjectArgument(), MD);
+ handleCall(Call, Call->getImplicitObjectArgument(),
+ llvm::makeArrayRef(Call->getArgs(), Call->getNumArgs()), MD);
propagateReturnType(Call, MD);
}
@@ -813,18 +815,19 @@
const CXXOperatorCallExpr *Call) {
const auto *FunDecl = dyn_cast_or_null<FunctionDecl>(Call->getDirectCallee());
if (!FunDecl) return;
+ ArrayRef<const Expr *> Args(Call->getArgs(), Call->getNumArgs());
if (Call->getOperator() == OO_Equal) {
- ConsumedState CS = getInfo(Call->getArg(1));
- if (!handleCall(Call, Call->getArg(0), FunDecl))
- setInfo(Call->getArg(0), CS);
+ ConsumedState CS = getInfo(Args[1]);
+ if (!handleCall(Call, Args[0], Args.drop_front(1), FunDecl))
+ setInfo(Args[0], CS);
return;
}
- if (const auto *MCall = dyn_cast<CXXMemberCallExpr>(Call))
- handleCall(MCall, MCall->getImplicitObjectArgument(), FunDecl);
+ if (isa<CXXMethodDecl>(FunDecl))
+ handleCall(Call, Args[0], Args.drop_front(1), FunDecl);
else
- handleCall(Call, Call->getArg(0), FunDecl);
+ handleCall(Call, nullptr, Args, FunDecl);
propagateReturnType(Call, FunDecl);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
