Previously, any macro that didn't look like a varargs macro
or a function style macro was reported with a warning that
it should be replaced with a constexpr const declaration.
This is only reasonable when the macro body contains constants
and not expansions like ",", "[[noreturn]]", "__declspec(xxx)",
etc.
So instead of always issuing a warning about every macro that
doesn't look like a varargs or function style macro, examine the
tokens in the macro and only warn about the macro if it contains
only comment and constant tokens.
Differential Revision: https://reviews.llvm.org/D116386Fixes#39945
The cppcoreguidelines-pro-bounds-array-to-pointer-decay check currently
accepts:
const char *b = i ? "foo" : "foobar";
but not
const char *a = i ? "foo" : "bar";
This is because the AST is slightly different in the latter case (see
https://godbolt.org/z/MkHVvs).
This eliminates the inconsistency by making it accept the latter form
as well.
Fixes https://github.com/llvm/llvm-project/issues/31155.
Currently, it's inconsistent that warnings are disabled if they
come from system headers, unless they come from macros.
Typically a user cannot act upon these warnings coming from
system macros, so clang-tidy should ignore them unless the
user specifically requests warnings from system headers
via the corresponding configuration.
This change broke the ProTypeVarargCheck check, because it
was checking for the usage of va_arg indirectly, expanding it
(it's a system macro) to detect the usage of __builtin_va_arg.
The check has been fixed by checking directly what the rule
is about: "do not use va_arg", by adding a PP callback that
checks if any macro with name "va_arg" is expanded. The old
AST matcher is still kept for compatibility with Windows.
Add unit test that ensures warnings from macros are disabled
when not using the -system-headers flag. Document the change
in the Release Notes.
Differential Revision: https://reviews.llvm.org/D116378
We want to deal with non-default constructors that just happen to
contain constant initializers. There was already a negative test case,
it is now a positive one. We find and refactor this case:
struct PositiveNotDefaultInt {
PositiveNotDefaultInt(int) : i(7) {}
int i;
};
This change adds an option to disable warnings from the
cppcoreguidelines-narrowing-conversions check on integer to floating-
point conversions which may be narrowing.
An example of a case where this might be useful:
```
std::vector<double> v = {1, 2, 3, 4};
double mean = std::accumulate(v.cbegin(), v.cend(), 0.0) / v.size();
```
The conversion from std::size_t to double is technically narrowing on
64-bit systems, but v almost certainly does not have enough elements
for this to be a problem.
This option would allow the cppcoreguidelines-narrowing-conversions
check to be enabled on codebases which might otherwise turn it off
because of cases like the above.
The purpose of this checker is to flag a missing throw keyword, and does so by checking for the construction of an exception class that is then unused.
This works great except that placement new expressions are also flagged as those lead to the construction of an object as well, even though they are not temporary (as that is dependent on the storage).
This patch fixes the issue by exempting the match if it is within a placement-new.
Fixes https://github.com/llvm/llvm-project/issues/51939
Differential Revision: https://reviews.llvm.org/D115576
Checks for various ways that the `const CharT*` constructor of `std::basic_string_view` can be passed a null argument and replaces them with the default constructor in most cases. For the comparison operators, braced initializer list does not compile so instead a call to `.empty()` or the empty string literal are used, where appropriate.
This prevents code from invoking behavior which is unconditionally undefined. The single-argument `const CharT*` constructor does not check for the null case before dereferencing its input. The standard is slated to add an explicitly-deleted overload to catch some of these cases: wg21.link/p2166
https://reviews.llvm.org/D114823 is a companion change to prevent duplicate warnings from the `bugprone-string-constructor` check.
Reviewed By: ymandel
Differential Revision: https://reviews.llvm.org/D113148
Fixes PR#47614. Deduction guides, implicit or user-defined, look like
function declarations in the AST. They aren't really functions, though,
and they always have a trailing return type, so it doesn't make sense
to issue this warning for them.
The google-readability-casting check is meant to be on par
with cpplint's readability/casting check, according to the
documentation. However it currently does not diagnose
functional casts, like:
float x = 1.5F;
int y = int(x);
This is detected by cpplint, however, and the guidelines
are clear that such a cast is only allowed when the type
is a class type (constructor call):
> You may use cast formats like `T(x)` only when `T` is a class type.
Therefore, update the clang-tidy check to check this
case.
Differential Revision: https://reviews.llvm.org/D114427
Detect when an identifier contains some Right-To-Left characters.
This pass relates to https://trojansource.codes/
Example of misleading source:
short int א = (short int)0;
short int ג = (short int)12345;
int main() {
int א = ג; // a local variable, set to zero?
printf("ג is %d\n", ג);
printf("א is %d\n", א);
}
This is a recommit of 299aa4dfa1 with missing
option registration fixed.
Differential Revision: https://reviews.llvm.org/D112914
This reverts commit 7f92a1a84b.
It triggers an assert, see http://45.33.8.238/linux/60293/step_9.txt
"AST/Decl.h:277: llvm::StringRef clang::NamedDecl::getName() const: Assertion `Name.isIdentifier() && "Name is not a simple identifier"' failed."
Suggests switching the initialization pattern of `absl::Cleanup` instances from the factory function to class template argument deduction (CTAD) in C++17 and higher.
Reviewed By: ymandel
Differential Revision: https://reviews.llvm.org/D113195
The CERT rule ERR33-C can be modeled partially by the existing check
'bugprone-unused-return-value'. The existing check is reused with
a fixed set of checked functions.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D112409
To simplify suppressing warnings (for example, for
when multiple check aliases are enabled).
The globbing format reuses the same code as for
globbing when enabling checks, so the semantics
and behavior is identical.
Differential Revision: https://reviews.llvm.org/D111208
This requirement was introduced in the C++ Core guidelines in 2016:
1894380d0a
Then clang-tidy got updated to comply with the rule.
However in 2019 this decision was reverted:
5fdfb20b76
Therefore we need to apply the correct configuration to
clang-tidy again.
This also makes this cppcoreguidelines check consistent
with the other 2 alias checks: hicpp-use-override and
modernize-use-override.
Additionally, add another RUN line to the unit test,
to make sure cppcoreguidelines-explicit-virtual-functions
is tested.
Add support for NOLINTBEGIN ... NOLINTEND comments to suppress
clang-tidy warnings over multiple lines. All lines between the "begin"
and "end" markers are suppressed.
Example:
// NOLINTBEGIN(some-check)
<Code with warnings to be suppressed, line 1>
<Code with warnings to be suppressed, line 2>
<Code with warnings to be suppressed, line 3>
// NOLINTEND(some-check)
Follows similar syntax as the NOLINT and NOLINTNEXTLINE comments
that are already implemented, i.e. allows multiple checks to be provided
in parentheses; suppresses all checks if the parentheses are omitted,
etc.
If the comments are misused, e.g. using a NOLINTBEGIN but not
terminating it with a NOLINTEND, a clang-tidy-nolint diagnostic
message pointing to the misuse is generated.
As part of implementing this feature, the following bugs were fixed in
existing code:
IsNOLINTFound(): IsNOLINTFound("NOLINT", Str) returns true when Str is
"NOLINTNEXTLINE". This is because the textual search finds NOLINT as
the stem of NOLINTNEXTLINE.
LineIsMarkedWithNOLINT(): NOLINTNEXTLINEs on the very first line of a
file are ignored. This is due to rsplit('\n\').second returning a blank
string when there are no more newline chars to split on.
This reverts commit 626586fc25.
Tweak the test for Windows. Windows defaults to delayed template
parsing, which resulted in the main template definition not registering
the test on Windows. Process the file with the additional
`-fno-delayed-template-parsing` flag to change the default beahviour.
Additionally, add an extra check for the fix it and use a more robust
test to ensure that the value is always evaluated.
Differential Revision: https://reviews.llvm.org/D108893
This reverts commit 76dc8ac36d.
Restore the change. The test had an incorrect negative from testing.
The test is expected to trigger a failure as mentioned in the review
comments. This corrects the test and should resolve the failure.
This introduces a new check, readability-containter-data-pointer. This
check is meant to catch the cases where the user may be trying to
materialize the data pointer by taking the address of the 0-th member of
a container. With C++11 or newer, the `data` member should be used for
this. This provides the following benefits:
- `.data()` is easier to read than `&[0]`
- it avoids an unnecessary re-materialization of the pointer
* this doesn't matter in the case of optimized code, but in the case
of unoptimized code, this will be visible
- it avoids a potential invalid memory de-reference caused by the
indexing when the container is empty (in debug mode, clang will
normally optimize away the re-materialization in optimized builds).
The small potential behavioural change raises the question of where the
check should belong. A reasoning of defense in depth applies here, and
this does an unchecked conversion, with the assumption that users can
use the static analyzer to catch cases where we can statically identify
an invalid memory de-reference. For the cases where the static analysis
is unable to prove the size of the container, UBSan can be used to track
the invalid access.
Special thanks to Aaron Ballmann for the discussion on whether this
check would be useful and where to place it.
This also partially resolves PR26817!
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D108893
Finds base classes and structs whose destructor is neither public and
virtual nor protected and non-virtual.
A base class's destructor should be specified in one of these ways to
prevent undefined behaviour.
Fixes are available for user-declared and implicit destructors that are
either public and non-virtual or protected and virtual.
This check implements C.35 [1] from the CppCoreGuidelines.
Reviewed By: aaron.ballman, njames93
Differential Revision: http://reviews.llvm.org/D102325
[1]: http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rc-dtor-virtual
Add a check for enforcing minimum length for variable names. A default
minimum length of three characters is applied to regular variables
(including function parameters). Loop counters and exception variables
have a minimum of two characters. Additionally, the 'i', 'j' and 'k'
are accepted as legacy values.
All three sizes, as well as the list of accepted legacy loop counter
names are configurable.
The patch in http://reviews.llvm.org/D106431 landed in commit
4a097efe77 to the main branch.
However, this patch was also backported to upcoming release 13.0.0 in
commit 8dcdfc0de84f60b5b4af97ac5b357881af55bc6e, which makes this entry
in the release notes **NOT** a new thing for the purposes of 14.0.0.
FixIt, and add support for initialization check of scoped enum
In C++, the enumeration is never Integer, and the enumeration condition judgment is added to avoid compiling errors when it is initialized to an integer.
Add support for initialization check of scope enum.
As the following case show, clang-tidy will give a wrong automatic fix:
enum Color {Red, Green, Blue};
enum class Gender {Male, Female};
void func() {
Color color; // Color color = 0; <--- fix bug
Gender gender; // <--- no warning
}
Reviewd By: aaron.ballman, whisperity
Differential Revision: http://reviews.llvm.org/D106431
Finds function calls where the call arguments might be provided in an
incorrect order, based on the comparison (via string metrics) of the
parameter names and the argument names against each other.
A diagnostic is emitted if an argument name is similar to a *different*
parameter than the one currently passed to, and it is sufficiently
dissimilar to the one it **is** passed to currently.
False-positive warnings from this check are useful to indicate bad
naming convention issues, even if a swap isn't necessary.
This check does not generate FixIts.
Originally implemented by @varjujan as his Master's Thesis work.
The check was subsequently taken over by @barancsuk who added type
conformity checks to silence false positive matches.
The work by @whisperity involved driving the check's review and fixing
some more bugs in the process.
Reviewed By: aaron.ballman, alexfh
Differential Revision: http://reviews.llvm.org/D20689
Co-authored-by: János Varjú <varjujanos2@gmail.com>
Co-authored-by: Lilla Barancsuk <barancsuklilla@gmail.com>
Finds function definitions where parameters of convertible types follow
each other directly, making call sites prone to calling the function
with swapped (or badly ordered) arguments.
Such constructs are usually the result of inefficient design and lack of
exploitation of strong type capabilities that are possible in the
language.
This check finds and flags **function definitions** and **not** call
sites!
Reviewed By: aaron.ballman, alexfh
Differential Revision: http://reviews.llvm.org/D69560
This lint check is a part of the FLOCL (FPGA Linters for OpenCL) project
out of the Synergy Lab at Virginia Tech.
FLOCL is a set of lint checks aimed at FPGA developers who write code
in OpenCL.
The altera ID dependent backward branch lint check finds ID dependent
variables and fields used within loops, and warns of their usage. Using
these variables in loops can lead to performance degradation.
Overflows are never fun.
In most cases (in most of the code), they are rare,
because usually you e.g. don't have as many elements.
However, it's exceptionally easy to fall into this pitfail
in code that deals with images, because, assuming 4-channel 32-bit FP data,
you need *just* ~269 megapixel image to case an overflow
when computing at least the total byte count.
In [[ https://github.com/darktable-org/darktable | darktable ]], there is a *long*, painful history of dealing with such bugs:
* https://github.com/darktable-org/darktable/pull/7740
* https://github.com/darktable-org/darktable/pull/7419
* eea1989f2c
* 70626dd95b
* https://github.com/darktable-org/darktable/pull/670
* 38c69fb1b2
and yet they clearly keep resurfacing still.
It would be immensely helpful to have a diagnostic for those patterns,
which is what this change proposes.
Currently, i only diagnose the most obvious case, where multiplication
is directly widened with no other expressions inbetween,
(i.e. `long r = (int)a * (int)b` but not even e.g. `long r = ((int)a * (int)b)`)
however that might be worth relaxing later.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D93822
Fixes bug http://bugs.llvm.org/show_bug.cgi?id=49000.
This patch allows Clang-Tidy checks to do
diag(X->getLocation(), "text") << Y->getSourceRange();
and get the highlight of `Y` as expected:
warning: text [blah-blah]
xxx(something)
^ ~~~~~~~~~
Reviewed-By: aaron.ballman, njames93
Differential Revision: http://reviews.llvm.org/D98635
This lint check is a part of the FLOCL (FPGA Linters for OpenCL)
project out of the Synergy Lab at Virginia Tech.
FLOCL is a set of lint checks aimed at FPGA developers who write code
in OpenCL.
The altera unroll loops check finds inner loops that have not been
unrolled, as well as fully-unrolled loops that should be partially
unrolled due to unknown loop bounds or a large number of loop
iterations.
Based on the Altera SDK for OpenCL: Best Practices Guide.
The deprecation notice was cherrypicked to the release branch in f8b3298924 so its safe to remove this for the 13.X release cycle.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D98612
... For removal in next release cycle.
The clang warning that does the same thing is enabled by default and typically emits better diagnostics making this check surplus to requirements.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D97491
Added an option to control whether to apply the fixes found in notes attached to clang tidy errors or not.
Diagnostics may contain multiple notes each offering different ways to fix the issue, for that reason the default behaviour should be to not look at fixes found in notes.
Instead offer up all the available fix-its in the output but don't try to apply the first one unless `-fix-notes` is supplied.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D84924
Adds an option, `PreferResetCall`, currently defaulted to `false`, to the check.
When `true` the check will refactor by calling the `reset` member function.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D97630
Following a discussion about the current state of this check on the 12.X branch, it was decided to purge the check as it wasn't in a fit to release state, see https://llvm.org/PR49318.
This check has since had some of those issues addressed and should be good for the next release cycle now, pending any more bug reports about it.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D97275
An option is added to the check to select wich set of functions is
defined as asynchronous-safe functions.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D90851
Sockke