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.
This unbreaks C++20 buildbot that was broken since
402baea0a9.
With implicit conversion in C++20 compilation mode the spaceship
will unintentionally be based on `operator bool`:
```cpp
auto foo(Location L, Location R) {
return L <=> R;
// Equivalent to the following line due to implicit conversions.
// return L.operator bool() <=> R.operator bool();
}
```
The spaceship operator is rarely used explicitly, but its implicit uses
in the STL may cause surprising results, as exposed by the use of `std::tie`
in 402baea0a9, which ended up changing the
comparisons results unintentionally.
Parse friends into a new FriendInfo and serialize them in JSON. We keep track of the friend declaration's template and function information if applicable.
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.
In PR #128503, the CLI option would take precedence over the config option
only if it was set to `never`. This commit ensures the CLI option always takes
precedence over the config option.
ArrayRef has a constructor that accepts std::nullopt. This
constructor dates back to the days when we still had llvm::Optional.
Since the use of std::nullopt outside the context of std::optional is
kind of abuse and not intuitive to new comers, I would like to move
away from the constructor and eventually remove it.
This patch replaces std::nullopt with {}.
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>
Add checks to prevent rewriting when doing so might result in incorrect
code. The following cases are checked:
- There are multiple field declarations in one statement like `int a, b`
- Multiple fields are created from a single macro expansion
- Preprocessor directives are present in the struct
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`.
Add support for documenting concepts. This handles concepts and constraints on function and class templates.
Atomic constraints are not considered yet. We don't order constraints based on their conjunctive or disjunctive properties.
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
LLVM style prefers no default label on fully covered switches to warn if
new enums are added. This patch removes the default label for that
purpose or uses IT_default instead of default if that was the only enum
not covered.
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.
Adds a JSON generator backend to emit mapped information as JSON. This will enable a better testing format for upcoming changes. It can also potentially serve to feed our other backend generators in the future, like Mustache which already serializes information to JSON before emitting as HTML.
This patch contains functionality to emit classes and provides most of the basis of the generator.
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();
}
```
