Run misc-use-internal-linkage check over clang-tidy code.
Also fixed a couple of other clang-tidy warnings.
Apart from issues in header files, all '.cpp' in
`clang-tools-extra/clang-tidy` must be clang-tidy clear now.
before this change, we use an output parameter so
`UseAfterMoveFinder::find()` can return the found `UseAfterMove`, and
addition to it, `UseAfterMoveFinder::find()` return a bool, so we can
tell if a use-after-free is identified. this arrangement could be
confusing when one needs to understand when the each member variable of
the returned `UseAfterMove` instance is initialized.
in this change, we return an `std::optional<UseAfterMove>` instead of a
bool, so that it's more obvious on when/where the returned
`UseAfterMove` is initialized.
this change is a cleanup. so it does not change the behavior of this
check.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
This eliminates false positives in bugprone-use-after-move where a
variable
is used in the callee and moved from in the arguments.
We introduce one special case: If the callee is a MemberExpr with a
DeclRefExpr as its base, we consider it to be sequenced after the
arguments. This is because the variable referenced in the base will only
actually be accessed when the call happens, i.e. once all of the
arguments have been evaluated. This has no basis in the C++ standard,
but it reflects actual behavior that is relevant to a use-after-move
scenario:
```c++
a.bar(consumeA(std::move(a));
```
In this example, we end up accessing a after it has been moved from,
even though nominally the callee a.bar is evaluated before the argument
consumeA(std::move(a)).
Treating this scenario correctly has required rewriting the logic in
bugprone-use-after-move that governs whether the use happens in a later
loop iteration than the move. This was previously based on an unsound
heuristic (does the use come lexically before the move?); we now use a
more rigourous criterion based on reachability in the CFG.
Fixes#57758Fixes#59612
Co-authored-by: martinboehme <mboehme@google.com>
Ignore unevaluated expressions in rvalue-reference-param-not-moved
check since they are not actual uses of a move().
Reviewed By: PiotrZSL
Differential Revision: https://reviews.llvm.org/D146929
llvm::sort is beneficial even when we use the iterator-based overload,
since it can optionally shuffle the elements (to detect
non-determinism). However llvm::sort is not usable everywhere, for
example, in compiler-rt.
Reviewed By: nhaehnle
Differential Revision: https://reviews.llvm.org/D130406
Previously, we were treating a move in the lambda capture as if it happened
within the body of the lambda, not within the function that defines the lambda.
This fixes the same bug as https://reviews.llvm.org/D119165 (which it appears
may have been abandoned by the author?) but does so more simply.
Reviewed By: njames93
Differential Revision: https://reviews.llvm.org/D126780
Fixes PR#38187. Constructors are actually already checked,
but only as functions, i.e. the check only looks at the
constructor body and not at the initializers, which misses
the (common) case where constructor parameters are moved
as part of an initializer expression.
One remaining false negative is when both the move //and//
the use-after-move occur in constructor initializers.
This is a lot more difficult to handle, though, because
the `bugprone-use-after-move` check is currently based on
a CFG that only takes the body into account, not the
initializers, so e.g. initialization order would have to
manually be considered. I will file a follow-up issue for
this once PR#38187 is closed.
Reviewed By: carlosgalvezp
Differential Revision: https://reviews.llvm.org/D113708
We have no way to reason about the bool returned by try_emplace, so we
simply ignore any std::move()s that happen in a try_emplace argument.
A lot of the time in this situation, the code will be checking the
bool and doing something else if it turns out the value wasn't moved
into the map, and this has been causing false positives so far.
I don't currently have any intentions of handling "maybe move" functions
more generally.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D98034
Use mapAnyOf() and matchers based on it.
Use of binaryOperation() means that modernize-loop-convert and
readability-container-size-empty can now be used with rewritten binary
operators.
Differential Revision: https://reviews.llvm.org/D94131
Now that we've moved to C++14, we no longer need the llvm::make_unique
implementation from STLExtras.h. This patch is a mechanical replacement
of (hopefully) all the llvm::make_unique instances across the monorepo.
Differential revision: https://reviews.llvm.org/D66259
llvm-svn: 368944
to reflect the new license.
We understand that people may be surprised that we're moving the header
entirely to discuss the new license. We checked this carefully with the
Foundation's lawyer and we believe this is the correct approach.
Essentially, all code in the project is now made available by the LLVM
project under our new license, so you will see that the license headers
include that license only. Some of our contributors have contributed
code under our old license, and accordingly, we have retained a copy of
our old license notice in the top-level files in each project and
repository.
llvm-svn: 351636
Summary:
Previously, we weren't recognizing these as smart pointers and thus
weren't allowing non-dereference accesses as we should -- see new test
cases which fail without the fix.
Reviewers: alexfh, hokein, aaron.ballman, JonasToth
Reviewed By: JonasToth
Subscribers: xazax.hun, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D56585
llvm-svn: 351303
Removed the uses of the allOf() matcher inside node matchers that are implicit
allOf(). Replaced uses of allOf() with the explicit node matcher where it makes
matchers more readable. Replace anyOf(hasName(), hasName(), ...) with the more
efficient and readable hasAnyName().
llvm-svn: 347520
Summary:
Before this fix, the bugprone-use-after-move check could incorrectly
conclude that a use and move in a function template were not sequenced.
For details, see
https://bugs.llvm.org/show_bug.cgi?id=39149
Reviewers: alexfh, hokein, aaron.ballman, JonasToth
Reviewed By: aaron.ballman
Subscribers: xazax.hun, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D52782
llvm-svn: 343768
Summary:
This allows member functions to be marked as reinitializing the object. After a
moved-from object has been reinitialized, the check will no longer consider it
to be in an indeterminate state.
The patch that adds the attribute itself is at https://reviews.llvm.org/D49911
Reviewers: ilya-biryukov, aaron.ballman, alexfh, hokein, rsmith
Reviewed By: aaron.ballman
Subscribers: dblaikie, xazax.hun, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D49910
llvm-svn: 339571
