Commit Graph

2538 Commits

Author SHA1 Message Date
Balázs Kéri cc7ed0caac [clang-tidy] bugprone-signal-handler: Message improvement and code refactoring.
Another change of the code design.
Code simplified again, now there is a single place to check
a handler function and less functions for bug report emitting.
More details are added to the bug report messages.

Reviewed By: whisperity

Differential Revision: https://reviews.llvm.org/D118370
2022-04-07 09:38:58 +02:00
Aaron Siddhartha Mondal 04b42c99f6 Fix typo in new -config-file option
The new -config-file option introduced by 9e1f4f1 was accidentally
referenced as args.config_path on the python side. This patch renames
args.config_path to args.config_file.

To avoid confusion with python file objects, the input argument for
get_tidy_invocation has been renamed from config_path to
config_file_path.

See GitHub issue #54728 for a discussion.
2022-04-05 16:28:49 -04:00
Richard f547fc89c0 [clang-tidy] Add modernize-macro-to-enum check
[buildbot issues fixed]

This check performs basic analysis of macros and replaces them
with an anonymous unscoped enum.  Using an unscoped anonymous enum
ensures that everywhere the macro token was used previously, the
enumerator name may be safely used.

Potential macros for replacement must meet the following constraints:
- Macros must expand only to integral literal tokens.  The unary
  operators plus, minus and tilde are recognized to allow for positive,
  negative and bitwise negated integers.
- Macros must be defined on sequential source file lines, or with
  only comment lines in between macro definitions.
- Macros must all be defined in the same source file.
- Macros must not be defined within a conditional compilation block.
- Macros must not be defined adjacent to other preprocessor directives.
- Macros must not be used in preprocessor conditions

Each cluster of macros meeting the above constraints is presumed to
be a set of values suitable for replacement by an anonymous enum.
From there, a developer can give the anonymous enum a name and
continue refactoring to a scoped enum if desired.  Comments on the
same line as a macro definition or between subsequent macro definitions
are preserved in the output.  No formatting is assumed in the provided
replacements.

The check cppcoreguidelines-macro-to-enum is an alias for this check.

Fixes #27408

Differential Revision: https://reviews.llvm.org/D117522
2022-04-01 15:24:21 -06:00
Danny Mösch ff60af91ac [clang-tidy] Utilize comparison operation implemented in APInt
This is a fix for #53963.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D122544
2022-03-28 23:32:58 +02:00
Douglas Yung cef52105bd Revert "[clang-tidy] Add modernize-macro-to-enum check"
This reverts commit 39b80c8380.

This change was causing build failures on several build bots:
- https://lab.llvm.org/buildbot/#/builders/139/builds/19210
- https://lab.llvm.org/buildbot/#/builders/93/builds/7956
2022-03-25 11:53:42 -07:00
Richard 39b80c8380 [clang-tidy] Add modernize-macro-to-enum check
This check performs basic analysis of macros and replaces them
with an anonymous unscoped enum.  Using an unscoped anonymous enum
ensures that everywhere the macro token was used previously, the
enumerator name may be safely used.

Potential macros for replacement must meet the following constraints:
- Macros must expand only to integral literal tokens.  The unary
  operators plus, minus and tilde are recognized to allow for positive,
  negative and bitwise negated integers.
- Macros must be defined on sequential source file lines, or with
  only comment lines in between macro definitions.
- Macros must all be defined in the same source file.
- Macros must not be defined within a conditional compilation block.
- Macros must not be defined adjacent to other preprocessor directives.
- Macros must not be used in preprocessor conditions

Each cluster of macros meeting the above constraints is presumed to
be a set of values suitable for replacement by an anonymous enum.
From there, a developer can give the anonymous enum a name and
continue refactoring to a scoped enum if desired.  Comments on the
same line as a macro definition or between subsequent macro definitions
are preserved in the output.  No formatting is assumed in the provided
replacements.

The check cppcoreguidelines-macro-to-enum is an alias for this check.

Fixes #27408

Differential Revision: https://reviews.llvm.org/D117522
2022-03-25 09:45:55 -06:00
Fabian Wolff 07675b0b38 [clang-tidy] Fix false positives in `misc-redundant-expression` check
Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D122111
2022-03-23 00:32:45 +01:00
Eric Li 9edeceaece [libTooling] Generalize string explanation as templated metadata
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
2022-03-21 20:39:35 +00:00
Yitzhak Mandelbaum 8351726e6d Revert "[libTooling] Generalize string explanation as templated metadata"
This reverts commit 18440547d3. Causing failures
in some build modes.

e.g. https://lab.llvm.org/buildbot/#/builders/217/builds/1886
2022-03-21 19:06:59 +00:00
Eric Li 18440547d3 [libTooling] Generalize string explanation as templated metadata
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
2022-03-21 18:45:39 +00:00
Evgeny Shulgin fc354d3752 [clang-tidy] Skip parentheses in `readability-make-member-function-const`
The checker should ignore parentheses when looking whether the
function should be marked as `const`.

Fixes https://github.com/llvm/llvm-project/issues/52838

Reviewed By: mgehre-amd, njames93

Differential Revision: https://reviews.llvm.org/D122075
2022-03-21 14:18:17 +03:00
Shreyas Atre 9e1f4f1398 [clang-tidy][run-clang-tidy.py] Add --config-file=<string> option
Link to the GitHub Issue: https://github.com/llvm/llvm-project/issues/53745

Added config_path variable within the python script which makes the
required call to the clang-tidy binary with --config-file option.
If the config_path is None then config will be used. No error is raised
if both are given but silently chooses config_path over config
2022-03-17 07:30:28 -04:00
Evgeny Shulgin 6043520c20 [clang-tidy] Don't check decltype return types in `readability-const-return-type`
The checker removes `const`s that are superfluos and badly affect
readability. `decltype(auto)`/`decltype(expr)` are often const-qualified, but
have no effect on readability and usually can't stop being const-qualified
without significant code change.

Fixes https://github.com/llvm/llvm-project/issues/52890

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D119470
2022-03-17 14:27:09 +03:00
Danny Mösch 2b21fc5520 Allow newline characters as separators for checks in Clang-Tidy configurations
This is a fix for #53737. In addition to commas, newline characters are
considered as separators of checks.
2022-03-15 14:30:13 -04:00
Keith Smiley 5da83eeb91 clang-tidy: discover binaries in build dir
This changes the clang-tidy script to discover binaries you've built
locally without having to pass them.

Differential Revision: https://reviews.llvm.org/D100692
2022-03-14 09:04:36 -07:00
Balazs Benics 665879b9e7 [clang-tidy][docs] Fix wrong url in DontModifyStdNamespaceCheck
It was probably a copy-paste mistake.
The check was added as `cert-dcl58-cpp`, so the doc should point there.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D121373
2022-03-13 17:51:00 +01:00
Balazs Benics d321f09926 [clang-tidy][docs][NFC] Update URL and docs of PostfixOperatorCheck
The docs URL was dangling, and the docs suggested that it has no fixits,
but it actually had.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D121372
2022-03-13 17:51:00 +01:00
Richard 602ea53cb7 [clang-tidy] Refactor: Prefer single quoted strings over double quoted strings [NFC] 2022-03-11 15:08:24 -07:00
Jun Zhang ac616fbb05
[Clang-tidy] Check the existence of ElaboratedType's qualifiers
The ElaboratedType can have no qualifiers, so we should check it before
use.

Fix #issue53874(https://github.com/llvm/llvm-project/issues/53874)

Differential Revision: https://reviews.llvm.org/D119949
2022-03-01 23:52:44 +08:00
Sockke ba54ebeb5e [clang-tidy] Fix `readability-const-return-type` for pure virtual function.
It cannot match a `pure virtual function`. This patch fixes this behavior.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D116439
2022-03-01 20:55:28 +08:00
Whisperity 416e689ecd [clang-tidy] Fix `readability-suspicious-call-argument` crash for arguments without name-like identifier
As originally reported by @steakhal in
http://github.com/llvm/llvm-project/issues/54074, the name extraction logic of
`readability-suspicious-call-argument` crashes if the argument passed to a
function was a function call to a non-trivially named entity (e.g. an operator).

Fixed this crash case by ignoring such constructs and considering them as having
no name.

Reviewed By: aaron.ballman, steakhal

