This makes IncludeInserter's behavior consistent with include-cleaner,
as discussed in https://github.com/llvm/llvm-project/pull/140594.
(cherry picked from commit ff0093cecd0c807d122cfb6b74634074c962ade9)
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`.
Move `RecordDecl::isInjectedClassName` to
`CXXRecordDecl::isInjectedClassName`. C language doesn't have the term
"injected class name".
Co-authored-by: Matheus Izvekov <mizvekov@gmail.com>
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.
C++17's CTAD obsoletes `makeMatcher` (and many `make*` functions like
it).
The deduction guide is written out explicitly to avoid
`-Wctad-maybe-unsupported` warnings.
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.
Added `.clang-tidy` config as discussed in
[RFC](https://discourse.llvm.org/t/rfc-create-hardened-clang-tidy-config-for-clang-tidy-directory/87247).
Added `bugprone`, `readability`, `modernize`, `performance` checks that
didn't create many warnings.
Fixed minor warnings to make `/clang-tidy` directory complaint with
`clang-tidy-20`.
Disabled checks will be enabled in future PRs after fixing their
warnings.
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; }
```
Coming from internal using experience.
The original log is just confusing... Users can't understand it unless
he reads the code.
This patch tries to make this more understandable.
Note that the mentioned module files in the logs might be removed
intentionally. We need another option to allow users to remain these
module files. Let's done this in another patch.
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
```cpp
for (std::string OptionName : {
#define GET_CHECKER_OPTIONS
#define CHECKER_OPTION(TYPE, CHECKER, OPTION_NAME, DESCRIPTION, DEFAULT, \
RELEASE, HIDDEN) \
Twine(AnalyzerCheckNamePrefix).concat(CHECKER ":" OPTION_NAME).str(),
#include "clang/StaticAnalyzer/Checkers/Checkers.inc"
#undef CHECKER_OPTION
#undef GET_CHECKER_OPTIONS
}) {
Result.Options.insert(OptionName);
}
```
This code is doing a lot of unnecessary work at runtime. For each of the
(currently) 59 checker options, it runs
`Twine(AnalyzerCheckNamePrefix).concat(CHECKER ":" OPTION_NAME).str(),`,
which allocates a string (all of this is unrolled, leading to code
bloat). Then it copies all those strings (because `std::string
OptionName`, not `const std::string& OptionName`). All of this can be
done at compile time!
Some of these are even global mutable state — probably not what was
intended!
```cpp
static const char *AnalyzerCheckNamePrefix = "clang-analyzer-";
```
This fixes an issue compiling LLVM 20.1.7 on Ubuntu 22.04 with the
Ubuntu provided Clang 14 and C++20. That's probably happening due to
incomplete C++20 support in either Clang 14 or the GNU libstdc++ 12.
The actual error is:
```
[1/8] Building CXX object tools/clang/tools/extra/clang-tidy/bugprone/CMakeFiles/obj.clangTidyBugproneModule.dir/TaggedUnionMemberCountCheck.cpp.o
FAILED: tools/clang/tools/extra/clang-tidy/bugprone/CMakeFiles/obj.clangTidyBugproneModule.dir/TaggedUnionMemberCountCheck.cpp.o
/usr/lib/llvm-14/bin/clang++ -DGTEST_HAS_RTTI=0 -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/work/_build/tools/clang/tools/extra/clang-tidy/bugprone -I/work/llvm-project/clang-tools-extra/clang-tidy/bugprone -I/work/_build/tools/clang/tools/extra/clang-tidy -I/work/llvm-project/clang/include -I/work/_build/tools/clang/include -I/work/_build/include -I/work/llvm-project/llvm/include -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual -Wno-nested-anon-types -O3 -DNDEBUG -std=c++20 -fno-exceptions -funwind-tables -fno-rtti -MD -MT tools/clang/tools/extra/clang-tidy/bugprone/CMakeFiles/obj.clangTidyBugproneModule.dir/TaggedUnionMemberCountCheck.cpp.o -MF tools/clang/tools/extra/clang-tidy/bugprone/CMakeFiles/obj.clangTidyBugproneModule.dir/TaggedUnionMemberCountCheck.cpp.o.d -o tools/clang/tools/extra/clang-tidy/bugprone/CMakeFiles/obj.clangTidyBugproneModule.dir/TaggedUnionMemberCountCheck.cpp.o -c /work/llvm-project/clang-tools-extra/clang-tidy/bugprone/TaggedUnionMemberCountCheck.cpp
/work/llvm-project/clang-tools-extra/clang-tidy/bugprone/TaggedUnionMemberCountCheck.cpp:148:39: error: use of overloaded operator '==' is ambiguous (with operand types 'llvm::APSInt' and 'unsigned long')
(LastEnumConstant->getInitVal() == (EnumValues.size() - 1))) {
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~
/work/llvm-project/llvm/include/llvm/ADT/APInt.h:2080:13: note: candidate function (with reversed parameter order)
inline bool operator==(uint64_t V1, const APInt &V2) { return V2 == V1; }
^
/work/llvm-project/llvm/include/llvm/ADT/APSInt.h:188:8: note: candidate function
bool operator==(int64_t RHS) const {
^
/work/llvm-project/llvm/include/llvm/ADT/APSInt.h:357:13: note: candidate function (with reversed parameter order)
inline bool operator==(int64_t V1, const APSInt &V2) { return V2 == V1; }
^
1 error generated.
```
I know that clang-14 in combination with the GNU libstdc++ 12 might not
be 100% C++20 compliant. But this is the only error we see when
compiling LLVM 20.1.7 and it looks very fixable
Thanks,
Gregor
---------
Signed-off-by: Gregor Jasny <gjasny@googlemail.com>
This can significantly shorten file paths to standard library headers,
e.g. on my system, `<ranges>` is currently printed as
```console
/usr/lib/gcc/x86_64-redhat-linux/15/../../../../include/c++/15/ranges
```
but with this change, we instead print
```console
/usr/include/c++/15/ranges
```
This is of course just a heuristic; there are paths that would get longer
as a result of this, so we use whichever path ends up being shorter.
@AaronBallman pointed out that this might be problematic for network
file systems since path resolution might take a while, so this is enabled
only for paths that are part of a local filesystem—though not on Windows
since there we noticed that the check itself is slow.
The file names are cached in `SourceManager`.
The static analyzer we use internally complains about potential
dereference of `nullptr` for `Found`. I think both `Found` and `Member`
can't be null here (please confirm). I have added assertions.
Fix false positives when copy assignment operator function in a template
class returns the result of another assignment to `*this`, this check
doesn't consider this situation that there will be a `BinaryOperator`
for assignment rather than `CXXOperatorCallExpr` since `this`'s type is
dependent.
Closes#143237.
Fixed false negatives with type aliases in
`cppcoreguidlines-pro-bounds-pointer-arithmetic` check.
Added tests with pointer arithmetic in template functions to make test
cases more robust.
Closes https://github.com/llvm/llvm-project/issues/139241.
The comment for `DiagnosticConsumer::BeginSourceFile()` states that
"diagnostics with source range information are required to only be
emitted in between BeginSourceFile() and EndSourceFile().". While
working on some upcoming changes to the static analyzer, we hit some
crashes when diagnostics were reported from the `EndOfMainFile` callback
in the preprocessor. This turned out to be because
`FrontEndAction::EndSourceFile()` notifies the diagnostic clients of the
end of the source file before it notifies the preprocessor. Thus, the
diagnostics from the preprocessor callback are reported when the
diagnostic client is no longer expecting any diagnostics.
The fix is to swap the order of the `EndSourceFile()` calls so that the
preprocessor is notified first.
I've added asserts to the `ClangTidyDiagnosticConsumer` to catch
unexpected diagnostics outside of a source file. Before swapping the
order of the calls as described above, this causes several failures in
the clang-tidy regression tests. With the swap, there are no failures in
`check-all`.
rdar://141230583
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
402baea0a9ff7894565449e41f700c4e6a3f99cb.
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 402baea0a9ff7894565449e41f700c4e6a3f99cb, 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.
