Skip to content

Use builtin recognition to detect std::move / std::forward. #4668

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions clang/include/clang/AST/Expr.h
Original file line number Diff line number Diff line change
Expand Up @@ -3128,11 +3128,7 @@ class CallExpr : public Expr {
setDependence(getDependence() | ExprDependence::TypeValueInstantiation);
}

bool isCallToStdMove() const {
const FunctionDecl *FD = getDirectCallee();
return getNumArgs() == 1 && FD && FD->isInStdNamespace() &&
FD->getIdentifier() && FD->getIdentifier()->isStr("move");
}
bool isCallToStdMove() const;

static bool classof(const Stmt *T) {
return T->getStmtClass() >= firstCallExprConstant &&
Expand Down
2 changes: 1 addition & 1 deletion clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -4924,7 +4924,7 @@ def ext_adl_only_template_id : ExtWarn<
"with explicit template arguments is a C++20 extension">, InGroup<CXX20>;

def warn_unqualified_call_to_std_cast_function : Warning<
"unqualified call to %0">, InGroup<DiagGroup<"unqualified-std-cast-call">>;
"unqualified call to '%0'">, InGroup<DiagGroup<"unqualified-std-cast-call">>;

// C++ Template Argument Lists
def err_template_missing_args : Error<
Expand Down
13 changes: 8 additions & 5 deletions clang/lib/AST/Expr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1478,8 +1478,7 @@ Decl *Expr::getReferencedDeclOfCallee() {

/// If this is a call to a builtin, return the builtin ID. If not, return 0.
unsigned CallExpr::getBuiltinCallee() const {
auto *FDecl =
dyn_cast_or_null<FunctionDecl>(getCallee()->getReferencedDeclOfCallee());
auto *FDecl = getDirectCallee();
return FDecl ? FDecl->getBuiltinID() : 0;
}

Expand Down Expand Up @@ -3337,9 +3336,9 @@ bool Expr::isConstantInitializer(ASTContext &Ctx, bool IsForRef,
}

bool CallExpr::isBuiltinAssumeFalse(const ASTContext &Ctx) const {
const FunctionDecl* FD = getDirectCallee();
if (!FD || (FD->getBuiltinID() != Builtin::BI__assume &&
FD->getBuiltinID() != Builtin::BI__builtin_assume))
unsigned BuiltinID = getBuiltinCallee();
if (BuiltinID != Builtin::BI__assume &&
BuiltinID != Builtin::BI__builtin_assume)
return false;

const Expr* Arg = getArg(0);
Expand All @@ -3348,6 +3347,10 @@ bool CallExpr::isBuiltinAssumeFalse(const ASTContext &Ctx) const {
Arg->EvaluateAsBooleanCondition(ArgVal, Ctx) && !ArgVal;
}

bool CallExpr::isCallToStdMove() const {
return getBuiltinCallee() == Builtin::BImove;
}

namespace {
/// Look for any side effects within a Stmt.
class SideEffectFinder : public ConstEvaluatedExprVisitor<SideEffectFinder> {
Expand Down
11 changes: 5 additions & 6 deletions clang/lib/Sema/SemaExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6570,18 +6570,17 @@ static void DiagnosedUnqualifiedCallsToStdFunctions(Sema &S, CallExpr *Call) {
if (DRE->getQualifier())
return;

NamedDecl *D = dyn_cast_or_null<NamedDecl>(Call->getCalleeDecl());
if (!D || !D->isInStdNamespace())
const FunctionDecl *FD = Call->getDirectCallee();
if (!FD)
return;

// Only warn for some functions deemed more frequent or problematic.
static constexpr llvm::StringRef SpecialFunctions[] = {"move", "forward"};
auto it = llvm::find(SpecialFunctions, D->getName());
if (it == std::end(SpecialFunctions))
unsigned BuiltinID = FD->getBuiltinID();
if (BuiltinID != Builtin::BImove && BuiltinID != Builtin::BIforward)
return;

S.Diag(DRE->getLocation(), diag::warn_unqualified_call_to_std_cast_function)
<< D->getQualifiedNameAsString()
<< FD->getQualifiedNameAsString()
<< FixItHint::CreateInsertion(DRE->getLocation(), "std::");
}

Expand Down
4 changes: 2 additions & 2 deletions clang/test/SemaCXX/unqualified-std-call-fixits.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ using namespace std;

void f() {
int i = 0;
(void)move(i); // expected-warning {{unqualified call to std::move}}
(void)move(i); // expected-warning {{unqualified call to 'std::move}}
// CHECK: {{^}} (void)std::move
(void)forward(i); // expected-warning {{unqualified call to std::forward}}
(void)forward(i); // expected-warning {{unqualified call to 'std::forward}}
// CHECK: {{^}} (void)std::forward
}
30 changes: 15 additions & 15 deletions clang/test/SemaCXX/unqualified-std-call.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,23 +25,23 @@ using namespace std;
void f() {
int i = 0;
std::move(i);
move(i); // expected-warning{{unqualified call to std::move}}
(move)(i); // expected-warning{{unqualified call to std::move}}
move(i); // expected-warning{{unqualified call to 'std::move'}}
(move)(i); // expected-warning{{unqualified call to 'std::move'}}
std::dummy(1);
dummy(1);
std::move(1, 2);
move(1, 2);
forward<int>(i); // expected-warning{{unqualified call to std::forward}}
forward<int>(i); // expected-warning{{unqualified call to 'std::forward'}}
std::forward<int>(i);
}

template <typename T>
void g(T &&foo) {
std::move(foo);
move(foo); // expected-warning{{unqualified call to std::move}}
move(foo); // expected-warning{{unqualified call to 'std::move}}

std::forward<decltype(foo)>(foo);
forward<decltype(foo)>(foo); // expected-warning{{unqualified call to std::forward}}
forward<decltype(foo)>(foo); // expected-warning{{unqualified call to 'std::forward}}
move(1, 2);
dummy(foo);
}
Expand All @@ -59,16 +59,16 @@ using std::move;

void f() {
int i = 0;
move(i); // expected-warning{{unqualified call to std::move}}
move(i); // expected-warning{{unqualified call to 'std::move}}
move(1, 2);
forward<int>(i); // expected-warning{{unqualified call to std::forward}}
forward<int>(i); // expected-warning{{unqualified call to 'std::forward}}
}

template <typename T>
void g(T &&foo) {
move(foo); // expected-warning{{unqualified call to std::move}}
forward<decltype(foo)>(foo); // expected-warning{{unqualified call to std::forward}}
(forward<decltype(foo)>)(foo); // expected-warning{{unqualified call to std::forward}}
move(foo); // expected-warning{{unqualified call to 'std::move}}
forward<decltype(foo)>(foo); // expected-warning{{unqualified call to 'std::forward}}
(forward<decltype(foo)>)(foo); // expected-warning{{unqualified call to 'std::forward}}
move(1, 2);
}

Expand All @@ -90,7 +90,7 @@ void f() {

namespace adl {
void f() {
move(std::foo{}); // expected-warning{{unqualified call to std::move}}
move(std::foo{}); // expected-warning{{unqualified call to 'std::move}}
}

} // namespace adl
Expand All @@ -99,8 +99,8 @@ namespace std {

void f() {
int i = 0;
move(i); // expected-warning{{unqualified call to std::move}}
forward<int>(i); // expected-warning{{unqualified call to std::forward}}
move(i); // expected-warning{{unqualified call to 'std::move}}
forward<int>(i); // expected-warning{{unqualified call to 'std::forward}}
}

} // namespace std
Expand All @@ -110,9 +110,9 @@ namespace alias = std;
using namespace alias;
void f() {
int i = 0;
move(i); // expected-warning{{unqualified call to std::move}}
move(i); // expected-warning{{unqualified call to 'std::move}}
move(1, 2);
forward<int>(i); // expected-warning{{unqualified call to std::forward}}
forward<int>(i); // expected-warning{{unqualified call to 'std::forward}}
}

} // namespace test_alias
4 changes: 2 additions & 2 deletions clang/test/SemaCXX/warn-self-move.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ void int_test() {

using std::move;
x = move(x); // expected-warning{{explicitly moving}} \
expected-warning {{unqualified call to std::move}}
expected-warning {{unqualified call to 'std::move}}
}

int global;
Expand All @@ -28,7 +28,7 @@ void global_int_test() {

using std::move;
global = move(global); // expected-warning{{explicitly moving}} \
expected-warning {{unqualified call to std::move}}
expected-warning {{unqualified call to 'std::move}}
}

class field_test {
Expand Down