Differential Revision: http://reviews.llvm.org/D120555
2022-02-25 16:24:27 +01:00
Sockke 6cbf15e9b5 [clang-tidy] Fix `readability-non-const-parameter` for parameter referenced by an lvalue
The checker missed a check for a case when the parameter is referenced by an lvalue and this could cause build breakages.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D117090
2022-02-25 14:53:11 +08:00
Nathan James 79353f940c
[clang-tidy][NFC] Remove Tristate from CachedGlobList
The tristate is a little redundant as we can determine if the item was already in the cache based on the return from try_emplace.

Reviewed By: salman-javed-nz

Differential Revision: https://reviews.llvm.org/D120196
2022-02-23 08:35:31 +00:00
Kesavan Yogeswaran c57b8ca721
[clang-tidy] Provide fine control of color in run-clang-tidy
D90110 modified the behavior of `run-clang-tidy` to always pass the
`--use-color` option to clang-tidy, which enabled colored diagnostics
output regardless of TTY status or .clang-tidy settings. This left the
user with no option to disable the colored output.

This presents an issue when trying to parse the output of run-clang-tidy
programmaticall, as the output is polluted with ANSI escape characters.

This PR fixes this issue in two ways:
1. It restores the default behavior of `run-clang-tidy` to let
   `clang-tidy` decide whether to color output. This allows the user to
   configure color via the `UseColor` option in a .clang-tidy file.
2. It adds mutually exclusive, optional `-use-color` and `-no-use-color`
   argument flags that let the user explicitly set the color option via
   the invocation.

After this change the default behavior of `run-clang-tidy` when no
.clang-tidy file is available is now to show no color, presumably
because `clang-tidy` detects that the output is being piped and defaults
to not showing colored output. This seems like an acceptable tradeoff
to respect .clang-tidy configurations, as users can still use the
`-use-color` option to explicitly enable color.

Fixes #49441 (50097 in Bugzilla)

Reviewed By: njames93

Differential Revision: https://reviews.llvm.org/D119562
2022-02-20 22:00:28 +00:00
Evgeny Shulgin fc84ebfff3 [clang-tidy] Ignore variable template partial specializations in `misc-definitions-in-headers`
Variable template partial specializations are inline and can't lead
to ODR-violations. The checker now ignores them.

Fixes https://github.com/llvm/llvm-project/issues/53519

Reviewed By: hokein

Differential Revision: https://reviews.llvm.org/D119098
2022-02-14 11:38:27 +01:00
Simon Pilgrim 5d1e3ed3e2 [clang-tidy] SimplifyBooleanExprCheck - use cast<> instead of dyn_cast<> to avoid dereference of nullptr
The IfStmt pointer is always referenced inside the replaceCompoundReturnWithCondition call, so assert the cast is correct instead of returning nullptr
2022-02-12 19:59:30 +00:00
Simon Pilgrim 7ad94bd74b [clang-tidy] ContainerSizeEmptyCheck::check - simplify isa<> and dyn_cast<> repeated calls
Just use dyn_cast<> to determine literal + container values from the binop
2022-02-12 11:31:27 +00:00
Simon Pilgrim 869c066ca8 [clang-tidy] RedundantControlFlowCheck::issueDiagnostic - use cast<> instead of dyn_cast<> to avoid dereference of nullptr
The pointer is referenced immediately, so assert the cast is correct instead of returning nullptr
2022-02-12 11:05:59 +00:00
Simon Pilgrim 9acba5206a [clang-tidy] FunctionASTVisitor::TraverseStmt - use cast<> instead of dyn_cast<> to avoid dereference of nullptr
The pointer is referenced immediately, so assert the cast is correct instead of returning nullptr
2022-02-12 11:04:20 +00:00
Simon Pilgrim ed5d659038 [clang-tidy] NoexceptMoveConstructorCheck::check - use castAs<> instead of getAs<> to avoid dereference of nullptr
The pointer is referenced immediately, so assert the cast is correct instead of returning nullptr
2022-02-12 10:57:09 +00:00
Simon Pilgrim 99acc0dc64 [clang-tidy] UseNoexceptCheck::check - use cast<> instead of dyn_cast<> to avoid dereference of nullptr
The pointer is referenced immediately, so assert the cast is correct instead of returning nullptr
2022-02-12 10:51:48 +00:00
Simon Pilgrim 003e038264 [clang-tidy] getOutermostNamespace - remove redundant nullptr check
The pointer is always dereferenced
2022-02-12 10:49:54 +00:00
Simon Pilgrim 5bc13f9d94 [clang-tidy] ProBoundsConstantArrayIndexCheck::check - use cast<> instead of dyn_cast<> to avoid dereference of nullptr
The pointer is referenced immediately, so assert the cast is correct instead of returning nullptr
2022-02-12 10:43:21 +00:00
Simon Pilgrim 389bee71ea [clang-tidy] checkOverridingFunctionReturnType - use castAs<> instead of getAs<> to avoid dereference of nullptr
The pointers are referenced immediately, so assert the cast is correct instead of returning nullptr
2022-02-12 10:41:43 +00:00
Kirill Bobyrev 100ec80ab5 [clang-tidy] Add early exit for defaulted FunctionDecls
This prevents matching of defaulted comparison operators.

Resolves: https://github.com/llvm/llvm-project/issues/53355

Author: Febbe (Fabian Keßler)
2022-02-10 20:29:26 +01:00
Greg Miller d038faea46 [clang-tidy] add option performance-move-const-arg.CheckMoveToConstRef
This option allows callers to disable the warning from
https://clang.llvm.org/extra/clang-tidy/checks/performance-move-const-arg.html
that would warn on the following

```
void f(const string &s);
string s;
f(std::move(s));  // ALLOWED if performance-move-const-arg.CheckMoveToConstRef=false
```

The reason people might want to disable this check, is because it allows
callers to use `std::move()` or not based on local reasoning about the
argument, and without having to care about how the function `f` accepts
the argument. Indeed, `f` might accept the argument by const-ref today,
but change to by-value tomorrow, and if the caller had moved the
argument that they were finished with, the code would work as
efficiently as possible regardless of how `f` accepted the parameter.

Reviewed By: ymandel

Differential Revision: https://reviews.llvm.org/D119370
2022-02-10 13:31:07 +00:00
Corentin Jabot c151225096 [C++2b] Implement multidimentional subscript operator
Implement P2128R6 in C++23 mode.

Unlike GCC's implementation, this doesn't try to recover when a user
meant to use a comma expression.

Because the syntax changes meaning in C++23, the patch is *NOT*
implemented as an extension. Instead, declaring an array with not
exactly 1 parameter is an error in older languages modes. There is an
off-by-default extension warning in C++23 mode.

Unlike the standard, we supports default arguments;

Ie, we assume, based on conversations in WG21, that the proposed
resolution to CWG2507 will be accepted.

We allow arrays OpenMP sections and C++23 multidimensional array to
coexist:

[a , b] multi dimensional array
[a : b] open mp section
[a, b: c] // error

The rest of the patch is relatively straight forward: we take care to
support an arbitrary number of arguments everywhere.
2022-02-08 12:10:47 -05:00
Jesko Appelfeller 0851970af5 Make run-clang-tidy.py print the configured checks correctly
The test invocation at the start of run-clang-tidy.py (line 257) prints
all enabled checks - meaning either the default set or anything
configured via the -checks option. If any checks were (un-)configured
via the -config option, these are not printed. This is confusing to the
user, since the list of checks that are printed may be different from
the list of checks that are used by the non-testing calls to clang-tidy,
where the -config option is passed correctly.

This patch adds the -config option to the test invocation of clang-tidy
at the start of the script. This means that checks (un-)configured via
the -config option (rather than the -checks option) are applied
correctly, when printing the list of enabled checks.
2022-02-08 08:32:04 -05:00
Balázs Kéri 29fc5e0245 [clang-tidy] Fixed a compile warning (NFC).
Fixed a "override" related warning in SharedPtrArrayMismatchCheck.h.
Related to differential revision: https://reviews.llvm.org/D117306
2022-02-07 14:56:27 +01:00
Balázs Kéri c63522e6ba [clang-tidy] Add new check 'shared-ptr-array-mismatch'.
Reviewed By: LegalizeAdulthood

