[clang][ASTVisitor] Visit `HoldingVar` from `BindingDecl`. (#117858)

Tuple-like types introduce `VarDecl`s in the AST for their "holding
vars", but AST visitors do not visit those. As a result the

[clang][ASTVisitor] Visit `HoldingVar` from `BindingDecl`. (#117858)

Tuple-like types introduce `VarDecl`s in the AST for their "holding
vars", but AST visitors do not visit those. As a result the `VarDecl`
for the holding var is orphaned when trying to retreive its parents.

Fix a `FlowSensitive` test that assumes that only a `BindingDecl` is
introduced with the given name (the matcher now can also reach the
`VarDecl` for the holding var).

show more ...

[clang][dataflow] reland #96766 with fix (#98896)

- **Reapply "[clang][dataflow] Teach `AnalysisASTVisitor` that
`typeid()` can be evaluated." (#96766)**
- **Turn on RTTI explicitly in `checkDataflo

[clang][dataflow] reland #96766 with fix (#98896)

- **Reapply "[clang][dataflow] Teach `AnalysisASTVisitor` that
`typeid()` can be evaluated." (#96766)**
- **Turn on RTTI explicitly in `checkDataflowWithNoopAnalysis()`.**

show more ...

Revert "[clang][dataflow] Teach `AnalysisASTVisitor` that `typeid()` can be evaluated." (#96766)

Reverts llvm/llvm-project#96731

It causes CI failures.

[clang][nullability] Improve modeling of `++`/`--` operators. (#96601)

We definitely know that these operations change the value of their
operand, so
clear out any value associated with it. We don't

[clang][nullability] Improve modeling of `++`/`--` operators. (#96601)

We definitely know that these operations change the value of their
operand, so
clear out any value associated with it. We don't create a new value,
instead
leaving it to the analysis to do this if desired.

show more ...

[clang][dataflow] Teach `AnalysisASTVisitor` that `typeid()` can be evaluated. (#96731)

We were previously treating the operand of `typeid()` as being
definitely
unevaluated, but it can be evaluated

[clang][dataflow] Teach `AnalysisASTVisitor` that `typeid()` can be evaluated. (#96731)

We were previously treating the operand of `typeid()` as being
definitely
unevaluated, but it can be evaluated if it is a glvalue of polymorphic
type.

This patch includes a test that fails without the fix.

show more ...

[Clang] [Sema] Diagnose unknown std::initializer_list layout in SemaInit (#95580)

This checks if the layout of `std::initializer_list` is something Clang
can handle much earlier and deduplicates th

[Clang] [Sema] Diagnose unknown std::initializer_list layout in SemaInit (#95580)

This checks if the layout of `std::initializer_list` is something Clang
can handle much earlier and deduplicates the checks in
CodeGen/CGExprAgg.cpp and AST/ExprConstant.cpp

Also now diagnose `union initializer_list` (Fixes #95495), bit-field for
the size (Fixes a crash that would happen during codegen if it were
unnamed), base classes (that wouldn't be initialized) and polymorphic
classes (whose vtable pointer wouldn't be initialized).

show more ...

[clang][dataflow] Handle `AtomicExpr` in `ResultObjectVisitor`. (#94963)

This is one of the node kinds that should be considered an "original
initializer". The patch adds a test that was causing an

[clang][dataflow] Handle `AtomicExpr` in `ResultObjectVisitor`. (#94963)

This is one of the node kinds that should be considered an "original
initializer". The patch adds a test that was causing an assertion
failure in
`assert(Children.size() == 1)` without the fix.

show more ...

[clang][dataflow] Propagate storage location of compound assignment operators. (#94332)

To avoid generating unnecessary values, we don't create a new value but
instead
leave it to the specific analy

[clang][dataflow] Propagate storage location of compound assignment operators. (#94332)

To avoid generating unnecessary values, we don't create a new value but
instead
leave it to the specific analysis to do this if desired.

show more ...

[clang][nullability] Propagate storage location / value of `++`/`--` operators. (#94217)

To avoid generating unnecessary values, we don't create a new value but
instead
leave it to the specific anal

[clang][nullability] Propagate storage location / value of `++`/`--` operators. (#94217)

To avoid generating unnecessary values, we don't create a new value but
instead
leave it to the specific analysis to do this if desired.

show more ...

[Clang][Sema] Fix malformed AST for anonymous class access in template. (#90842)

# Observed erroneous behavior

Prior to this change, a `MemberExpr` that accesses an anonymous class
might have a prv

[Clang][Sema] Fix malformed AST for anonymous class access in template. (#90842)

# Observed erroneous behavior

Prior to this change, a `MemberExpr` that accesses an anonymous class
might have a prvalue as its base (even though C++ mandates that the base
of a `MemberExpr` must be a glvalue), if the code containing the
`MemberExpr` was in a template.

Here's an example on [godbolt](https://godbolt.org/z/Gz1Mer9oz) (that is
essentially identical to the new test this patch adds).

This example sets up a struct containing an anonymous struct:

```cxx
struct S {
struct {
int i;
};
};
```

It then accesses the member `i` using the expression `S().i`.

When we do this in a non-template function, we get the following AST:

```
`-ExprWithCleanups <col:10, col:14> 'int'
`-ImplicitCastExpr <col:10, col:14> 'int' <LValueToRValue>
`-MemberExpr <col:10, col:14> 'int' xvalue .i 0xbdcb3c0
`-MemberExpr <col:10, col:14> 'S::(anonymous struct at line:2:3)' xvalue .S::(anonymous struct at line:2:3) 0xbdcb488
`-MaterializeTemporaryExpr <col:10, col:12> 'S' xvalue
`-CXXTemporaryObjectExpr <col:10, col:12> 'S' 'void () noexcept' zeroing
```

As expected, the AST contains a `MaterializeTemporarExpr` to materialize
the prvalue `S()` before accessing its members.

When we perform this access in a function template (that doesn't
actually even use its template parameter), the AST for the template
itself looks the same as above. However, the AST for an instantiation of
the template looks different:

```
`-ExprWithCleanups <col:10, col:14> 'int'
`-ImplicitCastExpr <col:10, col:14> 'int' <LValueToRValue>
`-MemberExpr <col:10, col:14> 'int' xvalue .i 0xbdcb3c0
`-MaterializeTemporaryExpr <col:10, col:14> 'S::(anonymous struct at line:2:3)' xvalue
`-MemberExpr <col:10, col:14> 'S::(anonymous struct at line:2:3)' .S::(anonymous struct at line:2:3) 0xbdcb488
`-CXXTemporaryObjectExpr <col:10, col:12> 'S' 'void () noexcept' zeroing
```

Note how the inner `MemberExpr` (the one accessing the anonymous struct)
acts on a prvalue.

Interestingly, this does not appear to cause any problems for CodeGen,
probably because CodeGen is set up to deal with `MemberExpr`s on rvalues
in C. However, it does cause issues in the dataflow framework, which
only supports C++ today and expects the base of a `MemberExpr` to be a
glvalue.

Beyond the issues with the dataflow framework, I think this issue should
be fixed because it goes contrary to what the C++ standard mandates, and
the AST produced for the non-template case indicates that we want to
follow the C++ rules here.

# Reasons for erroneous behavior

Here's why we're getting this malformed AST.

First of all, `TreeTransform` [strips any
`MaterializeTemporaryExpr`s](https://github.com/llvm/llvm-project/blob/cd132dcbeb0fc79fd657bd5e0a8e9244c3fb5da6/clang/lib/Sema/TreeTransform.h#L14853)
from the AST.

It is therefore up to
[`TreeTransform::RebuildMemberExpr()`](https://github.com/llvm/llvm-project/blob/cd132dcbeb0fc79fd657bd5e0a8e9244c3fb5da6/clang/lib/Sema/TreeTransform.h#L2853)
to recreate a `MaterializeTemporaryExpr` if needed. In the [general
case](https://github.com/llvm/llvm-project/blob/cd132dcbeb0fc79fd657bd5e0a8e9244c3fb5da6/clang/lib/Sema/TreeTransform.h#L2915),
it does this: It calls `Sema::BuildMemberReferenceExpr()`, which ensures
that the base is a glvalue by [materializing a
temporary](https://github.com/llvm/llvm-project/blob/cd132dcbeb0fc79fd657bd5e0a8e9244c3fb5da6/clang/lib/Sema/SemaExprMember.cpp#L1016)
if needed. However, when `TreeTransform::RebuildMemberExpr()` encounters
an anonymous class, it [calls
`Sema::BuildFieldReferenceExpr()`](https://github.com/llvm/llvm-project/blob/cd132dcbeb0fc79fd657bd5e0a8e9244c3fb5da6/clang/lib/Sema/TreeTransform.h#L2880),
which, unlike `Sema::BuildMemberReferenceExpr()`, does not make sure
that the base is a glvalue.

# Proposed fix

I considered several possible ways to fix this issue:

- Add logic to `Sema::BuildFieldReferenceExpr()` that materializes a
temporary if needed. This appears to work, but it feels like the fix is
in the wrong place:
- AFAIU, other callers of `Sema::BuildFieldReferenceExpr()` don't need
this logic.
- The issue is caused by `TreeTransform` removing the
`MaterializeTemporaryExpr`, so it seems the fix should also be in
`TreeTransform`

- Materialize the temporary directly in
`TreeTransform::RebuildMemberExpr()` if needed (within the case that
deals with anonymous classes).

This would work, too, but it would duplicate logic that already exists
in `Sema::BuildMemberReferenceExpr()` (which we leverage for the general
case).

- Use `Sema::BuildMemberReferenceExpr()` instead of
`Sema::BuildFieldReferenceExpr()` for the anonymous class case, so that
it also uses the existing logic for materializing the temporary.

This is the option I've decided to go with here. There's a slight
wrinkle in that we create a `LookupResult` that claims we looked up the
unnamed field for the anonymous class -- even though we would obviously
never be able to look up an unnamed field. I think this is defensible
and still better than the other alternatives, but I would welcome
feedback on this from others who know the code better.

show more ...

[clang][dataflow] Strengthen pointer comparison. (#75170)

- Instead of comparing the identity of the `PointerValue`s, compare the
underlying `StorageLocation`s.

- If the `StorageLocation`s a

[clang][dataflow] Strengthen pointer comparison. (#75170)

- Instead of comparing the identity of the `PointerValue`s, compare the
underlying `StorageLocation`s.

- If the `StorageLocation`s are the same, return a definite "true" as
the
result of the comparison. Before, if the `PointerValue`s were different,
we
would return an atom, even if the storage locations themselves were the
same.

- If the `StorageLocation`s are different, return an atom (as before).
Pointers
that have different storage locations may still alias, so we can't
return a
definite "false" in this case.

The application-level gains from this are relatively modest. For the
Crubit
nullability check running on an internal codebase, this change reduces
the
number of functions on which the SAT solver times out from 223 to 221;
the
number of "pointer expression not modeled" errors reduces from 3815 to
3778.

Still, it seems that the gain in precision is generally worthwhile.

@Xazax-hun inspired me to think about this with his

[comments](https://github.com/llvm/llvm-project/pull/73860#pullrequestreview-1761484615)
on a different PR.

show more ...

[clang][dataflow] Don't propagate result objects in unevaluated contexts (reland #90438) (#91172)

This relands #90348 with a fix for a [buildbot
failure](https://lab.llvm.org/buildbot/#/builders/216

[clang][dataflow] Don't propagate result objects in unevaluated contexts (reland #90438) (#91172)

This relands #90348 with a fix for a [buildbot
failure](https://lab.llvm.org/buildbot/#/builders/216/builds/38446)
caused by the test being run with `-fno-rtti`.

show more ...

[clang][dataflow] Fix crash when `operator=` result type is not destination type. (#90898)

The existing code was full of comments about how we assume this is
always the
case, but it's not mandated b

[clang][dataflow] Fix crash when `operator=` result type is not destination type. (#90898)

The existing code was full of comments about how we assume this is
always the
case, but it's not mandated by the standard, and there is code out there
that
returns a different type. So check that the result type is in fact the
same as
the destination type before attempting to copy to the result.

To make sure that we don't bail out in more cases than intended, I've
extended
existing tests to verify that in the common case, we do return the
destination
object (by reference or value, as the case may be).

show more ...

Revert "[clang][dataflow] Don't propagate result objects in unevaluated contexts (#90438)"

This reverts commit 597a3150e932a9423c65b5ea4b53dd431aff5865.

Caused test failure on the following buildbo

Revert "[clang][dataflow] Don't propagate result objects in unevaluated contexts (#90438)"

This reverts commit 597a3150e932a9423c65b5ea4b53dd431aff5865.

Caused test failure on the following buildbot:
https://lab.llvm.org/buildbot/#/builders/216/builds/38446

show more ...

[clang][dataflow] Don't propagate result objects in unevaluated contexts (#90438)

Trying to do so can cause crashes -- see newly added test and the
comments in
the fix.

We're starting to see a

[clang][dataflow] Don't propagate result objects in unevaluated contexts (#90438)

Trying to do so can cause crashes -- see newly added test and the
comments in
the fix.

We're starting to see a repeating pattern here: We're getting crashes
because
`ResultObjectVisitor` and `getReferencedDecls()` don't agree on which
parts of
the AST to visit and, hence, which fields should be modeled.

I think we should ensure consistency between these two parts of the code
by
using a `RecursiveASTVisitor` in `getReferencedDecls()`[^1]; the
`Traverse...()` functions that control which parts of the AST we visit
would go
in a common base class that would be used for both `ResultObjectVisitor`
and
`getReferencedDecls()`.

I'd like to focus this PR, however, on a targeted fix for the current
crash and
postpone the refactoring to a later PR (which will be easier to revert
if there
are unintended side-effects).

[^1]: As an added bonus, this would make the code better structured and
more
efficient than the current sequence of `if (dyn_cast<T>(...))`
statements).

show more ...

[clang][dataflow] Fix crash when `ConstantExpr` is used in conditional operator. (#90112)

`ConstantExpr` does not appear as a `CFGStmt` in the CFG, so
`StmtToEnvMap::getEnvironment()` was not findin

[clang][dataflow] Fix crash when `ConstantExpr` is used in conditional operator. (#90112)

`ConstantExpr` does not appear as a `CFGStmt` in the CFG, so
`StmtToEnvMap::getEnvironment()` was not finding an entry for it in the
map,
causing a crash when we tried to access the iterator resulting from the
map
lookup.

The fix is to make `ignoreCFGOmittedNodes()` ignore `ConstantExpr`, but
in
addition, I'm hardening `StmtToEnvMap::getEnvironment()` to make sure
release
builds don't crash in similar situations in the future.

show more ...

[clang][dataflow] Crash fix for `widenDistinctValues()`. (#89895)

We used to crash if the previous iteration contained a `BoolValue` and
the
current iteration contained an `IntegerValue`. The accomp

[clang][dataflow] Crash fix for `widenDistinctValues()`. (#89895)

We used to crash if the previous iteration contained a `BoolValue` and
the
current iteration contained an `IntegerValue`. The accompanying test
sets up
this situation -- see comments there for details.

While I'm here, clean up the tests for integral casts to use the test
helpers we
have available now. I was looking at these tests to understand how we
handle
integral casts, and the test helpers make the tests easier to read.

show more ...

[clang][dataflow] Don't propagate result objects in nested declarations. (#89903)

Trying to do so can cause crashes -- see newly added test and the
comments in
the fix.

Reapply "[clang][dataflow] Model conditional operator correctly." with fixes (#89596)

I reverted https://github.com/llvm/llvm-project/pull/89213 beause it was
causing buildbots to fail with assertio

Reapply "[clang][dataflow] Model conditional operator correctly." with fixes (#89596)

I reverted https://github.com/llvm/llvm-project/pull/89213 beause it was
causing buildbots to fail with assertion failures.

Embarrassingly, it turns out I had been running tests locally in
`Release` mode, i.e. with `assert()` compiled away.

This PR re-lands #89213 with fixes for the failing assertions.

show more ...

Revert "[clang][dataflow] Model conditional operator correctly." (#89577)

Reverts llvm/llvm-project#89213

This is causing buildbot failures.

[clang][dataflow] Model conditional operator correctly. (#89213)

[clang][nullability] Remove `RecordValue`. (#89052)

This class no longer serves any purpose; see also the discussion here:
https://reviews.llvm.org/D155204#inline-1503204

A lot of existing tests in

[clang][nullability] Remove `RecordValue`. (#89052)

This class no longer serves any purpose; see also the discussion here:
https://reviews.llvm.org/D155204#inline-1503204

A lot of existing tests in TransferTest.cpp check for the existence of
`RecordValue`s. Some of these checks are now simply redundant and have
been
removed. In other cases, tests were checking for the existence of a
`RecordValue` as a way of testing whether a record has been initialized.
I have
typically changed these test to instead check whether a field of the
record has
a value.

show more ...

[clang][dataflow] Support `CXXParenListInitExpr` in `PropagateResultObject()`. (#89235)

[clang][dataflow] Treat `BuiltinBitCastExpr` correctly in `PropagateResultObject()`. (#88875)

This patch includes a test that assert-fails without the fix.

[clang][dataflow] Support `StmtExpr` in `PropagateResultObject()`. (#88872)

This patch adds a test that assert-fails without the fix.