this PR fixes#126424
for `ArraySubScriptExpr`, `hasBase` Matcher will get right operand when
it is not integer type, but is not for sure that left operand is integer
type. For the example code below `hasBase` will get `r` for the
Subsequent matching and causing false positive.
```
template <typename R>
int f(std::map<R*, int>& map, R* r) {
return map[r];
}
```
so is needed to see if index is integer type to avoid this situation.
Optimizations:
- Only build the skeleton for each identifier once, rather than once for
each declaration of that identifier.
- Only compute the contexts in which identifiers are declared for
identifiers that have the same skeleton as another identifier in the
translation unit.
- Only compare pairs of declarations that are declared in related
contexts, rather than comparing all pairs of declarations with the same
skeleton.
Also simplify by removing the caching of enclosing `DeclContext` sets,
because with the above changes we don't even compute the enclosing
`DeclContext` sets in common cases. Instead, we terminate the traversal
to enclosing `DeclContext`s immediately if we've already found another
declaration in that context with the same identifier. (This optimization
is not currently applied to the `forallBases` traversal, but could be
applied there too if needed.)
This also fixes two bugs that together caused the check to fail to find
some of the issues it was looking for:
- The old check skipped comparisons of declarations from different
contexts unless both declarations were type template parameters. This
caused the checker to not warn on some instances of the CVE it is
intended to detect.
- The old check skipped comparisons of declarations in all base classes
other than the first one found by the traversal. This appears to be an
oversight, incorrectly returning `false` rather than `true` from the
`forallBases` callback, which terminates traversal.
This also fixes an issue where the check would have false positives for
template parameters and function parameters in some cases, because those
parameters sometimes have a parent `DeclContext` that is the parent of
the parameterized entity, or sometimes is the translation unit. In
either case, this would cause warnings about declarations that are never
visible together in any scope.
This decreases the runtime of this check, especially in the common case
where there are few or no skeletons with two or more different
identifiers. Running this check over LLVM, clang, and clang-tidy, the
wall time for the check as reported by clang-tidy's internal profiler is
reduced from 5202.86s to 3900.90s.
Since std::move is nothing more than a cast, part of STL and not the
language itself, it's easy to provide a custom implementation if one
wishes not to include the entirety of <utility>.
Added flag (MoveFunction) provides a way to continue using this
essential check even with the custom implementation of moving.
---------
Co-authored-by: EugeneZelenko <eugene.zelenko@gmail.com>
Since std::forward is nothing more than a cast, part of STL and not the
language itself, it's easy to provide a custom implementation if one
wishes not to include the entirety of <utility>.
Added flag (ForwardFunction) provides a way to continue using this
essential check even with the custom implementation of forwarding.
---------
Co-authored-by: EugeneZelenko <eugene.zelenko@gmail.com>
Add new clang-tidy check that finds uses of `std::lock_guard` and suggests
replacing them with C++17's more flexible and safer alternative
`std::scoped_lock`.
Here is a small description of how it works for better understanding of
the code:
Two separate AST matchers are registered:
- The first one matches declarations of `std::lock_guard` that are
single in their scope (only one `std::lock_guard` in `CompoundStmt`).
It's an easy case, we can emit warning right away.
- The second one matches `CompoundStmt`'s that have multiple
`std::lock_guard` declarations, which means that we may have consecutive
declarations of `std::lock_guard` that can be replaced by a single
`std::scoped_lock`. In order to ensure that declarations are
consecutive, we need to loop over `Stmt`'s in `CompoundStmt`. Here is a
small example:
```cpp
{
std::mutex m1, m2;
std::lock(m1, m2);
std::lock_guard<std::mutex> l1(m, std::adopt_lock); // first declaration of 'std::lock_guard'
std::lock_guard<std::mutex> l2(m, std::adopt_lock); // second declaration of 'std::lock_guard' that can be merged with first using 'scoped_lock'
}
```
This PR closes https://github.com/llvm/llvm-project/issues/107839.
This patch fixes:
clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp:371:26:
error: variable 'E' set but not used
[-Werror,-Wunused-but-set-variable]
The sizeof operator misuses in loop conditionals can be a source of
bugs. The common misuse is attempting to retrieve the number of elements
in the array by using the sizeof expression and forgetting to divide the
value by the sizeof the array elements. This results in an incorrect
computation of the array length and requires a warning from the sizeof
checker.
Example:
```
int array[20];
void test_for_loop() {
// Needs warning.
for(int i = 0; i < sizeof(array); i++) {
array[i] = i;
}
}
void test_while_loop() {
int count = 0;
// Needs warning.
while(count < sizeof(array)) {
array[count] = 0;
count = count + 2;
}
}
```
rdar://151403083
---------
Co-authored-by: MalavikaSamak <malavika2@apple.com>
The adds a check that replaces specific numeric literals like `32767`
with the equivalent call to `std::numeric_limits` (such as
`std::numeric_limits<int16_t>::max())`.
Partially addresses #34434, but notably does not handle cases listed in
the title post such as `~0` and `-1`.
Summary:
Replacing by-value parameters with passing by-reference is not safe for
coroutines because the caller may be executed in parallel with the
callee, which increases the chances of resulting in dangling references
and hard-to-find crashes. See for the reference
[cppcoreguidelines-avoid-reference-coroutine-parameters](https://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/avoid-reference-coroutine-parameters.html).
Test Plan: check-clang-tools
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.
Aims to fix#127471
Covered the edge case where an int expression is not necessarily
directly wrapped around an 'ImplicitCastExpr' which seemed to be a
requirement in 'use-integer-sign-comparison.cpp' check to trigger.
**For instance**:
```cpp
#include <vector>
bool f() {
std::vector<int> v;
unsigned int i = 0;
return i >= v.size();
}
```
New option AllowNoNamespaceComments for
`google-readability-namespace-comments.AllowNoNamespaceComments` is
added.
When true, the check will allow that no namespace comment is present. If
a namespace comment is added but it is not matching, the check will
fail. Default is `false`
Fixes#124264
Add check for `DeclRefExpr` which points to an explicit object
parameter.
Fixes#141381.
---------
Co-authored-by: fubowen <fubowen@protomail.com>
Co-authored-by: flovent <flbven@protomail.com>
Sometimes a project may want to enable this check only in C++, and
disable it in C, since the patterns the check warns about are quite
common and idiomatic in C, and there are no better alternatives.
Fixes#140659
Co-authored-by: Carlos Gálvez <carlos.galvez@zenseact.com>
Add support for lambda-expression in `use-trailing-return-type` check.
Added two new options:
1. `TransformFunctions` will trigger function declarations to use
trailing return type.
2. `TransformLambdas` will trigger lambda expression to use trailing
return type if it was not stated explicitly.
Fixed false positives when lambda was matched as a function in C++11
mode.
Closes https://github.com/llvm/llvm-project/issues/95711
We have multiple different attributes in clang representing device
kernels for specific targets/languages. Refactor them into one attribute
with different spellings to make it more easily scalable for new
languages/targets.
---------
Signed-off-by: Sarnie, Nick <nick.sarnie@intel.com>
This matcher would never match anything, because all record types
as-written would be wrapped in an ElaboratedType.
Just fixing that leads to a matcher which has too many false positives
to be useful.
The warning message itself is not great either, it has a hard-coded type
name.
Currently we generate an incorrect suggestion for shared/unique pointers
to arrays; for instance ([Godbolt](https://godbolt.org/z/Tens1reGP)):
```c++
#include <memory>
void test_shared_ptr_to_array() {
std::shared_ptr<int[]> i;
auto s = sizeof(*i.get());
}
```
```
<source>:5:20: warning: redundant get() call on smart pointer [readability-redundant-smartptr-get]
5 | auto s = sizeof(*i.get());
| ^~~~~~~
| i
1 warning generated.
```
`sizeof(*i)` is incorrect, though, because the array specialization of
`std::shared/unique_ptr` does not have an `operator*()`. Therefore I
have disabled this check for smart pointers to arrays for now; future
work could, of course, improve on this by suggesting, say,
`sizeof(i[0])` in the above example.
Fix false positives in `bugprone-crtp-constructor-accessibility` check
on deleted constructors that cannot be used to construct objects, even
if they have public or protected access.
Closes https://github.com/llvm/llvm-project/issues/131737.
These are identified by misc-include-cleaner. I've filtered out those
that break builds. Also, I'm staying away from llvm-config.h,
config.h, and Compiler.h, which likely cause platform- or
compiler-specific build failures.
This aims to address a portion of #122480 by adding matchers on binary
operators. **This allows the detection of explicit arithmetic operations
within initializers.**
- Deleted unused includes
- Deleted useless braces
- Converted private methods to static function to improve compilations
speed and readability
- Modernized tests to use `cxx-or-later`
std::string::rfind accepts anything that can be converted to
std::string_view starting in C++17. Since StringRef can be converted
to std::string_view, we do not need to create a temporary instance of
std::string here.
The `DiagnosticOptions` class is currently intrusively
reference-counted, which makes reasoning about its lifetime very
difficult in some cases. For example, `CompilerInvocation` owns the
`DiagnosticOptions` instance (wrapped in `llvm::IntrusiveRefCntPtr`) and
only exposes an accessor returning `DiagnosticOptions &`. One would
think this gives `CompilerInvocation` exclusive ownership of the object,
but that's not the case:
```c++
void shareOwnership(CompilerInvocation &CI) {
llvm::IntrusiveRefCntPtr<DiagnosticOptions> CoOwner = &CI.getDiagnosticOptions();
// ...
}
```
This is a perfectly valid pattern that is being actually used in the
codebase.
I would like to ensure the ownership of `DiagnosticOptions` by
`CompilerInvocation` is guaranteed to be exclusive. This can be
leveraged for a copy-on-write optimization later on. This PR changes
usages of `DiagnosticOptions` across `clang`, `clang-tools-extra` and
`lldb` to not be intrusively reference-counted.
- Deleted unused includes
- Deleted useless braces
- Modernized tests to use `CHECK-MESSAGES-NOT` and `CHECK-FIXES-NOT` for
better readability and maintainability
The docs of the check state:
> Glibc’s list is compiled from GNU web documentation with a search for
MT-Safe tag
And strerror fulfills exactly that:
https://www.gnu.org/software/libc/manual/html_node/Error-Messages.html
> Function: char * strerror (int errnum)
> Preliminary: | MT-Safe | AS-Unsafe heap i18n | AC-Unsafe mem | See
POSIX Safety Concepts.
So concurrency-mt-unsafe should not flag it.
Fixes#140515
---------
Co-authored-by: Carlos Gálvez <carlos.galvez@zenseact.com>
Co-authored-by: EugeneZelenko <eugene.zelenko@gmail.com>
