[PATCH] Warn, if parameter is used in ctor body after being used for move-construct

David Blaikie dblaikie at gmail.com
Thu Jun 25 10:26:23 PDT 2015


A few thoughts - but probably having some review by rtreiu would be good. He's more involved in the diagnostic development than I am.

Have you run this over any substantial existing codebase to see what the stats are like (true/false positives, etc)?

The other issue is that, of course, we'd like a much more general version of this warning but it's unclear how much work that'll be and what the right tradeoff is for compile time. Some common analysis might be shared between a bunch of such warnings - we already have "sometimes-uninitialized" which is a CFG diagnostic that could be useful (treat moved-from as the uninitialized state and do the same analysis).


================
Comment at: lib/Sema/SemaDecl.cpp:10694
@@ +10693,3 @@
+    FieldInitMovedFromParamVisitor V(FD->getASTContext(), FD->getName());
+    V.Visit(E);
+    if (ParmVarDecl *P = V.getParam())
----------------
I'm not sure how much of an efficiency concern there is doing a generic RAV here, rather than something more targeted. I mean I suppose in general the std::move could happen in any subexpression of an init - but I assume the case you're trying to catch here is just the one where the name of the parameter and variable are the same (or very similar - like foo_ and foo, etc) and the user accidentally uses the parameter instead of the variable they moved-in-to?

================
Comment at: lib/Sema/SemaDecl.cpp:10783
@@ -10674,1 +10782,3 @@
         DiagnoseUnusedParameters(FD->param_begin(), FD->param_end());
+        if (getLangOpts().CPlusPlus11)
+          DiagnoseUseOfMovedParams(*this, FD);
----------------
I forget, is CPlusPlus11 true in modes greater than 11? (so we still get the warning in 14, etc?)

================
Comment at: test/SemaCXX/rval-references.cpp:95
@@ -94,1 +94,3 @@
 }
+
+namespace std {
----------------
This test case seems a bit long/verbose - you probably don't need to reproduce a substantial amount of this template machinery to reproduce the warning (a trivial movable type rather than a copy of a substantial amount of unique_ptr would suffice, for example - and there's no need for the extra type traits (remove_ref, etc) work in the faux-move implementation, etc)

http://reviews.llvm.org/D10425

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the cfe-commits mailing list