Hi rsmith, dblaikie,
If a member of type unique_ptr or shared_ptr is move-constructed from
a parameter in ctor-init, the parameter will be left with nullptr at
the completion of member's initialization. If the parameter's and
member's names are the same, use of member's name in the constructor
body will not refer to the member, but to the parameter.
#include <memory>
struct X {
int v;
X() : v() { }
int f() { return v + 42; }
};
struct Y {
std::unique_ptr<X> p;
int x;
explicit Y(std::unique_ptr<X> p)
: p{std::move(p)} {
x = p->f(); // 'p' is nullptr
decltype(p->f()) v = x; // ignore unevaluated context
}
int f() const { return x; }
};
int main() {
std::unique_ptr<X> p(new X);
Y y(std::move(p));
return y.f();
}
http://reviews.llvm.org/D10425
Files:
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaDecl.cpp
test/SemaCXX/rval-references.cpp
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
Index: include/clang/Basic/DiagnosticGroups.td
===================================================================
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -307,6 +307,7 @@
def SelfAssignmentField : DiagGroup<"self-assign-field">;
def SelfAssignment : DiagGroup<"self-assign", [SelfAssignmentField]>;
def SelfMove : DiagGroup<"self-move">;
+def UseOfMovedParameter : DiagGroup<"use-of-moved-parameter">;
def SemiBeforeMethodBody : DiagGroup<"semicolon-before-method-body">;
def Sentinel : DiagGroup<"sentinel">;
def MissingMethodReturnType : DiagGroup<"missing-method-return-type">;
@@ -580,7 +581,8 @@
def IntToPointerCast : DiagGroup<"int-to-pointer-cast",
[IntToVoidPointerCast]>;
-def Move : DiagGroup<"move", [PessimizingMove, RedundantMove, SelfMove]>;
+def Move : DiagGroup<"move", [PessimizingMove, RedundantMove, SelfMove,
+ UseOfMovedParameter]>;
def Extra : DiagGroup<"extra", [
MissingFieldInitializers,
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -4794,6 +4794,10 @@
"moving a temporary object prevents copy elision">,
InGroup<PessimizingMove>, DefaultIgnore;
def note_remove_move : Note<"remove std::move call here">;
+def warn_move_from_param_to_member : Warning<
+ "parameter '%0' owns nullptr, because it has been moved into member "
+ "'%0' ; did you mean 'this->%0'?">, InGroup<UseOfMovedParameter>;
+def note_moved_from_here : Note<"parameter '%0' moved into member '%0' here">;
def warn_string_plus_int : Warning<
"adding %0 to a string does not append to the string">,
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -10602,6 +10602,114 @@
return ActOnFinishFunctionBody(D, BodyArg, false);
}
+namespace {
+class FindUsesOfParamsMovedFrom
+ : public EvaluatedExprVisitor<FindUsesOfParamsMovedFrom> {
+ const ParmVarDecl *Param;
+ llvm::SmallVector<SourceRange, 4> &UseRanges;
+ using Inherited = EvaluatedExprVisitor<FindUsesOfParamsMovedFrom>;
+
+public:
+ FindUsesOfParamsMovedFrom(ASTContext &Context, const ParmVarDecl *Param,
+ llvm::SmallVector<SourceRange, 4> &UseRanges)
+ : Inherited(Context), Param(Param), UseRanges(UseRanges) {}
+ void VisitCallExpr(CallExpr *CE) {
+ if (CXXOperatorCallExpr *OCE = dyn_cast<CXXOperatorCallExpr>(CE))
+ // We care about operator-> of smart pointers
+ // Check that thr first arg is Param!
+ if (OCE->getOperator() != OO_Arrow) {
+ return;
+ }
+ for (auto *Arg : CE->arguments()) {
+ if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(Arg->IgnoreCasts()))
+ if (dyn_cast<ParmVarDecl>(DRE->getDecl()) == Param) {
+ UseRanges.push_back(CE->getSourceRange());
+ return;
+ }
+ }
+ Inherited::VisitCallExpr(CE);
+ }
+};
+
+class FieldInitMovedFromParamVisitor
+ : public EvaluatedExprVisitor<FieldInitMovedFromParamVisitor> {
+ ParmVarDecl *Param;
+ StringRef FieldName;
+ static bool IsCXX11MovableSmartPtrType(QualType T) {
+ CXXRecordDecl *RD = T->getAsCXXRecordDecl();
+ if (!RD)
+ return false;
+ if (!RD->isInStdNamespace())
+ return false;
+
+ StringRef Name = RD->getName();
+ return Name == "shared_ptr" || Name == "unique_ptr";
+ }
+ bool CheckMovedFromParam(Expr *E) {
+ if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E)) {
+ if (ParmVarDecl *PVD = dyn_cast<ParmVarDecl>(DRE->getDecl())) {
+ if (PVD->getName() == FieldName &&
+ IsCXX11MovableSmartPtrType(PVD->getType())) {
+ Param = PVD;
+ return true;
+ }
+ }
+ }
+ return false;
+ }
+
+public:
+ typedef EvaluatedExprVisitor<FieldInitMovedFromParamVisitor> Inherited;
+ FieldInitMovedFromParamVisitor(ASTContext &Context, StringRef FieldName)
+ : Inherited(Context), Param(nullptr), FieldName(FieldName) {}
+ void VisitCallExpr(CallExpr *E) {
+ if (E->getNumArgs() == 1) {
+ if (FunctionDecl *FD = E->getDirectCallee()) {
+ if (FD->isInStdNamespace() && FD->getIdentifier() &&
+ FD->getIdentifier()->isStr("move") &&
+ CheckMovedFromParam(E->getArg(0)))
+ return;
+ }
+ }
+ Inherited::VisitCallExpr(E);
+ }
+ ParmVarDecl *getParam() { return Param; }
+};
+}
+
+static void DiagnoseUseOfMovedParams(Sema &SemaRef, const FunctionDecl *FD) {
+ // FIXME: This is limited to ctors for now
+ const CXXConstructorDecl *CD = dyn_cast<CXXConstructorDecl>(FD);
+ if (!CD)
+ return;
+ llvm::SmallVector<std::pair<ParmVarDecl *, SourceRange>, 4> ParamsAndMoveLoc;
+ for (const auto *FieldInit : CD->inits()) {
+ FieldDecl *FD = FieldInit->getAnyMember();
+ if (!FD)
+ continue;
+ Expr *E = FieldInit->getInit();
+ if (!E)
+ continue;
+ FieldInitMovedFromParamVisitor V(FD->getASTContext(), FD->getName());
+ V.Visit(E);
+ if (ParmVarDecl *P = V.getParam())
+ ParamsAndMoveLoc.push_back(std::make_pair(P, E->getSourceRange()));
+ }
+ for (const auto &Pair : ParamsAndMoveLoc) {
+ llvm::SmallVector<SourceRange, 4> UseRanges;
+ FindUsesOfParamsMovedFrom Vis(CD->getASTContext(), Pair.first, UseRanges);
+ Vis.Visit(CD->getBody());
+ for (const SourceRange &UseRange : UseRanges) {
+ SourceLocation Loc = UseRange.getBegin();
+ StringRef Name = Pair.first->getName();
+ SemaRef.Diag(Loc, diag::warn_move_from_param_to_member)
+ << Name << UseRange << FixItHint::CreateInsertion(Loc, "this->");
+ SemaRef.Diag(Pair.second.getBegin(), diag::note_moved_from_here)
+ << Name << Pair.second;
+ }
+ }
+}
+
Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body,
bool IsInstantiation) {
FunctionDecl *FD = dcl ? dcl->getAsFunction() : nullptr;
@@ -10670,8 +10778,11 @@
if (!FD->isInvalidDecl()) {
// Don't diagnose unused parameters of defaulted or deleted functions.
- if (!FD->isDeleted() && !FD->isDefaulted())
+ if (!FD->isDeleted() && !FD->isDefaulted()) {
DiagnoseUnusedParameters(FD->param_begin(), FD->param_end());
+ if (getLangOpts().CPlusPlus11)
+ DiagnoseUseOfMovedParams(*this, FD);
+ }
DiagnoseSizeOfParametersAndReturnValue(FD->param_begin(), FD->param_end(),
FD->getReturnType(), FD);
Index: test/SemaCXX/rval-references.cpp
===================================================================
--- test/SemaCXX/rval-references.cpp
+++ test/SemaCXX/rval-references.cpp
@@ -92,3 +92,82 @@
else // Construction from different type can't be elided
return i; // expected-error {{no viable conversion from 'int' to 'MoveOnly'}}
}
+
+namespace std {
+template <class T>
+struct remove_reference { typedef T type; };
+template <class T>
+struct remove_reference<T &> { typedef T type; };
+template <class T>
+struct remove_reference<T &&> { typedef T type; };
+
+template <typename T>
+typename remove_reference<T>::type &&move(T &&arg) {
+ return static_cast<typename remove_reference<T>::type &&>(arg);
+}
+
+template <typename T>
+class unique_ptr {
+ T *p;
+
+public:
+ unique_ptr() : p(nullptr) {}
+ unique_ptr(T *p) : p(std::move(p)) {}
+ unique_ptr(unique_ptr &&other) : p(other.release()) {}
+ T *release() {
+ T *tmp = p;
+ p = nullptr;
+ return tmp;
+ }
+ T *get() const { return p; }
+ T *operator->() const { return get(); }
+};
+}
+
+namespace ParmMovedFromWarning {
+struct X {
+ int v;
+ X() : v(0) {}
+ int f() {
+ v += 42;
+ return v;
+ }
+};
+
+int f(std::unique_ptr<X> p) {
+ return p->f();
+}
+
+template <typename T>
+T g(T v) {
+ return v + 42;
+}
+
+struct Y {
+ std::unique_ptr<X> p;
+ explicit Y(std::unique_ptr<X> p);
+};
+
+struct YBraceInit {
+ std::unique_ptr<X> p;
+ explicit YBraceInit(std::unique_ptr<X> p)
+ : p{std::move(p)} { }
+};
+
+Y::Y(std::unique_ptr<X> p)
+ : p(std::move(p)) { // expected-note 3{{here}}
+
+ p->f(); // expected-warning {{moved into member}}
+ f(std::move(p)); // expected-warning {{moved into member}}
+ p->v = 42; // expected-warning {{moved into member}}
+ g<decltype(p->f())>(decltype(p->v){});
+ decltype(p->f()) k = 0;
+ decltype(f(std::move(p))) j = 0;
+}
+
+void gain() {
+ std::unique_ptr<X> p(new X);
+ Y y(std::move(p));
+}
+}
+
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