Differential Revision: https://reviews.llvm.org/D117306
2022-02-07 12:57:58 +01:00
Kadir Cetinkaya 0447ec2fb0
[clang-tidy] Fix LLVM include order check policy
Clang-format LLVM style has a custom include category for gtest/ and
gmock/ headers between regular includes and angled includes. Do the same here.

Fixes https://github.com/llvm/llvm-project/issues/53525.

Differential Revision: https://reviews.llvm.org/D118913
2022-02-03 17:32:43 +01:00
Jameson Nash 84f137a590 Reland "enable plugins for clang-tidy"
This reverts commit ab3b89855c but
disables the new test if the user has disabled support for building it.
2022-02-01 17:37:24 -05:00
Simon Pilgrim 1307f66d17 [clang-tidy] getLambdaProperties - use cast<> instead of dyn_cast<> to avoid dereference of nullptr
The pointers are dereferenced immediately, so assert the cast is correct instead of returning nullptr
2022-01-31 17:25:16 +00:00
Benjamin Kramer e07823ad72 [clang-tidy] Make header compile standalone. NFC. 2022-01-31 11:18:00 +01:00
Balázs Kéri b088237f08 [clang-tidy] bugprone-signal-handler improvements: display call chain
Display notes for a possible call chain if an unsafe function is found to be
called (maybe indirectly) from a signal handler.
The call chain displayed this way includes probably not the first calls of
the functions, but it is a valid possible (in non path-sensitive way) one.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D118224
2022-01-31 10:35:23 +01:00
Petr Hosek ab3b89855c Revert "enable plugins for clang-tidy"
This reverts commit 36892727e4 which
breaks the build when LLVM_INSTALL_TOOLCHAIN_ONLY is enabled with:

  CMake Error at cmake/modules/AddLLVM.cmake:683 (add_dependencies):
  The dependency target "clang-tidy-headers" of target "CTTestTidyModule"
  does not exist.
2022-01-31 00:55:43 -08:00
Kazu Hirata 2f18b02de7 [clang-tools-extra] Remove unused forward declarations (NFC) 2022-01-30 16:05:00 -08:00
Jameson Nash 36892727e4 enable plugins for clang-tidy
Fixes #32739

Differential Revision: https://reviews.llvm.org/D111100
2022-01-29 14:21:19 -05:00
Richard 99217fa8a0 [clang-tidy] Recognize labelled statements when simplifying boolean exprs
Inside a switch the caseStmt() and defaultStmt() have a nested statement
associated with them.  Similarly, labelStmt() has a nested statement.
These statements were being missed when looking for a compound-if of the
form "if (x) return true; return false;" when the if is nested under one
of these labelling constructs.

Enhance the matchers to look for these nested statements using some
private matcher hasSubstatement() traversal matcher on case, default
and label statements.  Add the private matcher hasSubstatementSequence()
to match the compound "if (x) return true; return false;" pattern.

- Add unit tests for private matchers and corresponding test
  infrastructure
- Add corresponding test file readability-simplify-bool-expr-case.cpp.
- Fix variable name copy/paste error in readability-simplify-bool-expr.cpp.
- Drop the asserts, which were used only for debugging matchers.
- Run clang-format on the whole check.
- Move local functions out of anonymous namespace and declare state, per
  LLVM style guide
- Declare labels constexpr
- Declare visitor arguments as pointer to const
- Drop braces around simple control statements per LLVM style guide
- Prefer explicit arguments over default arguments to methods

Differential Revision: https://reviews.llvm.org/D56303

Fixes #27078
2022-01-28 16:09:46 -07:00
MuAlphaOmegaEpsilon ccce1a03c9 Don't trigger unused-parameter warnings on naked functions
This commit checks if a function is marked with the naked attribute
and, if it is, will silence the emission of any unused-parameter
warning.

Inside a naked function only the usage of basic ASM instructions is
expected. In this context the parameters can actually be used by
fetching them according to the underlying ABI. Since parameters might
be used through ASM instructions, the linter and the compiler will have
a hard time understanding if one of those is unused or not, therefore
no unused-parameter warning should ever be triggered whenever a
function is marked naked.
2022-01-27 11:40:08 -05:00
Evgeny Shulgin 836950c4e6 [clang-tidy] Fix nested namespaces in `readability-static-definition-in-anonymous-namespace` check
The check previously inspected only the immediate parent namespace.
`static` in a named namespace within an unnamed namespace is still
redundant.
We will use `Decl::isInAnonymousNamespace()` method that traverses the
namespaces hierarchy recursively.

Differential Revision: https://reviews.llvm.org/D118010
2022-01-26 21:54:17 -07:00
Salman Javed c283c8dfb5 Rewrite Doxygen comment to resolve -Wdocumentation warning (NFC)
Comment change only, no functional change intended.
Example of warning:
https://lab.llvm.org/buildbot/#/builders/188/builds/8696/steps/4/logs/warnings__2_
2022-01-27 02:21:05 +13:00
Salman Javed 5da7c04003 Re-land "Cache the locations of NOLINTBEGIN/END blocks" with fix for build bot 2022-01-27 01:03:27 +13:00
Salman Javed 8e29d19b8d Revert "[clang-tidy] Cache the locations of NOLINTBEGIN/END blocks"
Build warning here:
https://lab.llvm.org/buildbot/#/builders/57/builds/14322
2022-01-27 00:52:44 +13:00
Salman Javed 19eaad94c4 [clang-tidy] Cache the locations of NOLINTBEGIN/END blocks
Support for NOLINT(BEGIN/END) blocks (implemented in D108560) is
currently costly. This patch aims to improve the performance with the
following changes:

- The use of tokenized NOLINTs instead of a series of repetitive ad-hoc
string operations (`find()`, `split()`, `slice()`, regex matching etc).
- The caching of NOLINT(BEGIN/END) block locations. Determining these
locations each time a new diagnostic is raised is wasteful as it
requires reading and parsing the entire source file.

Move NOLINT-specific code from `ClangTidyDiagnosticConsumer` to new
purpose-built class `NoLintDirectiveHandler`.

Differential Revision: https://reviews.llvm.org/D116085
2022-01-27 00:12:16 +13:00
Zinovy Nis 19d7a0b47b [clang-tidy] [bugprone-assert-side-effect] Ignore list for functions/methods
A semicolon-separated list of the names of functions or methods to be considered as not having side-effects was added for bugprone-assert-side-effect. It can be used to exclude methods like iterator::begin/end from being considered as having side-effects.

Differential Revision: https://reviews.llvm.org/D116478
2022-01-25 21:04:07 +03:00
Balázs Kéri 9d8c3ad94f [clang-tidy] Change code of SignalHandlerCheck (NFC).
Using clang::CallGraph to get the called functions.
This makes a better foundation to improve support for
C++ and print the call chain.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D118016
2022-01-25 15:52:38 +01:00
Jim Lin f3314e3747 [clang-tidy] Pop Files only if FileChangeReason is ExitFile
enum FileChangeReason has four possible type EnterFile, ExitFile,
SystemHeaderPragma and RenameFile,
It should pop the back element of Files only if FileChangeReason is ExitFile.
2022-01-25 22:46:12 +08:00
serge-sans-paille a0d5e938fe Add missing include llvm/ADT/STLExtras 2022-01-24 14:41:24 +01:00
Adrian Vogelsgesang 3696c70e67 [clang-tidy] Add `readability-container-contains` check
This commit introduces a new check `readability-container-contains` which finds
usages of `container.count()` and `container.find() != container.end()` and
instead recommends the `container.contains()` method introduced in C++20.

For containers which permit multiple entries per key (`multimap`, `multiset`,
...), `contains` is more efficient than `count` because `count` has to do
unnecessary additional work.

While this this performance difference does not exist for containers with only
a single entry per key (`map`, `unordered_map`, ...), `contains` still conveys
the intent better.

Reviewed By: xazax.hun, whisperity

Differential Revision: http://reviews.llvm.org/D112646
2022-01-24 12:57:18 +01:00
Richard d2e8fb3318 [clang-tidy] Add readability-duplicate-include check
Looks for duplicate includes and removes them.

Every time an include directive is processed, check a vector of filenames
to see if the included file has already been included.  If so, it issues
a warning and a replacement to remove the entire line containing the
duplicated include directive.

When a macro is defined or undefined, the vector of filenames is cleared.
This enables including the same file multiple times, but getting
different expansions based on the set of active macros at the time of
inclusion.  For example:

  #undef NDEBUG
  #include "assertion.h"
  // ...code with assertions enabled

  #define NDEBUG
  #include "assertion.h"
  // ...code with assertions disabled

