This patch improves, but doens't fully resolve the layering violation,
which stems from relying on Sema. There's one function that needs to
convert enumerator to a string (`buildQualifier` in
`FixItHintUtils.cpp`), but `Qualifiers::TQ` doesn't offer such function.
Even more, the set of enumerators is not complete compared to
`DeclSpec::TQ`, so I'm afraid that this would be a functional change.
…and operators that have non-const overloads.
This allows `unnecessary-copy-initialization` to warn on more cases.
The common case is a class with a a set of const/non-sconst overloads
(e.g. std::vector::operator[]).
```
void F() {
std::vector<Expensive> v;
// ...
const Expensive e = v[i];
}
```
This uses a more systematic approach for determining whcich
`DeclRefExpr`s mutate the underlying object: Instead of using a few
matchers, we walk up the AST until we find a parent that we can prove
cannot change the underlying object.
This allows us to handle most address taking and dereference, bindings
to value and const& variables, and track constness of pointee (see
changes in DeclRefExprUtilsTest.cpp).
This allows supporting more patterns in
`performance-unnecessary-copy-initialization`.
Those two patterns are relatively common:
```
const auto e = (*vector_ptr)[i]
```
and
```
const auto e = vector_ptr->at(i);
```
In our codebase, we have around 25% additional findings from
`performance-unnecessary-copy-initialization` with this change. I did
not see any additional false positives.
This patch replaces uses of StringRef::{starts,ends}with with
StringRef::{starts,ends}_with for consistency with
std::{string,string_view}::{starts,ends}_with in C++20.
I'm planning to deprecate and eventually remove
StringRef::{starts,ends}with.
We received some user feedback around this being disruptful rather than
useful in certain workflows so add an option to control the output behaviour.
Differential Revision: https://reviews.llvm.org/D157390
Until now when determining all the const uses of a VarDecl we only considered
how the variable itself was used. This change extends checking for const usages
of the type's members as well.
This increases the number of true positives for various performance checks that
share the same const usage analysis.
Path by Felix Berger
Reviewed By: njames93, PiotrZSL
Differential Revision: https://reviews.llvm.org/D97567
A previous patch update the clang-tidy documentation
incorrectly claiming that SystemHeaders can be provided
in the .clang-tidy configuration file.
This patch adds support for it, together with tests.
Differential Revision: https://reviews.llvm.org/D149899
Re-introduce the patch that was reverted previously.
In the first attempt, the checks would not be able to
read from the global option, since getLocalOrGlobal
only works with string types. Additional logic is needed
in order to support both use cases in the transition
period. All that logic will be removed when the local
options are fully removed.
We have a number of checks designed to analyze problems
in header files only, for example:
bugprone-suspicious-include
google-build-namespaces
llvm-header-guard
misc-definitions-in-header
...
All these checks duplicate the same logic and options
to determine whether a location is placed in the main
source file or in the header. More checks are coming
up with similar requirements.
Thus, to remove duplication, let's move this option
to the top-level configuration of clang-tidy (since
it's something all checks should share).
Add a deprecation notice for all checks that use the
local option, prompting to update to the global option.
Differential Revision: https://reviews.llvm.org/D142655
This reverts commit 4240c9146248ac0a91c45dee421c6ef07709ba74.
The current solution won't work since getLocalOrGlobal does not
support returning a vector. More work needs to be put into
ensuring both the local and global way of setting the options
are available during the transition period.
We have a number of checks designed to analyze problems
in header files only, for example:
bugprone-suspicious-include
google-build-namespaces
llvm-header-guard
misc-definitions-in-header
...
All these checks duplicate the same logic and options
to determine whether a location is placed in the main
source file or in the header. More checks are coming
up with similar requirements.
Thus, to remove duplication, let's move this option
to the top-level configuration of clang-tidy (since
it's something all checks should share).
Since the checks fetch the option via getLocalOrGlobal,
the behavior is unchanged.
Add a deprecation notice for all checks that use the
local option, prompting to update to the global option.
The functionality for parsing the option will need to
remain in the checks during the transition period.
Once the local options are fully removed, the goal
is to store the parsed options in the ClangTidyContext,
that checks can easily have access to.
Differential Revision: https://reviews.llvm.org/D141000
The Annotations helper class does not have a gtest or gmock dependency, but because it's bundled with the rest of TestingSupport, it gets one. By splitting it out, a target can use it without being forced to use LLVM's copy of gtest.
Reviewed By: GMNGeoffrey, sammccall, gribozavr2
Differential Revision: https://reviews.llvm.org/D141175
std::optional::value() has undesired exception checking semantics and is
unavailable in older Xcode (see _LIBCPP_AVAILABILITY_BAD_OPTIONAL_ACCESS). The
call sites block std::optional migration.
This patch mechanically replaces None with std::nullopt where the
compiler would warn if None were deprecated. The intent is to reduce
the amount of manual work required in migrating from Optional to
std::optional.
This is part of an effort to migrate from llvm::Optional to
std::optional:
https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716
Right now we can only add a single warning, notes are not possible.
Apparently some provisions were made to allow notes, but they were never
propagated all the way to the diagnostics.
Differential Revision: https://reviews.llvm.org/D128807
Given that we provide an EditGenerator edit(ASTEdit), we can't ever be
sure that the user won't give us an empty replacement.
Differential Revision: https://reviews.llvm.org/D128887
D56303 added testing code that was then made redundant by the changes in D125026. However this code wasn't completely removed in the latter patch.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D130026
Without this fix we get
../../clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp:270:2: error: extra ';' outside of a function is incompatible with C++98 [-Werror,-Wc++98-compat-extra-semi]
};
^
1 error generated.
when compiling with -Werror.
C requires that enum values fit into an int. Scan the macro tokens
present in an initializing expression and reject macros that contain
tokens that have suffixes making them larger than int.
C forbids the comma operator in enum initializing expressions, so
optionally reject comma operator.
Differential Revision: https://reviews.llvm.org/D125622Fixes#55467
Reimplement the matching logic using Visitors instead of matchers.
Benchmarks from running the check over SemaCodeComplete.cpp
Before 0.20s, After 0.04s
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D125026
Clang erroneously flagged the function as "unused", but it is most
definitely used by gtest to pretty print the parameter value when
a test fails.
Make the pretty printing function a friend function in the parameter
class similar to other clang unit tests.
Add a recursive descent parser to match macro expansion tokens against
fully formed valid expressions of integral literals. Partial
expressions will not be matched -- they can't be valid initializing
expressions for an enum.
Differential Revision: https://reviews.llvm.org/D124500Fixes#55055
Currently it rejects "// FOO_BAR_H" as an endif comment due to the extra space.
A user complained that this is too picky, which seems fair enough.
Differential Revision: https://reviews.llvm.org/D124955
Messages generated by Transformer rules may have `%` in them, which
needs to be escaped before being passed to `diag`, which interprets them
specially (and crashes if they are misused).
Differential Revision: https://reviews.llvm.org/D124952
Adds a flag to `ClangTidyContext` that is used to indicate to checks that fixes will only be applied one at a time.
This is to indicate to checks that each fix emitted should not depend on any other fixes emitted across the translation unit.
I've currently implemented the `IncludeInserter`, `LoopConvertCheck` and `PreferMemberInitializerCheck` to use these support these modes.
Reasoning behind this is in use cases like `clangd` it's only possible to apply one fix at a time.
For include inserter checks, the include is only added once for the first diagnostic that requires it, this will result in subsequent fixes not having the included needed.
A similar issue is seen in the `PreferMemberInitializerCheck` where the `:` will only be added for the first member that needs fixing.
Fixes emitted in `StandaloneDiagsMode` will likely result in malformed code if they are applied all together, conversely fixes currently emitted may result in malformed code if they are applied one at a time.
For this reason invoking `clang-tidy` from the binary will always with `StandaloneDiagsMode` disabled, However using it as a library its possible to select the mode you wish to use, `clangd` always selects `StandaloneDiagsMode`.
This is an example of the current behaviour failing
```lang=c++
struct Foo {
int A, B;
Foo(int D, int E) {
A = D;
B = E; // Fix Here
}
};
```
Incorrectly transformed to:
```lang=c++
struct Foo {
int A, B;
Foo(int D, int E), B(E) {
A = D;
// Fix Here
}
};
```
In `StandaloneDiagsMode`, it gets transformed to:
```lang=c++
struct Foo {
int A, B;
Foo(int D, int E) : B(E) {
A = D;
// Fix Here
}
};
```
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D97121
Change RewriteRule from holding an `Explanation` to being able to generate
arbitrary metadata. Where TransformerClangTidyCheck was interested in a string
description for the diagnostic, other tools may be interested in richer metadata
at a higher level of abstraction than at the edit level (which is currently
available as ASTEdit::Metadata).
Reviewed By: ymandel
Differential Revision: https://reviews.llvm.org/D120360
Change RewriteRule from holding an `Explanation` to being able to generate
arbitrary metadata. Where TransformerClangTidyCheck was interested in a string
description for the diagnostic, other tools may be interested in richer metadata
at a higher level of abstraction than at the edit level (which is currently
available as ASTEdit::Metadata).
Reviewed By: ymandel
Differential Revision: https://reviews.llvm.org/D120360
