[PATCH] D54557: [analyzer] MoveChecker Pt.2: Restrict the warning to STL objects and locals.

2018-12-03 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC348210: [analyzer] MoveChecker: Restrict to locals and std:: objects. (authored by dergachev, committed by ). Changed prior to commit: https://reviews.llvm.org/D54557?vs=174475=176491#toc Repository:

[PATCH] D54557: [analyzer] MoveChecker Pt.2: Restrict the warning to STL objects and locals.

2018-11-29 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus added a comment. In D54557#1313382 , @NoQ wrote: > In D54557#1301896 , @Szelethus wrote: > > > Wasn't the point of this patch to turn off part of this checkers > >

[PATCH] D54557: [analyzer] MoveChecker Pt.2: Restrict the warning to STL objects and locals.

2018-11-29 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. In D54557#1301896 , @Szelethus wrote: > Wasn't the point of this patch to turn off part of this checkers > functionality because it's immature just yet? From what I understand it is > desired, but the FP rate is a little too high. I

[PATCH] D54557: [analyzer] MoveChecker Pt.2: Restrict the warning to STL objects and locals.

2018-11-17 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. In https://reviews.llvm.org/D54557#1300654, @NoQ wrote: > In https://reviews.llvm.org/D54557#1299736, @xazax.hun wrote: > > > It would be great to have a way to extend the list of (possibly non-stl) > > types to check. But I do understand that the analyzer does not

[PATCH] D54557: [analyzer] MoveChecker Pt.2: Restrict the warning to STL objects and locals.

2018-11-16 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. In https://reviews.llvm.org/D54557#1301869, @NoQ wrote: > In https://reviews.llvm.org/D54557#1300671, @Szelethus wrote: > > > Nice catch, would you like to make an issue on their project or shall I? > > > I guess it can be an outright pull request :) I'll see if i have

[PATCH] D54557: [analyzer] MoveChecker Pt.2: Restrict the warning to STL objects and locals.

2018-11-16 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. In https://reviews.llvm.org/D54557#1300671, @Szelethus wrote: > Nice catch, would you like to make an issue on their project or shall I? I guess it can be an outright pull request :) I'll see if i have time for that soon, please feel free to take initiative>< In

[PATCH] D54557: [analyzer] MoveChecker Pt.2: Restrict the warning to STL objects and locals.

2018-11-16 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ updated this revision to Diff 174475. NoQ added a comment. Give the rvalue reference test function a sensible name. https://reviews.llvm.org/D54557 Files: lib/StaticAnalyzer/Checkers/MoveChecker.cpp test/Analysis/use-after-move.cpp Index: test/Analysis/use-after-move.cpp

[PATCH] D54557: [analyzer] MoveChecker Pt.2: Restrict the warning to STL objects and locals.

2018-11-16 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. > In https://reviews.llvm.org/D54557#1300653, @NoQ wrote: > >> In https://reviews.llvm.org/D54557#1299899, @Szelethus wrote: >> >> > I think we should either remove the non-default functionality (which >> > wouldn't be ideal), or emphasise somewhere (open projects?)

[PATCH] D54557: [analyzer] MoveChecker Pt.2: Restrict the warning to STL objects and locals.

2018-11-15 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. In https://reviews.llvm.org/D54557#1300581, @NoQ wrote: > In https://reviews.llvm.org/D54557#1299899, @Szelethus wrote: > > > 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.

[PATCH] D54557: [analyzer] MoveChecker Pt.2: Restrict the warning to STL objects and locals.

2018-11-15 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. In https://reviews.llvm.org/D54557#1299736, @xazax.hun wrote: > It would be great to have a way to extend the list of (possibly non-stl) > types to check. But I do understand that the analyzer does not have a great > way to set such configuration options right now. Do

[PATCH] D54557: [analyzer] MoveChecker Pt.2: Restrict the warning to STL objects and locals.

2018-11-15 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. In https://reviews.llvm.org/D54557#1299899, @Szelethus wrote: > 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

[PATCH] D54557: [analyzer] MoveChecker Pt.2: Restrict the warning to STL objects and locals.

2018-11-15 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. In https://reviews.llvm.org/D54557#1299899, @Szelethus wrote: > 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. Whoops, sry, i accidentally had a look because i misread your

[PATCH] D54557: [analyzer] MoveChecker Pt.2: Restrict the warning to STL objects and locals.

2018-11-15 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus added a comment. Alpha checkers, at least to developers, are clearly stating "Please finish me!", but a non-alpha, enabled-by-default checker with a flag that you'd never know about unless you looked at the source code (at least up until I fix these) sounds like it'll be forgotten

[PATCH] D54557: [analyzer] MoveChecker Pt.2: Restrict the warning to STL objects and locals.

2018-11-15 Thread Umann Kristóf via Phabricator via cfe-commits
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

[PATCH] D54557: [analyzer] MoveChecker Pt.2: Restrict the warning to STL objects and locals.

2018-11-15 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. It would be great to have a way to extend the list of (possibly non-stl) types to check. But I do understand that the analyzer does not have a great way to set such configuration

[PATCH] D54557: [analyzer] MoveChecker Pt.2: Restrict the warning to STL objects and locals.

2018-11-14 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. So, like, with locals it should be possible to easily and intuitively suppress the false positive, even if there is one, by simply using an extra variable, and additionally there's the `[[clang::reinitializes]]` attribute added by @xazax.hun that will help people suppress

[PATCH] D54557: [analyzer] MoveChecker Pt.2: Restrict the warning to STL objects and locals.

2018-11-14 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet, rnkovacs, Szelethus. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho, baloghadamsoftware. This patch implements the post important part of the plan proposed in