Since macros are redefined between the inclusion of assertion.h,
they are not flagged as redundant.

Differential Revision: https://reviews.llvm.org/D7982
2022-01-23 09:23:04 -07:00
Carlos Galvez eb3f20e8fa [clang-tidy] Remove gsl::at suggestion from cppcoreguidelines-pro-bounds-constant-array-index
Currently the fix hint is hardcoded to gsl::at(). This poses
a problem for people who, for a number of reasons, don't want
or cannot use the GSL library (introducing a new third-party
dependency into a project is not a minor task).

In these situations, the fix hint does more harm than good
as it creates confusion as to what the fix should be. People
can even misinterpret the fix "gsl::at" as e.g. "std::array::at",
which can lead to even more trouble (e.g. when having guidelines
that disallow exceptions).

Furthermore, this is not a requirement from the C++ Core Guidelines.
simply that array indexing needs to be safe. Each project should
be able to decide upon a strategy for safe indexing.

The fix-it is kept for people who want to use the GSL library.

Differential Revision: https://reviews.llvm.org/D117857
2022-01-23 15:52:42 +00:00
Simon Pilgrim 86497026a2 [clang-tidy] Use cast<>/castAs<> instead of dyn_cast<>/getAs<> to avoid dereference of nullptr
The pointer is dereferenced immediately, so assert the cast is correct instead of returning nullptr
2022-01-23 12:57:12 +00:00
John Ericson 7c16647c36 [clang-tools-extra][cmake] Use `GNUInstallDirs` to support custom installation dirs.
This is the original patch in my GNUInstallDirs series, now last to merge as the final piece!

It arose as a new draft of D28234. I initially did the unorthodox thing of pushing to that when I wasn't the original author, but since I ended up

 - Using `GNUInstallDirs`, rather than mimicking it, as the original author was hesitant to do but others requested.

 - Converting all the packages, not just LLVM, effecting many more projects than LLVM itself.

I figured it was time to make a new revision.

I have used this patch series (and many back-ports) as the basis of https://github.com/NixOS/nixpkgs/pull/111487 for my distro (NixOS), which was merged last spring (2021). It looked like people were generally on board in D28234, but I make note of this here in case extra motivation is useful.

---

As pointed out in the original issue, a central tension is that LLVM already has some partial support for these sorts of things. Variables like `COMPILER_RT_INSTALL_PATH` have already been dealt with. Variables like `LLVM_LIBDIR_SUFFIX` however, will require further work, so that we may use `CMAKE_INSTALL_LIBDIR`.

These remaining items will be addressed in further patches. What is here is now rote and so we should get it out of the way before dealing more intricately with the remainder.

Reviewed By: #libunwind, #libc, #libc_abi, compnerd

Differential Revision: https://reviews.llvm.org/D99484
2022-01-22 20:57:21 +00:00
Benjamin Kramer b8102449a7 [clang-tidy] Avoid binding nullptr to a reference
That's undefined behavior. Found by -fsanitize=null.
2022-01-21 15:55:17 +01:00
Jan Svoboda 622354a522 [llvm][ADT] Implement `BitVector::{pop_,}back`
LLVM Programmer’s Manual strongly discourages the use of `std::vector<bool>` and suggests `llvm::BitVector` as a possible replacement.

Currently, some users of `std::vector<bool>` cannot switch to `llvm::BitVector` because it doesn't implement the `pop_back()` and `back()` functions.

To enable easy transition of `std::vector<bool>` users, this patch implements `llvm::BitVector::pop_back()` and `llvm::BitVector::back()`.

Reviewed By: dexonsmith

Differential Revision: https://reviews.llvm.org/D117115
2022-01-21 14:50:53 +01:00
Sockke a7f8aea714 [clang-tidy] Fix wrong FixIt in performance-move-const-arg
There are incorrect Fixit and missing warnings:
case :
A trivially-copyable object wrapped by std::move is passed to the function with rvalue reference parameters. Removing std::move will cause compilation errors.
```
void showInt(int&&) {}
void testInt() {
    int a = 10;
    // expect: warning + nofix
    showInt(std::move(a));  // showInt(a) <--- wrong fix
}

struct Tmp {};
void showTmp(Tmp&&) {}
void testTmp() {
    Tmp t;
    // expect: warning + nofix
    showTmp(std::move(t));  // showTmp(t) <--- wrong fix
}
```

Reviewed By: aaron.ballman, Quuxplusone

Differential Revision: https://reviews.llvm.org/D107450
2022-01-21 14:23:52 +08:00
Fabian Wolff d3b188a2d7 [clang-tidy] Include constructor initializers in `bugprone-exception-escape` check
Fixes PR#52435.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D113507
2022-01-21 00:53:57 +01:00
CJ Johnson a568411444 [clang-tidy] Update bugprone-stringview-nullptr to consistently prefer the empty string when passing arguments to constructors/functions
Previously, function(nullptr) would have been fixed with function({}). This unfortunately can change overload resolution and even become ambiguous. T(nullptr) was already being fixed with T(""), so this change just brings function calls in line with that.

Differential Revision: https://reviews.llvm.org/D117840
2022-01-20 18:08:40 -05:00
Nathan James 39f779afb3
[clang-tidy][NFC] Remove redundant string creation for comparison 2022-01-20 22:20:10 +00:00
Richard d83ecd77cc [clang-tidy] Narrow cppguidelines-macro-usage to actual constants
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/D116386

Fixes #39945
2022-01-19 12:28:22 -07:00
Nathan James a4bdeb9a56
[clang-tidy][NFC] replace some redundant std::string creations 2022-01-19 18:36:35 +00:00
Haojian Wu 6c78703e3a [AST] Fix the incorrect auto-keyword loc for constrained auto type loc.
E.g.  `Concept auto Func();`

The nameLoc for the constained auto type loc pointed to the concept name
loc, it should be the auto token loc. This patch fixes it, and remove
a relevant hack in clang-tidy check.

Reviewed By: sammccall

Differential Revision: https://reviews.llvm.org/D117009
2022-01-19 14:18:38 +01:00
Fabian Wolff f7b7138a62 [clang-tidy] Make `readability-container-data-pointer` more robust
Fixes PR#52245. I've also added a few test cases beyond PR#52245 that would also fail with the current implementation, which is quite brittle in many respects (e.g. it uses the `hasDescendant()` matcher to find the container that is being accessed, which is very easy to trick, as in the example in PR#52245).

I have not been able to reproduce the second issue mentioned in PR#52245 (namely that using the `data()` member function is suggested even for containers that don't have it), but I've added a test case for it to be sure.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D113863
2022-01-18 21:08:59 +01:00
Jan Svoboda c6fb636667 [clangd][clang-tidy] Remove uses of `std::vector<bool>`
LLVM Programmer’s Manual strongly discourages the use of `std::vector<bool>` and suggests `llvm::BitVector` as a possible replacement.

This patch does just that for clangd and clang-tidy.

Reviewed By: dexonsmith

Differential Revision: https://reviews.llvm.org/D117119
2022-01-18 17:59:40 +01:00
Richard e598913a47 [clang-tidy] Force LF newlines when writing files
The recommendation on Windows is to checkout from git with
core.autolf=false in order to preserve LF line endings on
test files.  However, when creating a new check this results
in modified files as having switched all the line endings on
Windows.  Write all files with explicit LF line endings to
prevent this.

Fixes #52968

Differential Revision: https://reviews.llvm.org/D117535
2022-01-18 09:39:42 -07:00
Jan Svoboda 105c913156 [clang][lex] NFC: Simplify calls to `LookupFile`
The `{HeaderSearch,Preprocessor}::LookupFile()` functions take an out-parameter `const DirectoryLookup *&`. Most callers end up creating a `const DirectoryLookup *` variable that's otherwise unused.

This patch changes the out-parameter from reference to a pointer, making it possible to simply pass `nullptr` to the function without the ceremony.

Reviewed By: ahoppen

Differential Revision: https://reviews.llvm.org/D117312
2022-01-18 16:02:18 +01:00
Fabian Wolff 2cd2accc61 [clang-tidy] Fix false positives involving type aliases in `misc-unconventional-assign-operator` check
clang-tidy currently reports false positives even for simple cases such as:
```
struct S {
    using X = S;
    X &operator=(const X&) { return *this; }
};
```
This is due to the fact that the `misc-unconventional-assign-operator` check fails to look at the //canonical// types. This patch fixes this behavior.

