Szelethus added a comment.
I think we should either remove the non-default functionality (which wouldn't
be ideal), or emphasise somewhere (open projects?) that there is still work to
be done, but leaving it to be forgotten and essentially making it an extra
maintenance work would be, in my optinion, the worst case scenario.
`Aggressive` isn't `Pedantic` because it actually emits warnings on correct
code, and it's not a simple matter of too many reports being emitted, let's
also document that this is an experimental feature, not a power-user-only thing.
Whenever I compile `Rtags`, I get a bunch of move related warnings, I'm curious
how this patch behaves on that project. I'll take a look.
================
Comment at: lib/StaticAnalyzer/Checkers/MoveChecker.cpp:92-95
+ bool IsAggressive = false;
+
+public:
+ void setAggressiveness(bool Aggressive) { IsAggressive = Aggressive; }
----------------
How about a simple public data member? Since all callbacks are const, it
wouldn't be modified after registration anyways.
================
Comment at: lib/StaticAnalyzer/Checkers/MoveChecker.cpp:134
+static const MemRegion *unwrapRValueReferenceIndirection(const MemRegion *MR) {
+ if (const auto *SR = dyn_cast_or_null<SymbolicRegion>(MR)) {
+ SymbolRef Sym = SR->getSymbol();
----------------
You use `dyn_cast_or_null`, which to me implies that the returned value may be
`nullptr`, but you never check it on the call sites.
================
Comment at: lib/StaticAnalyzer/Checkers/MoveChecker.cpp:273-282
+ // In non-aggressive mode, only warn on use-after-move of local variables (or
+ // local rvalue references) and of STL objects. The former is possible
because
+ // local variables (or local rvalue references) are not tempting their user
to
+ // re-use the storage. The latter is possible because STL objects are known
+ // to end up in a valid but unspecified state after the move and their
+ // state-reset methods are also known, which allows us to predict
+ // precisely when use-after-move is invalid.
----------------
I've seen checker option descriptions in registry functions, in-class where
they are declared, and on top of the file, I think any of them would be more
ideal. Since my checker option related efforts are still far in the distance
(which will put all of them in `Checkers.td`), let's move (or at least copy)
this somewhere else.
================
Comment at: test/Analysis/use-after-move.cpp:47-52
+template <typename T>
+class vector {
+public:
+ vector();
+ void push_back(const T &t);
+};
----------------
Any particular reason for not using `cxx-system-header-simulator.h`?
Repository:
rC Clang
https://reviews.llvm.org/D54557
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
