This revision was automatically updated to reflect the committed changes.
Closed by commit rL281453: [clang-tidy] Add check 'misc-use-after-move'
(authored by mboehme).
Changed prior to commit:
https://reviews.llvm.org/D23353?vs=71312&id=71313#toc
Repository:
rL LLVM
https://reviews.llvm.or
mboehme marked an inline comment as done.
mboehme added a comment.
https://reviews.llvm.org/D23353
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
mboehme updated this revision to Diff 71312.
mboehme marked 6 inline comments as done.
mboehme added a comment.
Herald added subscribers: mgorny, beanz.
Responses to reviewer comments.
https://reviews.llvm.org/D23353
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
LG
Comment at: clang-tidy/misc/UseAfterMoveCheck.cpp:30
@@ +29,3 @@
+/// a `CFGBlock`.
+///
+/// While a `CFGBlock` does contain individual `CFGElement`s for some
Prazek added inline comments.
Comment at: clang-tidy/misc/UseAfterMoveCheck.cpp:191
@@ +190,3 @@
+
+if (const Stmt *S = Node.get()) {
+ Result.push_back(S);
Dry: const auto *
https://reviews.llvm.org/D23353
__
mboehme marked 9 inline comments as done.
mboehme added a comment.
> > > 2. Also it would be good to make link in cppcoreguidelines.
>
> >
>
> >
>
> > How exactly would I create such a "link"? Are you just thinking of a link
> > in the documentation, or is there a way to have one clang
mboehme updated this revision to Diff 69206.
mboehme added a comment.
Responses to reviewer comments.
https://reviews.llvm.org/D23353
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
clang-tidy/misc/UseAfterMoveCheck.cpp
clang-tidy/misc/UseAfterMoveCheck.h
docs
alexfh requested changes to this revision.
This revision now requires changes to proceed.
Comment at: clang-tidy/misc/UseAfterMoveCheck.cpp:29
@@ +28,3 @@
+/// Provides information about the evaluation order of (sub-)expressions within
+/// a CFGBlock.
+///
Please
Prazek added a comment.
In https://reviews.llvm.org/D23353#516314, @mboehme wrote:
> In https://reviews.llvm.org/D23353#511362, @Prazek wrote:
>
> > I will review it later, but my first thoughts:
> >
> > 1. I think we should make some other group, because misc seems to be
> > overloaded. I discu
mboehme marked 2 inline comments as done.
Comment at: clang-tidy/misc/UseAfterMoveCheck.cpp:659
@@ +658,3 @@
+ UseAfterMove Use;
+ if (finder.find(FunctionBody, MovingCall, MovedVariable, &Use)) {
+emitDiagnostic(MovingCall, MovedVariable, Use, this, Result.Context);
---
mboehme updated this revision to Diff 68366.
mboehme added a comment.
Remove braces around another single-line if statement block
https://reviews.llvm.org/D23353
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
clang-tidy/misc/UseAfterMoveCheck.cpp
clang-tidy/mis
mboehme updated this revision to Diff 68365.
mboehme added a comment.
Remove braces around single-line bodies of if statements
https://reviews.llvm.org/D23353
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
clang-tidy/misc/UseAfterMoveCheck.cpp
clang-tidy/misc/U
alexfh added inline comments.
Comment at: clang-tidy/misc/UseAfterMoveCheck.cpp:659
@@ +658,3 @@
+ UseAfterMove Use;
+ if (finder.find(FunctionBody, MovingCall, MovedVariable, &Use)) {
+emitDiagnostic(MovingCall, MovedVariable, Use, this, Result.Context);
om
omtcyfz added a subscriber: omtcyfz.
Comment at: clang-tidy/misc/UseAfterMoveCheck.cpp:659
@@ +658,3 @@
+ UseAfterMove Use;
+ if (finder.find(FunctionBody, MovingCall, MovedVariable, &Use)) {
+emitDiagnostic(MovingCall, MovedVariable, Use, this, Result.Context);
mboehme marked 4 inline comments as done.
mboehme added a comment.
In https://reviews.llvm.org/D23353#511362, @Prazek wrote:
> I will review it later, but my first thoughts:
>
> 1. I think we should make some other group, because misc seems to be
> overloaded. I discussed it with Alex months ago
mboehme marked 9 inline comments as done.
Comment at: clang-tidy/misc/UseAfterMoveCheck.cpp:493
@@ +492,3 @@
+if (!S)
+ continue;
+
For some reason, I thought I had read that they weren't compatible with CFG /
CFGBlock -- but obviously I must have been i
mboehme updated this revision to Diff 68147.
mboehme added a comment.
Responses to reviewer comments.
https://reviews.llvm.org/D23353
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
clang-tidy/misc/UseAfterMoveCheck.cpp
clang-tidy/misc/UseAfterMoveCheck.h
docs
alexfh added a comment.
A few initial comments.
Comment at: clang-tidy/misc/UseAfterMoveCheck.cpp:492
@@ +491,3 @@
+ DeclRefs->clear();
+ for (CFGBlock::const_iterator I = Block->begin(), E = Block->end(); I != E;
+ ++I) {
Any reason to avoid range-based
Eugene.Zelenko added inline comments.
Comment at: clang-tidy/misc/UseAfterMoveCheck.cpp:512
@@ +511,3 @@
+}
+void UseAfterMoveFinder::getReinits(
+const CFGBlock *Block, const ValueDecl *MovedVariable,
Please insert empty line before.
Repository:
rL LLVM
Eugene.Zelenko added a subscriber: Eugene.Zelenko.
Comment at: docs/ReleaseNotes.rst:75
@@ +74,3 @@
+
+ This check warns if an object is used after it has been moved, without an
+ intervening reinitialization.
Please remove //This check// and capitalize //warns/
Prazek added a subscriber: Prazek.
Prazek added a reviewer: Prazek.
Prazek added a comment.
I will review it later, but my first thoughts:
1. I think we should make some other group, because misc seems to be
overloaded. I discussed it with Alex months ago - something like bugprone would
be good
fowles added a subscriber: fowles.
Comment at: test/clang-tidy/misc-use-after-move.cpp:158
@@ +157,3 @@
+std::move(ptr);
+ptr.get();
+ }
would this warn on:
std::unique_ptr ptr;
std::move(ptr);
ptr->Foo();
I would like it to since that is a likely segfa
mboehme added a comment.
Apologies for the size of this patch. alexfh@ said he was willing to review it
as-is -- however, I'm happy to resubmit in smaller pieces if necessary.
https://reviews.llvm.org/D23353
___
cfe-commits mailing list
cfe-commits
mboehme created this revision.
mboehme added a reviewer: alexfh.
mboehme added a subscriber: cfe-commits.
The check warns if an object is used after it has been moved, without an
intervening reinitialization.
See user-facing documentation for details.
https://reviews.llvm.org/D23353
Files:
cl
24 matches
Mail list logo