I cannot minimal produce it, but when I run clangd with
`misc-const-correctness` check in a big project
clangd crashed due to deref nullptr here.
clang may pass a nullptr to `InitExprs` when meets some error cases.
It is hard to understand for `ExprMutationAnalyzer::isUnevaluated` to
accept 2 Stmt as parameters.
This patch wants to redesign the API to accept only 1 Stmt. Now it will
only check whether stmt is a sub-stmt of an unevaluated stmt.
Summary:
If callExpr is type dependent, there is no way to analyze individual
arguments until template specialization. Before this diff only calls
with dependent callees were skipped so unnecessary-value-param was
processing arguments that had non-dependent type that gave false
positives because the call was not fully resolved till specialization.
So now instead of checking type dependent callee, the whole expression
will be checked for type dependent.
Test Plan: check-clang-tools
The check no longer emits a diagnostic for non-parameter-pack
variables in C++17 fold expressions.
The operator used is type-dependent because of the parameter pack
and can therefore not be guaranteed to not mutate the variable.
Fixes: #70323
Replaced certain AST matchers in ExprMutationAnalyzer with a more direct
utilization of AST classes. The primary bottleneck was identified in the
canResolveToExpr AST matcher. Since this matcher was employed multiple
times and used recursively, each invocation led to the constant creation
and destruction of other matchers within it. Additionally, the continual
comparison of DynTypedNode resulted in significant performance
degradation.
The optimization was tested on the TargetLowering.cpp file. Originally,
the check took 156 seconds on that file, but after implementing this
enhancement, it now takes approximately 40 seconds, making it nearly
four times faster.
Despite this improvement, there are still numerous issues in this file.
To further reduce the computational cost of this class, it is advisable
to consider removing the remaining matchers and exploring alternatives
such as leveraging RecursiveASTVisitor and increasing the direct use of
AST classes.
Closes#71786
The `ExprMutationAnalyzer`s matcher of `binaryOperator`s
that contained the variable expr, were previously narrowing the
variable to be type dependent, when the `binaryOperator` should
have been narrowed as dependent.
The variable we are trying to find mutations for does
not need to be the dependent type, the other operand of
the `binaryOperator` could be dependent.
Fixes#57297
_Generic accepts an expression operand whose type is matched against a
list of associations. The expression operand is unevaluated, but the
type matched is the type after lvalue conversion. This conversion loses
type information, which makes it more difficult to match against
qualified or incomplete types.
This extension allows _Generic to accept a type operand instead of an
expression operand. The type operand form does not undergo any
conversions and is matched directly against the association list.
This extension is also supported in C++ as we already supported
_Generic selection expressions there.
The RFC for this extension can be found at:
https://discourse.llvm.org/t/rfc-generic-selection-expression-with-a-type-operand/70388
Differential Revision: https://reviews.llvm.org/D149904
This patch connects the check for const-correctness with the new general
utility to add `const` to variables.
The code-transformation is only done, if the detected variable for const-ness
is not part of a group-declaration.
The check allows to control multiple facets of adding `const`, e.g. if pointers themself should be
marked as `const` if they are not changed.
Reviewed By: njames93
Differential Revision: https://reviews.llvm.org/D54943
This allows ASTs to be merged when they contain GenericSelectionExpr
nodes (this is _Generic from C11). This is needed, for example, for
CTU analysis of C code that makes use of _Generic, like the Linux
kernel.
The node is already supported in the AST, but it didn't have a matcher
in ASTMatchers. So, this change adds the matcher and adds support to
ASTImporter. Additionally, this change adds support for structural
equivalence of _Generic in the AST.
Reviewed By: martong, aaron.ballman
Differential Revision: https://reviews.llvm.org/D92600
This patch extracts the ExprMutAnalyzer changes from https://reviews.llvm.org/D54943
into its own revision for simpler review and more atomic changes.
The analysis results are improved. Nested expressions (e.g. conditional
operators) are now detected properly. Some edge cases, especially
template induced imprecisions are improved upon.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D88088
We should track mutation of a variable within a comma operator expression.
Current code in ExprMutationAnalyzer does not handle it.
This will handle cases like:
(a, b) ++ < == b is modified
(a, b) = c < == b is modifed
Patch by Djordje Todorovic.
Differential Revision: https://reviews.llvm.org/D58894
llvm-svn: 355605
to reflect the new license.
We understand that people may be surprised that we're moving the header
entirely to discuss the new license. We checked this carefully with the
Foundation's lawyer and we believe this is the correct approach.
Essentially, all code in the project is now made available by the LLVM
project under our new license, so you will see that the license headers
include that license only. Some of our contributors have contributed
code under our old license, and accordingly, we have retained a copy of
our old license notice in the top-level files in each project and
repository.
llvm-svn: 351636
Summary:
This is a follow up of D52008 and should make the analyzer being able to handle perfect forwardings in real world cases where forwardings are done through multiple layers of function calls with `std::forward`.
Fixes PR38891.
Reviewers: lebedev.ri, JonasToth, george.karpenkov
Subscribers: xazax.hun, szepet, a.sidorin, mikhail.ramalho, Szelethus, cfe-commits
Differential Revision: https://reviews.llvm.org/D52120
llvm-svn: 342409
Summary:
We used to treat an `Expr` mutated whenever it's passed as non-const
reference argument to a function. This results in false positives in
cases like this:
```
int x;
std::vector<int> v;
v.emplace_back(x); // `x` is passed as non-const reference to `emplace_back`
```
In theory the false positives can be suppressed with
`v.emplace_back(std::as_const(x))` but that's considered overly verbose,
inconsistent with existing code and spammy as diags.
This diff handles such cases by following into the function definition
and see whether the argument is mutated inside.
Reviewers: lebedev.ri, JonasToth, george.karpenkov
Subscribers: xazax.hun, szepet, a.sidorin, mikhail.ramalho, Szelethus, cfe-commits
Differential Revision: https://reviews.llvm.org/D52008
llvm-svn: 342271
Summary:
This is 1/2 of moving ExprMutationAnalyzer from clangtidy to
clang/Analysis.
This diff along simply copies the ExprMutationAnalyzer over with trivial
modifications (e.g. include path, namespace)
2/2 will migrate existing usage of ExprMutationAnalyzer and remove the
original copy inside clangtidy.
Reviewers: george.karpenkov
Subscribers: mgorny, xazax.hun, szepet, a.sidorin, mikhail.ramalho, Szelethus, cfe-commits, JonasToth
Differential Revision: https://reviews.llvm.org/D51948
llvm-svn: 341994