Reviewed By: aaron.ballman, mizvekov

Differential Revision: https://reviews.llvm.org/D114197
2022-01-17 21:16:17 +01:00
Fabian Wolff 42bc3275d3 [clang-tidy] Fix `readability-redundant-declaration` false positive for template friend declaration
Fixes [[ https://bugs.llvm.org/show_bug.cgi?id=48086 | PR#48086 ]]. The problem is that the current matcher uses `hasParent()` to detect friend declarations, but for a template friend declaration, the immediate parent of the `FunctionDecl` is a `FunctionTemplateDecl`, not the `FriendDecl`. Therefore, I have replaced the matcher with `hasAncestor()`.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D114299
2022-01-17 20:50:32 +01:00
Haojian Wu ab3f100bec Reland (2) "[AST] Add RParen loc for decltype AutoTypeloc.""
The patch was reverted because it caused a crash during PCH build -- we
missed to update the RParenLoc in TreeTransform<Derived>::TransformAutoType.

This relands 55d96ac and 37ec65e with a test and fix.
2022-01-17 11:33:11 +01:00
John Ericson da77db58d7 Revert "[cmake] Use `GNUInstallDirs` to support custom installation dirs."
https://lab.llvm.org/buildbot/#/builders/46/builds/21146 Still have
this odd error, not sure how to reproduce, so I will just try breaking
up my patch.

This reverts commit 4a678f8072.
2022-01-16 05:48:30 +00:00
John Ericson 4a678f8072 [cmake] Use `GNUInstallDirs` to support custom installation dirs.
This is the original patch in my GNUInstallDirs series, now last to merge as the final piece!

It arose as a new draft of D28234. I initially did the unorthodox thing of pushing to that when I wasn't the original author, but since I ended up

 - Using `GNUInstallDirs`, rather than mimicking it, as the original author was hesitant to do but others requested.

 - Converting all the packages, not just LLVM, effecting many more projects than LLVM itself.

I figured it was time to make a new revision.

I have used this patch series (and many back-ports) as the basis of https://github.com/NixOS/nixpkgs/pull/111487 for my distro (NixOS), which was merged last spring (2021). It looked like people were generally on board in D28234, but I make note of this here in case extra motivation is useful.

---

As pointed out in the original issue, a central tension is that LLVM already has some partial support for these sorts of things. Variables like `COMPILER_RT_INSTALL_PATH` have already been dealt with. Variables like `LLVM_LIBDIR_SUFFIX` however, will require further work, so that we may use `CMAKE_INSTALL_LIBDIR`.

These remaining items will be addressed in further patches. What is here is now rote and so we should get it out of the way before dealing more intricately with the remainder.

Reviewed By: #libunwind, #libc, #libc_abi, compnerd

Differential Revision: https://reviews.llvm.org/D99484
2022-01-16 05:33:07 +00:00
John Ericson 6e52bfe09d Revert "[cmake] Use `GNUInstallDirs` to support custom installation dirs."
Sorry for the disruption, I will try again later.

This reverts commit efeb501970.
2022-01-15 07:35:02 +00:00
John Ericson efeb501970 [cmake] Use `GNUInstallDirs` to support custom installation dirs.
This is the original patch in my GNUInstallDirs series, now last to merge as the final piece!

It arose as a new draft of D28234. I initially did the unorthodox thing of pushing to that when I wasn't the original author, but since I ended up

 - Using `GNUInstallDirs`, rather than mimicking it, as the original author was hesitant to do but others requested.

 - Converting all the packages, not just LLVM, effecting many more projects than LLVM itself.

I figured it was time to make a new revision.

I have used this patch series (and many back-ports) as the basis of https://github.com/NixOS/nixpkgs/pull/111487 for my distro (NixOS), which was merged last spring (2021). It looked like people were generally on board in D28234, but I make note of this here in case extra motivation is useful.

---

As pointed out in the original issue, a central tension is that LLVM already has some partial support for these sorts of things. Variables like `COMPILER_RT_INSTALL_PATH` have already been dealt with. Variables like `LLVM_LIBDIR_SUFFIX` however, will require further work, so that we may use `CMAKE_INSTALL_LIBDIR`.

These remaining items will be addressed in further patches. What is here is now rote and so we should get it out of the way before dealing more intricately with the remainder.

Reviewed By: #libunwind, #libc, #libc_abi, compnerd

Differential Revision: https://reviews.llvm.org/D99484
2022-01-15 01:08:35 +00:00
CJ Johnson 81c330e23d Filter string_view from the nullptr diagnosis of bugprone-string-constructor to prevent duplicate warnings with bugprone-stringview-nullptr
Updates the check and tests to not diagnose the null case for string_view (but retains it for string). This prevents the check from giving duplicate warnings that are caught by bugprone-stringview-nullptr ([[ https://reviews.llvm.org/D113148 | D113148 ]]).

Reviewed By: ymandel

Differential Revision: https://reviews.llvm.org/D114823
2022-01-12 17:04:44 -05:00
CJ Johnson 7e29da875c Add support for return values in bugprone-stringview-nullptr
bugprone-stringview-nullptr was not initially written with tests for return statements. After landing the check, the thought crossed my mind to add such tests. After writing them, I realized they needed additional handling in the matchers.

Differential Revision: https://reviews.llvm.org/D115121
2022-01-12 16:53:13 -05:00
Richard fff59f4817 [clang-tidy] Improve modernize-redundant-void-arg to recognize macro uses
Sometimes a macro invocation will look like an argument list
declaration.  Improve the check to detect this situation and not
try to modify the macro invocation.

Thanks to Nathan James for the fix.

- Ignore implicit typedefs (e.g. compiler builtins)
- Improve lexing state machine to locate void argument tokens
- Add additional return_t() macro tests
- clang-format control in the test case file
- remove braces around single statements per LLVM style guide

Fixes #43791

Differential Revision: https://reviews.llvm.org/D116425
2022-01-12 13:51:50 -07:00
Simon Pilgrim dcc20143e1 [clang-tidy] UseDefaultMemberInitCheck::checkDefaultInit - Use cast<> instead of dyn_cast<> to avoid dereference of nullptr
The pointer is always dereferenced immediately below, so assert the cast is correct instead of returning nullptr
2022-01-12 15:35:37 +00:00
serge-sans-paille 35cca45b09 Misleading bidirectional detection
This patch implements detection of incomplete bidirectional sequence withing
comments and string literals within clang-tidy.

It detects the bidi part of https://www.trojansource.codes/trojan-source.pdf

Differential Revision: https://reviews.llvm.org/D112913
2022-01-12 11:38:36 +01:00
Florian Hahn eadb4cfeef
Revert (2) "[AST] Add RParen loc for decltype AutoTypeloc."
This reverts commit 41fbdfa4d5.

The commit breaks stage 2 builds with debug info, e.g.
https://green.lab.llvm.org/green/job/clang-stage2-Rthinlto/5088/console

Clang crashes with the following assertion when building
llvm-project/llvm/lib/Support/Timer.cpp

/usr/local/bin/sccache /Users/buildslave/jenkins/workspace/clang-stage2-Rthinlto/host-compiler/bin/clang++  -DGTEST_HAS_RTTI=0 -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Ilib/Support -I/Users/buildslave/jenkins/workspace/clang-stage2-Rthinlto/llvm-project/llvm/lib/Support -Iinclude -I/Users/buildslave/jenkins/workspace/clang-stage2-Rthinlto/llvm-project/llvm/include -fno-stack-protector -fno-common -Wno-profile-instr-unprofiled -fPIC -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -fmodules -fmodules-cache-path=/Users/buildslave/jenkins/workspace/clang-stage2-Rthinlto/clang-build/Build/module.cache -fcxx-modules -Xclang -fmodules-local-submodule-visibility -gmodules -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 -fdiagnostics-color -flto=thin  -O2 -g -DNDEBUG -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk   -std=c++14  -fno-exceptions -fno-rtti -MD -MT lib/Support/CMakeFiles/LLVMSupport.dir/Timer.cpp.o -MF lib/Support/CMakeFiles/LLVMSupport.dir/Timer.cpp.o.d -o lib/Support/CMakeFiles/LLVMSupport.dir/Timer.cpp.o -c /Users/buildslave/jenkins/workspace/clang-stage2-Rthinlto/llvm-project/llvm/lib/Support/Timer.cpp
Assertion failed: (((getOffset()+Offset) & MacroIDBit) == 0 && "offset overflow"), function getLocWithOffset, file /Users/buildslave/jenkins/workspace/clang-stage1-RA/llvm-project/clang/include/clang/Basic/SourceLocation.h, line 135.
2022-01-12 10:09:37 +00:00
Elvis Stansvik 36af073342 Accept string literal decay in conditional operator
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.
2022-01-11 15:05:30 -05:00
Haojian Wu 41fbdfa4d5 Reland "[AST] Add RParen loc for decltype AutoTypeloc."
Reland 55d96ac and 37ec65e with a clang-tidy fix.
2022-01-11 12:06:18 +01:00
Logan Smith 988c3f5f96 [clang-tidy] Fix RenamerClangTidyChecks suggesting invalid macro identifiers
This behavior was fixed for regular identifiers in
9f3edc323a, but the same fix was not applied to
macro fixits. This addresses https://github.com/llvm/llvm-project/issues/52895.

Differential Revision: https://reviews.llvm.org/D116824
2022-01-10 10:35:04 -08:00
Kazu Hirata b12fd13812 Fix bugprone argument comments.
Identified by bugprone-argument-comment.
2022-01-09 12:21:02 -08:00
Qiu Chaofan c2cc70e4f5 [NFC] Fix endif comments to match with include guard 2022-01-07 15:52:59 +08:00
Carlos Galvez 670de10f9d Disable clang-tidy warnings from system macros
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
2022-01-06 20:27:28 +00:00
Richard d7b6574c3b [clang-tidy] Recognize transformer checks as providing fixits
- Recognize older checks that might not end with Check.cpp
- Update list of checks based on improvements to add_new_check
- Fix spelling error in TransformerClangTidyCheck.h

Fixes #52962

Differential Revision: https://reviews.llvm.org/D116550
2022-01-05 16:13:52 -07:00
Clement Courbet ed8ff29aa6 [clang-tidy] Fix false positive in modernize-pass-by-value
The check should not trigger on lvalue/rvalue overload pairs:

```
struct S {
  S(const A& a) : a(a) {}
  S(A&& a) : a(std::move(a)) {}

  A a;
}
```

Differential Revision: https://reviews.llvm.org/D116535
2022-01-05 14:33:40 +01:00
Oleg Smolsky 051847cfec Improve the 'modernize-use-default-member-init'
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;
};
2022-01-04 07:27:02 -05:00
Kazu Hirata e47a224ccf [clang-tidy] Use nullptr instead of 0 or NULL (NFC)
Identified with modernize-use-nullptr.
2021-12-31 13:54:34 -08:00
Kazu Hirata 0542d15211 Remove redundant string initialization (NFC)
Identified with readability-redundant-string-init.
2021-12-26 09:39:26 -08:00
Kazu Hirata 2d303e6781 Remove redundant return and continue statements (NFC)
Identified with readability-redundant-control-flow.
2021-12-24 23:17:54 -08:00
Kazu Hirata 3cfe375ae4 Use StringRef::contains (NFC) 2021-12-24 22:05:34 -08:00
Kazu Hirata 62e48ed10f Use isa instead of dyn_cast (NFC) 2021-12-24 21:22:27 -08:00
Salman Javed d840f3edf0 Resolve -Wdocumentation warning in ClangTidyDiagnosticConsumer (NFC)
Change to comments only; NFC.

```
ClangTidyDiagnosticConsumer.h:245:6: warning: '\param' command used in a
comment that is not attached to a function declaration [-Wdocumentation]
```

See this build for an example:
https://lab.llvm.org/buildbot/#/builders/109/builds/27293/steps/5/logs/warnings__702_
2021-12-23 10:25:05 +13:00
Salman Javed 86618e37bd Resolve lint warning about converting unsigned to signed (NFC)
FileOffset is unsigned while getLocWithOffset() requires a signed value.
2021-12-23 09:46:14 +13:00
Ivan Gerasimov fd8fc5e8d9 [clang-tidy] abseil-string-find-startswith: detect `s.rfind(z, 0) == 0`
Suggest converting `std::string::rfind()` calls to `absl::StartsWith()`
where possible.
2021-12-22 16:45:51 +01:00
Sam McCall af27466c50 Reland "[AST] Add UsingType: a sugar type for types found via UsingDecl"
This reverts commit cc56c66f27.
Fixed a bad assertion, the target of a UsingShadowDecl must not have
*local* qualifiers, but it can be a typedef whose underlying type is qualified.
2021-12-20 18:03:15 +01:00
Sam McCall cc56c66f27 Revert "[AST] Add UsingType: a sugar type for types found via UsingDecl"
This reverts commit e1600db19d.

Breaks sanitizer tests, at least on windows:
https://lab.llvm.org/buildbot/#/builders/127/builds/21592/steps/4/logs/stdio
2021-12-20 17:53:56 +01:00
Sam McCall e1600db19d [AST] Add UsingType: a sugar type for types found via UsingDecl
Currently there's no way to find the UsingDecl that a typeloc found its
underlying type through. Compare to DeclRefExpr::getFoundDecl().

Design decisions:
- a sugar type, as there are many contexts this type of use may appear in
- UsingType is a leaf like TypedefType, the underlying type has no TypeLoc
- not unified with UnresolvedUsingType: a single name is appealing,
  but being sometimes-sugar is often fiddly.
- not unified with TypedefType: the UsingShadowDecl is not a TypedefNameDecl or
  even a TypeDecl, and users think of these differently.
- does not cover other rarer aliases like objc @compatibility_alias,
  in order to be have a concrete API that's easy to understand.
- implicitly desugared by the hasDeclaration ASTMatcher, to avoid
  breaking existing patterns and following the precedent of ElaboratedType.

Scope:
- This does not cover types associated with template names introduced by
  using declarations. A future patch should introduce a sugar TemplateName
  variant for this. (CTAD deduced types fall under this)
- There are enough AST matchers to fix the in-tree clang-tidy tests and
  probably any other matchers, though more may be useful later.

Caveats:
- This changes a fairly common pattern in the AST people may depend on matching.
  Previously, typeLoc(loc(recordType())) matched whether a struct was
  referred to by its original scope or introduced via using-decl.
  Now, the using-decl case is not matched, and needs a separate matcher.
  This is similar to the case of typedefs but nevertheless both adds
  complexity and breaks existing code.

Differential Revision: https://reviews.llvm.org/D114251
2021-12-20 17:15:38 +01:00
Paul Altin 9198d04c06 Allow disabling integer to floating-point narrowing conversions for cppcoreguidelines-narrowing-conversions
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.
2021-12-16 08:24:09 -05:00
Markus Böck b7d55771ce [clang-tidy][#51939] Exempt placement-new expressions from 'bugprone-throw-keyword-missing'
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
2021-12-15 16:59:14 +01:00
CJ Johnson a1968d5341 Prevent abseil-cleanup-ctad check from stomping on surrounding context
This change applies two fixes to the abseil-cleanup-ctad check. It uses hasSingleDecl() to ensure only declStmt()s with one varDecl() are matched (leaving compount declStmt()s unchanged). It also addresses a bug in the handling of comments that surround the absl::MakeCleanup() calls by switching to the callArgs() combinator from Clang Transformer.

Reviewed By: ymandel

Differential Revision: https://reviews.llvm.org/D115452
2021-12-09 17:41:12 +00:00
Adrian Vogelsgesang 5d3b8956e8 Don't offer partial fix-its for `modernize-pass-by-value`
This commit improves the fix-its of modernize-pass-by-value by
no longer proposing partial fixes. In the presence of using/typedef,
we failed to rewrite the function signature but still adjusted the
function body. This led to incorrect, partial fix-its. Instead, the
check now simply doesn't offer any fixes at all in such a situation.
2021-12-08 13:31:30 -05:00
Carlos Galvez 946eb7a037 [clang-tidy][NFC] Move CachedGlobList to GlobList.h
Currently it's hidden inside ClangTidyDiagnosticConsumer,
so it's hard to know it exists.

Given that there are multiple uses of globs in clang-tidy,
it makes sense to have these classes publicly available
for other use cases that might benefit from it.

Also, add unit test by converting the existing tests
for GlobList into typed tests.

Reviewed By: salman-javed-nz

Differential Revision: https://reviews.llvm.org/D113422
2021-12-04 08:50:49 +00:00
Vy Nguyen aba8f320cc [clang-tidy][objc] Finds and fixes improper usages of XCTAssertEquals and XCTAssertNotEquals.
Using XCTAssertEqual on NSString* objects is almost always  wrong.

Unfortunately, we have seen a lot of tests doing this and reyling on pointer equality for strings with the same values (which happens to work sometimes - depending on the linker, but this assumption is not guaranteed by the language)

These fixes would make tests less brittle.

Differential Revision: https://reviews.llvm.org/D114975
2021-12-02 18:32:16 -05:00
Yitzhak Mandelbaum 081074e1ea [clang-tidy] Allow disabling support for NOLINTBEGIN/NOLINTEND blocks.
This patch parameterizes the clang-tidy diagnostic consumer with a boolean that
controls whether to honor NOLINTBEGIN/NOLINTEND blocks. The current support for
scanning these blocks is very costly -- O(n*m) in the size of files (n) and
number of diagnostics found (m), with a large constant factor.  So, the patch
allows clients to disable it.

Future patches should make the feature more efficient, but this will mitigate in
the interim.

Differential Revision: https://reviews.llvm.org/D114981
2021-12-02 22:19:05 +00:00
Yitzhak Mandelbaum c006ea6bde [clang-tidy] Fix build broken by commit 6a9487df73 (D113148) 2021-12-02 14:41:00 +00:00
CJ Johnson 6a9487df73 Add new clang-tidy check for string_view(nullptr)
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
2021-12-02 13:25:28 +00:00
Fabian Wolff 987a21522f [clang-tidy] Use `hasCanonicalType()` matcher in `bugprone-unused-raii` check
Fixes PR#52217.

Reviewed By: simon.giesecke

Differential Revision: https://reviews.llvm.org/D113429
2021-12-02 01:53:12 +01:00
Fabian Wolff 844a8d3cec Fix false positives in `fuchsia-trailing-return` check involving deduction guides
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.
2021-12-01 15:28:01 -05:00
Michael Jones 155f5a6dac [libc][clang-tidy] fix namespace check for externals
Up until now, all references to `errno` were marked with `NOLINT`, since
it was technically calling an external function. This fixes the lint
rules so that `errno`, as well as `malloc`, `calloc`, `realloc`, and
`free` are all allowed to be called as external functions. All of the
relevant `NOLINT` comments have been removed, and the documentation has
been updated.

Reviewed By: sivachandra, lntue, aaron.ballman

Differential Revision: https://reviews.llvm.org/D113946
2021-11-30 11:44:24 -08:00
Carlos Galvez 5bbe50148f [clang-tidy] Warn on functional C-style casts
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
2021-11-30 07:31:17 +00:00
Salman Javed c7aa358798 [clang-tidy] Fix pr48613: "llvm-header-guard uses a reserved identifier"
Fixes https://bugs.llvm.org/show_bug.cgi?id=48613.

llvm-header-guard is suggesting header guards with leading underscores
if the header file path begins with a '/' or similar special character.
Only reserved identifiers should begin with an underscore.

Differential Revision: https://reviews.llvm.org/D114149
2021-11-30 12:43:35 +13:00
Balazs Benics a8120a7711 [clang-tidy] Ignore narrowing conversions in case of bitfields
Bitfields are special. Due to integral promotion [conv.prom/5] bitfield
member access expressions are frequently wrapped by an implicit cast to
`int` if that type can represent all the values of the bitfield.

Consider these examples:
  struct SmallBitfield { unsigned int id : 4; };
  x.id & 1;             (case-1)
  x.id & 1u;            (case-2)
  x.id << 1u;           (case-3)
  (unsigned)x.id << 1;  (case-4)

Due to the promotion rules, we would get a warning for case-1. It's
debatable how useful this is, but the user at least has a convenient way
of //fixing// it by adding the `u` unsigned-suffix to the literal as
demonstrated by case-2. However, this won't work for shift operators like
the one in case-3. In case of a normal binary operator, both operands
contribute to the result type. However, the type of the shift expression is
the promoted type of the left operand. One could still suppress this
superfluous warning by explicitly casting the bitfield member access as
case-4 demonstrates, but why? The compiler already knew that the value from
the member access should safely fit into an `int`, why do we have this
warning in the first place? So, hereby we suppress this specific scenario,
when a bitfield's value is implicitly cast to int (likely due to integral
promotion).

Note that the bitshift operation might invoke unspecified/undefined
behavior, but that's another topic, this checker is about detecting
conversion-related defects.

Example AST for `x.id << 1`:
  BinaryOperator 'int' '<<'
  |-ImplicitCastExpr 'int' <IntegralCast>
  | `-ImplicitCastExpr 'unsigned int' <LValueToRValue>
  |   `-MemberExpr 'unsigned int' lvalue bitfield .id
  |     `-DeclRefExpr 'SmallBitfield' lvalue ParmVar 'x' 'SmallBitfield'
  `-IntegerLiteral 'int' 1

Reviewed By: courbet

Differential Revision: https://reviews.llvm.org/D114105
2021-11-29 09:56:43 +01:00
Balazs Benics e1d0673aee [clang-tidy] Fix crashing altera-struct-pack-align on invalid RecordDecls
Reviewed-By: martong

Differential Revision: https://reviews.llvm.org/D114256
2021-11-29 09:56:43 +01:00
Balazs Benics 0540485436 [libtooling][clang-tidy] Fix crashing on rendering invalid SourceRanges
Invalid SourceRanges can occur generally if the code does not compile,
thus we expect clang error diagnostics.
Unlike `clang`, `clang-tidy` did not swallow invalid source ranges, but
tried to highlight them, and blow various assertions.

The following two examples produce invalid source ranges, but this is
not a complete list:

  void test(x); // error: unknown type name 'x'
  struct Foo {
    member; // error: C++ requires a type specifier for all declarations
  };

Thanks @whisperity helping me fix this.

Reviewed-By: xazax.hun

Differential Revision: https://reviews.llvm.org/D114254
2021-11-29 09:56:43 +01:00
Balazs Benics 0685e83534 Fix cppcoreguidelines-virtual-base-class-destructor in macros
The `cppcoreguidelines-virtual-base-class-destructor` checker crashed on
this example:

  #define DECLARE(CLASS) \
  class CLASS {          \
  protected:             \
    virtual ~CLASS();    \
  }
  DECLARE(Foo); // no-crash

The checker will hit the following assertion:

  clang-tidy: llvm/include/llvm/ADT/Optional.h:196: T &llvm::optional_detail::OptionalStorage<clang::Token, true>::getValue() & [T = clang::Token]: Assertion `hasVal' failed."

It turns out, `Lexer::findNextToken()` returned `llvm::None` within the
`getVirtualKeywordRange()` function when the `VirtualEndLoc`
SourceLocation represents a macro expansion.
To prevent this from happening, I decided to propagate the `llvm::None`
further up and only create the removal of `virtual` if the
`getVirtualKeywordRange()` succeeds.

I considered an alternative fix for this issue:
I could have checked the `Destructor.getLocation().isMacroID()` before
doing any Fixit calculation inside the `check()` function.
In contrast to this approach my patch will preserve the diagnostics and
drop the fixits only if it would have crashed.

Reviewed By: whisperity

Differential Revision: https://reviews.llvm.org/D113558
2021-11-29 09:56:43 +01:00
Clement Courbet ba4411e7c6 [clang-tidy] performance-unnecessary-copy-initialization: Fix false negative.
`isConstRefReturningMethodCall` should be considering
`CXXOperatorCallExpr` in addition to `CXXMemberCallExpr`. Clang considers
these to be distinct (`CXXOperatorCallExpr` derives from `CallExpr`, not
`CXXMemberCallExpr`), but we don't care in the context of this
check.

This is important because of
`std::vector<Expensive>::operator[](size_t) const`.

Differential Revision: https://reviews.llvm.org/D114249
2021-11-24 08:07:21 +01:00
Felix Berger fefe20b993 [clang-tidy] performance-unnecessary-copy-initialization: Correctly match the type name of the thisPointertype.
The matching did not work correctly for pointer and reference types.

Differential Revision: https://reviews.llvm.org/D114212

Reviewed-by: courbet
2021-11-20 15:13:41 -05:00
Fabian Wolff 6259016361 [clang-tidy] Fix false positive in readability-identifier-naming check involving override attribute
Overriding methods should not get a readability-identifier-naming
warning because the issue can only be fixed in the base class; but the
current check for whether a method is overriding does not take the
override attribute into account.

Differential Revision: https://reviews.llvm.org/D113830
2021-11-19 22:31:11 +13:00
Haojian Wu 4ea066acc9 [clangd] Fix assertion crashes on unmatched NOLINTBEGIN comments.
The overload shouldSuppressDiagnostic seems unnecessary, and it is only
used in clangd.

This patch removes it and use the real one (suppression diagnostics are
discarded in clangd at the moment).

Fixes https://github.com/clangd/clangd/issues/929

Differential Revision: https://reviews.llvm.org/D113999
2021-11-17 15:31:38 +01:00
Fabian Wolff 738e7f1231 Fix false positive in `bugprone-throw-keyword-missing` check
Fixes PR#52400. The tests for bugprone-throw-keyword-missing actually
already contain exceptions as class members, but not as members with
initializers, which was probably just an oversight.
2021-11-16 07:09:17 -05:00
Carlos Galvez 9699c0fea3 [clang-tidy][NFC] Simplify ClangTidyStats
- Use NSDMI and remove constructor.

Differential Revision: https://reviews.llvm.org/D113847
2021-11-16 07:36:05 +00:00
Matheus Izvekov c9e46219f3
[clang] retain type sugar in auto / template argument deduction
This implements the following changes:
* AutoType retains sugared deduced-as-type.
* Template argument deduction machinery analyses the sugared type all the way
down. It would previously lose the sugar on first recursion.
* Undeduced AutoType will be properly canonicalized, including the constraint
template arguments.
* Remove the decltype node created from the decltype(auto) deduction.

As a result, we start seeing sugared types in a lot more test cases,
including some which showed very unfriendly `type-parameter-*-*` types.

Signed-off-by: Matheus Izvekov <mizvekov@gmail.com>

Reviewed By: rsmith, #libc, ldionne

Differential Revision: https://reviews.llvm.org/D110216
2021-11-15 23:07:45 +01:00
Kirstóf Umann d896c9f40a Fix an unused variable warning 2021-11-15 15:45:43 +01:00
Kristóf Umann 29a8d45c5a [clang-tidy] Fix a crash in modernize-loop-convert around conversion operators
modernize-loop-convert checks and fixes when a loop that iterates over the
elements of a container can be rewritten from a for(...; ...; ...) style into
the "new" C++11 for-range format. For that, it needs to parse the elements of
that loop, like its init-statement, such as ItType it = cont.begin().
modernize-loop-convert checks whether the loop variable is initialized by a
begin() member function.

When an iterator is initialized with a conversion operator (e.g. for
(const_iterator it = non_const_container.begin(); ...), attempts to retrieve the
name of the initializer expression resulted in an assert, as conversion
operators don't have a valid IdentifierInfo.

I fixed this by making digThroughConstructors dig through conversion operators
as well.

Differential Revision: https://reviews.llvm.org/D113201
2021-11-15 13:11:29 +01:00
Fabian Wolff c3e3c76209 [clang-tidy] Fix `bugprone-use-after-move` check to also consider moves in constructor initializers
Fixes PR#38187. Constructors are actually already checked,
but only as functions, i.e. the check only looks at the
constructor body and not at the initializers, which misses
the (common) case where constructor parameters are moved
as part of an initializer expression.

One remaining false negative is when both the move //and//
the use-after-move occur in constructor initializers.
This is a lot more difficult to handle, though, because
the `bugprone-use-after-move` check is currently based on
a CFG that only takes the body into account, not the
initializers, so e.g. initialization order would have to
manually be considered. I will file a follow-up issue for
this once PR#38187 is closed.

Reviewed By: carlosgalvezp

Differential Revision: https://reviews.llvm.org/D113708
2021-11-15 07:41:35 +00:00
Matheus Izvekov 6438a52df1
Revert "[clang] retain type sugar in auto / template argument deduction"
This reverts commit 4d8fff477e.
2021-11-15 00:29:05 +01:00
Matheus Izvekov 4d8fff477e
[clang] retain type sugar in auto / template argument deduction
This implements the following changes:
* AutoType retains sugared deduced-as-type.
* Template argument deduction machinery analyses the sugared type all the way
down. It would previously lose the sugar on first recursion.
* Undeduced AutoType will be properly canonicalized, including the constraint
template arguments.
* Remove the decltype node created from the decltype(auto) deduction.

As a result, we start seeing sugared types in a lot more test cases,
including some which showed very unfriendly `type-parameter-*-*` types.

Signed-off-by: Matheus Izvekov <mizvekov@gmail.com>

Reviewed By: rsmith

Differential Revision: https://reviews.llvm.org/D110216
2021-11-13 03:35:22 +01:00
Adrian Kuegel 1d7fdbbc18 Revert "[clang] retain type sugar in auto / template argument deduction"
This reverts commit 9b6036deed.
Breaks two libc++ tests.
2021-11-12 13:21:59 +01:00
Salman Javed 9089a1dff0 [clang-tidy] Re-apply 0076957 with fix for failing ASan tests
Re-apply "Fix lint warning in ClangTidyDiagnosticConsumer.cpp (NFC)"
with fixes for the failing ASan tests.

This reverts commit 74add1b6d6.
2021-11-13 00:01:12 +13:00
Matheus Izvekov 9b6036deed
[clang] retain type sugar in auto / template argument deduction
This implements the following changes:
* AutoType retains sugared deduced-as-type.
* Template argument deduction machinery analyses the sugared type all the way
down. It would previously lose the sugar on first recursion.
* Undeduced AutoType will be properly canonicalized, including the constraint
template arguments.
* Remove the decltype node created from the decltype(auto) deduction.

As a result, we start seeing sugared types in a lot more test cases,
including some which showed very unfriendly `type-parameter-*-*` types.

Signed-off-by: Matheus Izvekov <mizvekov@gmail.com>

Reviewed By: rsmith

Differential Revision: https://reviews.llvm.org/D110216
2021-11-12 01:16:31 +01:00
serge-sans-paille a35efc4dcb Misleading unicode identifier detection pass
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
2021-11-10 10:21:27 +01:00
Salman Javed b4f6f1c936 [clang-tidy] Fix llvm-header-guard so that it works with Windows paths
Fixes pr40372 (https://bugs.llvm.org/show_bug.cgi?id=40372).

The llvm-header-guard check does not take into account that the path
separator on Windows is `\`, not `/`.

This means that instead of suggesting a header guard in the form of:
LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FOO_H

it incorrectly suggests:
C:\LLVM_PROJECT\CLANG_TOOLS_EXTRA\CLANG_TIDY\FOO_H

Differential Revision: https://reviews.llvm.org/D113450
2021-11-10 18:35:57 +13:00
serge-sans-paille c178ada3c3 Revert "Misleading unicode identifier detection pass"
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."
2021-11-09 22:40:18 +01:00
serge-sans-paille 7f92a1a84b Misleading unicode identifier detection pass
Detect when an identifier contains some Right-To-Left characters.
This pass relates to https://trojansource.codes/

This is a recommit of 299aa4dfa1 with missing
option registration fixed.

Differential Revision: https://reviews.llvm.org/D112914
2021-11-09 21:46:35 +01:00
Jorge Gorbe Moya 74add1b6d6 Revert "[clang-tidy] Fix lint warning in ClangTidyDiagnosticConsumer.cpp (NFC)"
The change causes multiple clang-tidy tests to fail under ASan.

This reverts commit 0076957202.
2021-11-09 11:28:48 -08:00
Simon Pilgrim 5338629333 Revert rG299aa4dfa1d8c120648b1404b481d858b76c8173 "Misleading unicode identifier detection pass"
This is failing on various buildbots: https://lab.llvm.org/buildbot/#/builders/109/builds/25932
2021-11-09 18:25:55 +00:00
serge-sans-paille 299aa4dfa1 Misleading unicode identifier detection pass
Detect when an identifier contains some Right-To-Left characters.
This pass relates to https://trojansource.codes/

Differential Revision: https://reviews.llvm.org/D112914
2021-11-09 16:01:28 +01:00