Makes sure UnconventionalAssignOperatorCheck checks if the types
reference the same entity, not the exact declaration.
This adds a new matcher to support this check.
This fixes a regression introduced by #147835. Since this regression was
never released, there are no release notes.
Fixes#153770
Before this change, `-quiet` mode in clang-tidy generated meaningless
messages `xxx warnings generated` in output:
```cpp
// main.cpp
#include <iostream>
int main() {
std::cout << 42;
}
```
```console
> clang-tidy -checks='-*,readability-magic-numbers' -quiet main.cpp
82 warnings generated.
main.cpp:4:16: warning: 42 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
4 | std::cout << 42;
| ^
```
As you can see, `82 warnings generated.` does not say much `quiet` mode,
this patch removes this message completely:
```console
> ./build/bin/clang-tidy -p build -checks='-*,readability-magic-numbers' -quiet main.cpp
main.cpp:4:16: warning: 42 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
4 | std::cout << 42;
| ^
```
In contrast, when running without `quiet`, It gives some meaningful
information because we know how many messages were suppressed thus
calculating total messages count:
```console
> clang-tidy -checks='-*,readability-magic-numbers' main.cpp
82 warnings generated.
main.cpp:4:16: warning: 42 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
4 | std::cout << 42;
| ^
Suppressed 81 warnings (81 in non-user code).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
```
Fixes#47042
```cpp
struct Base {
int m;
};
template <class T>
struct Derived : Base {
Derived() { m = 0; }
};
```
would previously generate the following output:
```
<source>:7:15: warning: 'm' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
7 | Derived() { m = 0; }
| ^~~~~~
| : m(0)
```
This patch fixes this false positive.
Note that before this patch the checker won't give false positive for
```cpp
struct Derived : Base {
Derived() { m = 0; }
};
```
and the constructor's AST is
```
`-CXXConstructorDecl 0x557df03d1fb0 <line:7:3, col:22> col:3 Derived 'void ()' implicit-inline
|-CXXCtorInitializer 'Base'
| `-CXXConstructExpr 0x557df03d2748 <col:3> 'Base' 'void () noexcept'
`-CompoundStmt 0x557df03d2898 <col:13, col:22>
`-BinaryOperator 0x557df03d2878 <col:15, col:19> 'int' lvalue '='
|-MemberExpr 0x557df03d2828 <col:15> 'int' lvalue ->m 0x557df03d1c40
| `-ImplicitCastExpr 0x557df03d2808 <col:15> 'Base *' <UncheckedDerivedToBase (Base)>
| `-CXXThisExpr 0x557df03d27f8 <col:15> 'Derived *' implicit this
`-IntegerLiteral 0x557df03d2858 <col:19> 'int' 0
```
so `isAssignmentToMemberOf` would return empty due to
f0967fca04/clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp (L118-L119)Fixes#104400
This commit is a re-do of e4a8969e56572371201863594b3a549de2e23f32,
which got reverted, with the same goal: dramatically speed-up clang-tidy
by avoiding doing work in system headers (which is wasteful as warnings
are later discarded). This proposal was already discussed here with
favorable feedback: https://github.com/llvm/llvm-project/pull/132725
The novelty of this patch is:
- It's less aggressive: it does not fiddle with AST traversal. This
solves the issue with the previous patch, which impacted the ability to
inspect parents of a given node.
- Instead, what we optimize for is exitting early in each `Traverse*`
function of `MatchASTVisitor` if the node is in a system header, thus
avoiding calling the `match()` function with its corresponding callback
(when there is a match).
- It does not cause any failing tests.
- It does not move `MatchFinderOptions` - instead we add a user-defined
default constructor which solves the same problem.
- It introduces a function `shouldSkipNode` which can be extended for
adding more conditions. For example there's a PR open about skipping
modules in clang-tidy where this could come handy:
https://github.com/llvm/llvm-project/pull/145630
As a benchmark, I ran clang-tidy with all checks activated, on a single
.cpp file which #includes all the standard C++ headers, then measure the
time as well as found warnings.
On trunk:
```
Suppressed 75413 warnings (75413 in non-user code).
real 0m12.418s
user 0m12.270s
sys 0m0.129s
```
With this patch:
```
Suppressed 11448 warnings (11448 in non-user code).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
real 0m1.666s
user 0m1.538s
sys 0m0.129s
```
With the original patch that got reverted:
```
Suppressed 11428 warnings (11428 in non-user code).
real 0m1.193s
user 0m1.096s
sys 0m0.096s
```
We therefore get a dramatic reduction in number of warnings and runtime,
with no change in functionality.
The remaining warnings are due to `PPCallbacks` - implementing a similar
system-header exclusion mechanism there can lead to almost no warnings
left in system headers. This does not bring the runtime down as much,
though, so it's probably not worth the effort.
Fixes#52959
Co-authored-by: Carlos Gálvez <carlos.galvez@zenseact.com>
These are implicit vardecls which its type was never written in source
code. Don't create a TypeLoc and give it a fake source location.
The fake as-written type also didn't match the actual type, which after
fixing this gives some unrelated test churn on a CFG dump, since
statement printing prefers type source info if thats available.
Fixes https://github.com/llvm/llvm-project/issues/153649
This is a regression introduced in
https://github.com/llvm/llvm-project/pull/147835
This regression was never released, so no release notes are added.
Correctly generating fix hints when size method is called from implicit
this.
Closes#152387.
---------
Co-authored-by: Baranov Victor <bar.victor.2002@gmail.com>
Co-authored-by: EugeneZelenko <eugene.zelenko@gmail.com>
This is a major change on how we represent nested name qualifications in
the AST.
* The nested name specifier itself and how it's stored is changed. The
prefixes for types are handled within the type hierarchy, which makes
canonicalization for them super cheap, no memory allocation required.
Also translating a type into nested name specifier form becomes a no-op.
An identifier is stored as a DependentNameType. The nested name
specifier gains a lightweight handle class, to be used instead of
passing around pointers, which is similar to what is implemented for
TemplateName. There is still one free bit available, and this handle can
be used within a PointerUnion and PointerIntPair, which should keep
bit-packing aficionados happy.
* The ElaboratedType node is removed, all type nodes in which it could
previously apply to can now store the elaborated keyword and name
qualifier, tail allocating when present.
* TagTypes can now point to the exact declaration found when producing
these, as opposed to the previous situation of there only existing one
TagType per entity. This increases the amount of type sugar retained,
and can have several applications, for example in tracking module
ownership, and other tools which care about source file origins, such as
IWYU. These TagTypes are lazily allocated, in order to limit the
increase in AST size.
This patch offers a great performance benefit.
It greatly improves compilation time for
[stdexec](https://github.com/NVIDIA/stdexec). For one datapoint, for
`test_on2.cpp` in that project, which is the slowest compiling test,
this patch improves `-c` compilation time by about 7.2%, with the
`-fsyntax-only` improvement being at ~12%.
This has great results on compile-time-tracker as well:
This patch also further enables other optimziations in the future, and
will reduce the performance impact of template specialization resugaring
when that lands.
It has some other miscelaneous drive-by fixes.
About the review: Yes the patch is huge, sorry about that. Part of the
reason is that I started by the nested name specifier part, before the
ElaboratedType part, but that had a huge performance downside, as
ElaboratedType is a big performance hog. I didn't have the steam to go
back and change the patch after the fact.
There is also a lot of internal API changes, and it made sense to remove
ElaboratedType in one go, versus removing it from one type at a time, as
that would present much more churn to the users. Also, the nested name
specifier having a different API avoids missing changes related to how
prefixes work now, which could make existing code compile but not work.
How to review: The important changes are all in
`clang/include/clang/AST` and `clang/lib/AST`, with also important
changes in `clang/lib/Sema/TreeTransform.h`.
The rest and bulk of the changes are mostly consequences of the changes
in API.
PS: TagType::getDecl is renamed to `getOriginalDecl` in this patch, just
for easier to rebasing. I plan to rename it back after this lands.
Fixes#136624
Fixes https://github.com/llvm/llvm-project/issues/43179
Fixes https://github.com/llvm/llvm-project/issues/68670
Fixes https://github.com/llvm/llvm-project/issues/92757
Useful when the check warns on template functions to know which type
it's complaining about. Otherwise, since the instantiation stack is not
printed, it's very hard to tell.
Co-authored-by: Carlos Gálvez <carlos.galvez@zenseact.com>
This patch implements a fix for the false-positive case described in
[issue #134840](https://github.com/llvm/llvm-project/issues/134840) for
the
[bugprone-tagged-union-member-count](https://clang.llvm.org/extra/clang-tidy/checks/bugprone/tagged-union-member-count.html)
clang-tidy check.
The example given in the linked issue was the following:
```C
#include <pthread.h>
typedef enum {
MYENUM_ONE,
MYENUM_TWO,
} myEnumT;
typedef struct {
pthread_mutex_t mtx;
myEnumT my_enum;
} myTypeT;
```
The
[bugprone-tagged-union-member-count](https://clang.llvm.org/extra/clang-tidy/checks/bugprone/tagged-union-member-count.html)
check emits the following a warning for this struct:
```
<source>:8:9: warning: tagged union has more data members (3) than tags (2)! [bugprone-tagged-union-member-count]
8 | typedef struct {
| ^
1 warning generated.
```
The issue is that `pthread_mutex_t` can be a union behind a typedef like
the following:
```C
typedef union
{
struct __pthread_mutex_s __data;
char __size[__SIZEOF_PTHREAD_MUTEX_T];
long int __align;
} pthread_mutex_t;
```
From the checker's point of view therefore `myTypeT` contains a data
member with an enum type and another data member with a union type and
starts analyzing it like a user-defined tagged union.
The proposed solution is that the types from system headers and the std
namespace are no longer candidates to be the enum part or the union part
of a user-defined tagged union. This filtering for enums and the std
namespace may not be strictly necessary in this example, however I added
it preemptively out of (perhaps unnecessary) caution.
Fixes https://github.com/llvm/llvm-project/issues/134840.
Let's consider the following code from the issue #139467:
```c
void test(int cond, char c) {
char ret = cond > 0 ? ':' : c;
}
```
Initializer of `ret` looks the following:
```
-ImplicitCastExpr 'char' <IntegralCast>
`-ConditionalOperator 'int'
|-BinaryOperator 'int' '>'
| |-ImplicitCastExpr 'int' <LValueToRValue>
| | `-DeclRefExpr 'int' lvalue ParmVar 'cond' 'int'
| `-IntegerLiteral 'int' 0
|-CharacterLiteral 'int' 58
`-ImplicitCastExpr 'int' <IntegralCast>
`-ImplicitCastExpr 'char' <LValueToRValue>
`-DeclRefExpr 'char' lvalue ParmVar 'c' 'char'
```
So it could be seen that `RHS` of the conditional operator is
`DeclRefExpr 'c'` which is casted to `int` and then the whole
conditional expression is casted to 'char'. But this last conversion is
not narrowing, because `RHS` was `char` _initially_. We should just
remove the cast from `char` to `int` before the narrowing conversion
check.
Fixes#139467
Add new option `enable-check-profiling` to `run-clang-tidy` for seamless
integration of `clang-tidy`'s `enable-check-profiling` option.
`run-clang-tidy` will post aggregated results report in the same style
as `clang-tidy`.
This PR will help users to benchmark their `clang-tidy` runs easily.
Also, `clang-tidy` developers could build benchmark infrastructure in
the future.
%T has been deprecated for about seven years since it is not unique to
each test and can thus lead to races. This patch removes uses of %T from
clang-tools-extra with the eventual goal of removing support for %T from
lit.
Changed the warning message:
- **From**: 'Attempt to free released memory'
**To**: 'Attempt to release already released memory'
- **From**: 'Attempt to free non-owned memory'
**To**: 'Attempt to release non-owned memory'
- **From**: 'Use of memory after it is freed'
**To**: 'Use of memory after it is released'
All connected tests and their expectations have been changed
accordingly.
Inspired by [this
PR](https://github.com/llvm/llvm-project/pull/147542#discussion_r2195197922)
`readability-qualified-auto` check currently looks at the unsugared
type, skipping any typedefs, to determine if the variable is a
pointer-type. This may not be the desired behaviour, in particular when
the type depends on compilation flags.
For example
```
#if CONDITION
using Handler = int *;
#else
using Handler = uint64_t;
#endif
```
A more common example is some implementations of `std::array` use
pointers as iterators.
This introduces the IgnoreAliasing option so that
`readability-qualified-auto` does not look beyond typedefs.
---------
Co-authored-by: juanbesa <juanbesa@devvm33299.lla0.facebook.com>
Co-authored-by: Kazu Hirata <kazu@google.com>
Co-authored-by: EugeneZelenko <eugene.zelenko@gmail.com>
Co-authored-by: Baranov Victor <bar.victor.2002@gmail.com>
Serializes parameter comments for all descriptions.
We do not support Doxygen's parameter checking, which warns if a documented parameter is not actually present.
Text comments were unnecessarily nested inside Paragraph comments as a
Children array. If they're at the top level, we can also avoid more
nesting in templates.
The Mustache basic project has comments in its headers but the comments were not
serialized. Now we serialize \brief and paragraph comments for classes
and add that output to the basic project test.
Comment categories will allow better comment organization in HTML.
Before, comments would just be serialized in whatever order they were
written, so groups like params or notes wouldn't be in the same
sections.
Change the default value of `-j` from `1` to `0` in `clang-tidy-diff.py`
script to autodetect number of CPU cores to run on.
Script `run-clang-tidy.py` already had this behavior by default.
Both scripts now also print the number of threads being used to
provide better visibility into their execution behavior.
Fixes https://github.com/llvm/llvm-project/issues/148624.
These checks previously failed with absl::StrFormat and absl::PrintF
etc. with:
Unable to use 'std::format' instead of 'StrFormat' because first
argument is not a narrow string literal [modernize-use-std-format]
because FormatStringConverter was rejecting the format string if it had
already converted into a different type. Fix the tests so that they
check this case properly by accepting string_view rather than const char
* and fix the check so that these tests pass. Update the existing tests
that checked for the error message that can no longer happen.
Fixes: https://github.com/llvm/llvm-project/issues/129484
Upstream is moving towards new create method invocation, add check to flag old
usage that will be deprecated.
---------
Co-authored-by: Baranov Victor <bar.victor.2002@gmail.com>
Co-authored-by: EugeneZelenko <eugene.zelenko@gmail.com>
Ignore false positives on C23 enums which allows setting the fixed
underlying type to signed char.
The AST tree for C enums includes a ImplicitCastExp
(that was matched) but this is not the case for C++ enums.
Fixes#145651
---------
Signed-off-by: Björn Svensson <bjorn.a.svensson@est.tech>
This patch integrates JSON as the source to generate HTML Mustache templates. The Mustache generator calls the JSON generator and reads JSON files on the disk to produce HTML serially.
This patch contains changes for the JSON generator that will enable compatibility with Mustache templates, like booleans to check for the existence and bounds of arrays to avoid duplication.
Ignore pure virtual member functions in
portability-template-virtual-member-function check.
Those functions will be represented in vtable
as __cxa_pure_virtual or something similar.
Fixes#139031.
This change enhances the `bugprone-unhandled-self-assignment` checker by
adding an additional matcher that generalizes the copy-and-swap idiom
pattern detection.
# What Changed
Added a new matcher that checks for:
- An instance of the current class being created in operator=
(regardless of constructor arguments)
- That instance being passed to a `swap` function call
# Problem Solved
This fix reduces false positives in PMR-like scenarios where "extended"
constructors are used (typically taking an additional allocator
argument). The checker now properly recognizes copy-and-swap
implementations that use extended copy/move constructors instead of
flagging them as unhandled self-assignment cases.
Fixes#146324
---------
Co-authored-by: Baranov Victor <bar.victor.2002@gmail.com>
Before this patch, this check only handles `VarDecl` as varaibles
declaration in statement, but this will ignore variables in structured
bindings (`BindingDecl` in AST), which leads to false positives.
Closes#138842.
Allow AST matches in clang-query to have a trailing comma at the end of
matcher arguments. Makes it nicer to work with queries that span
multiple lines.
So, for example, the following is possible:
```clang-query
match namedDecl(
isExpansionInMainFile(),
anyOf(
varDecl().bind("var"),
functionDecl().bind("func"),
# enumDecl().bind("enum"),
),
)
```
This check already understands how `constexpr` makes initialization
order problems impossible, and C++20's `constinit` provides the exact
same guarantees.
This PR add stacktrace of escaped exception to
`bugprone-exception-escape` check.
Changes:
1. Modified `ExceptionAnalyzer` and `ExceptionInfo` classes to hold
stacktrace of escaped exception in `llvm::MapVector`. `llvm::MapVector`
is needed to hold relative positions of functions in stack as well as
have fast lookup.
2. Added new diagnostics based of `misc-no-recursion` check.
Fixes https://github.com/llvm/llvm-project/issues/87422.
Added function to filter out `CheckOptions` that come from
`ClangTidyOptions::getDefaults()`, but does not have a corresponding
check enabled in the `Checks` configuration.
Fixes https://github.com/llvm/llvm-project/issues/146693.
Closes#132561.
This is a check that rewrites `#if`s and `#elif`s like so:
```cpp
#if defined(MEOW) // -> #ifdef MEOW
#if !defined(MEOW) // -> #ifndef MEOW
```
And, since C23 and C++23:
```cpp
#elif defined(MEOW) // -> #elifdef MEOW
#elif !defined(MEOW) // -> #elifndef MEOW
```
When having this code:
```cpp
namespace {
class MyClassOutOfAnon {
public:
MyClassOutOfAnon();
} // namespace
MyClassOutOfAnon::MyClassOutOfAnon() {}
```
`MyClassOutOfAnon::MyClassOutOfAnon` is located in anonymous namespace
in `DeclContext` but outside anonymous namespace in
`LexicalDeclContext`.
For this check to work correctly, we need to check if definition is
located inside `LexicalDeclContext`.
Revert llvm/llvm-project#143520 for now since it’s causing issues for
people who are using symlinks and prefer to preserve the original path
(i.e. looks like we’ll have to make this configurable after all; I just
need to figure out how to pass `-no-canonical-prefixes` down through the
driver); I’m planning to refactor this a bit and reland it in a few
days.
Finds function and variable declarations inside anonymous namespace and
suggests replacing them with ``static`` declarations.
The check will enforce that
[restrict-visibility](https://llvm.org/docs/CodingStandards.html#restrict-visibility)
rule in LLVM Coding Standards is followed correctly (by adding `static`
to functions instead of putting them in anonimous namespace).
The check has additional levels of "strictness" represented by Options.
By default, the check works in the most relaxed way by giving warning
only for methods and functions defined in anonymous namespaces. Also, It
finds `static` functions that are placed inside anonymous namespace -
there is no point in keeping them inside.
This patch changes JSON file serialization. Now, files are serialized
to a single directory instead of nesting them based on namespaces. The
global namespace retains the "index.json" name.
This solves the problem of class template specializations being serialized to the
same file as its base template. This is also planned as part of
future integration with the Mustache generator which will consume the JSON files.
Add InsertPlainNamesInForwardDecls option to readability-named-parameter
check to insert parameter names without comments for forward
declarations only.
When enabled, forward declarations get plain parameter names (e.g., `int
param`) while function definitions continue to use commented names
(e.g., `int /*param*/`). Named parameters in forward decls don't cause
compiler warnings and some developers prefer to have names without
comments but in sync between declarations and the definition.
Default behavior remains unchanged
(InsertPlainNamesInForwardDecls=false).
Example with InsertPlainNamesInForwardDecls=true:
```cpp
// Forward declaration - gets plain name because the definition has name.
void func(int param);
void func(int param) { ... = param; }
```
This patch switches over uses of use_clang to clang_setup to fix a
potential race condition that has been impacting CI.
This is split from the refactoring to ensure I'm not missing anything
major here on the clang-tools-extra side.
This should fix#145703.
Reviewers: AaronBallman, HighCommander4, HerrCai0907, petrhosek, Keenuts
Reviewed By: petrhosek
Pull Request: https://github.com/llvm/llvm-project/pull/147437
