Fix [-Werror,-Wsign-compare] error. NFC.

[clang][dataflow] Produce pointer values for callees of member operator calls.

Calls to member operators are a special case in that their callees have pointer
type. The callees of non-operator non-s

[clang][dataflow] Produce pointer values for callees of member operator calls.

Calls to member operators are a special case in that their callees have pointer
type. The callees of non-operator non-static member functions are not pointers.
See the comments in the code for details.

This issue came up in the Crubit nullability check; the fact that we weren't
modeling the `PointerValue` caused non-convergence.

Reviewed By: ymandel, xazax.hun

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

show more ...

[clang][dataflow] Add two repros for non-convergence involving pointers in loops.

These are broken out from https://reviews.llvm.org/D156658, which it now seems obvious isn't the right way to solve

[clang][dataflow] Add two repros for non-convergence involving pointers in loops.

These are broken out from https://reviews.llvm.org/D156658, which it now seems obvious isn't the right way to solve the non-convergence.

Instead, my plan is to address the non-convergence through pointer value widening, but the exact way this should be implemented is TBD. In the meantime, I think there's value in getting these repros submitted to record the current undesirable behavior.

Reviewed By: ymandel, xazax.hun

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

show more ...

[clang][dataflow] Rename `AggregateStorageLocation` to `RecordStorageLocation` and `StructValue` to `RecordValue`.

- Both of these constructs are used to represent structs, classes, and unions;
Cl

[clang][dataflow] Rename `AggregateStorageLocation` to `RecordStorageLocation` and `StructValue` to `RecordValue`.

- Both of these constructs are used to represent structs, classes, and unions;
Clang uses the collective term "record" for these.

- The term "aggregate" in `AggregateStorageLocation` implies that, at some
point, the intention may have been to use it also for arrays, but it don't
think it's possible to use it for arrays. Records and arrays are very
different and therefore need to be modeled differently. Records have a fixed
set of named fields, which can have different type; arrays have a variable
number of elements, but they all have the same type.

- Futhermore, "aggregate" has a very specific meaning in C++
(https://en.cppreference.com/w/cpp/language/aggregate_initialization).
Aggregates of class type may not have any user-declared or inherited
constructors, no private or protected non-static data members, no virtual
member functions, and so on, but we use `AggregateStorageLocations` to model all objects of class type.

In addition, for consistency, we also rename the following:

- `getAggregateLoc()` (in `RecordValue`, formerly known as `StructValue`) to
simply `getLoc()`.

- `refreshStructValue()` to `refreshRecordValue()`

We keep the old names around as deprecated synonyms to enable clients to be migrated to the new names.

Reviewed By: ymandel, xazax.hun

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

show more ...

[clang][dataflow] Reverse course on `getValue()` deprecation.

In the [value categories RFC](https://discourse.llvm.org/t/70086), I proposed that the end state of the migration should be that `getVal

[clang][dataflow] Reverse course on `getValue()` deprecation.

In the [value categories RFC](https://discourse.llvm.org/t/70086), I proposed that the end state of the migration should be that `getValue()` should only be legal to call on prvalues.

As a stepping stone, to allow migrating off existing calls to `getValue()`, I proposed introducing `getValueStrict()`, which would already have the new semantics.

However, I've now reconsidered this. Any expression, whether prvalue or glvalue, has a value, so really there isn't any reason to forbid calling `getValue()` on glvalues. I'm therefore removing the deprecation from `getValue()` and transitioning existing `getValueStrict()` calls back to `getValue()`.

The other "strict" accessors are a different case. `setValueStrict()` should only be called on prvalues because glvalues need to have a storage location associated with them; it doesn't make sense to only set a value for them. And, of course, `getStorageLocationStrict()` and `setStorageLocationStrict()` should obviously only be called on glvalues because prvalues don't have storage locations.

Reviewed By: ymandel, xazax.hun

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

show more ...

[clang][dataflow] Don't crash when constructing an array of records.

When I wrote https://reviews.llvm.org/D155446, I assumed that a `CXXConstructExpr` would always have record type, but this isn't

[clang][dataflow] Don't crash when constructing an array of records.

When I wrote https://reviews.llvm.org/D155446, I assumed that a `CXXConstructExpr` would always have record type, but this isn't true: It can have array type when constructing an array of records. The code would crash in this situation because `createValue()` would return null.

This patch includes a test that reproduces the crash without the other changes in the patch.

Reviewed By: sammccall

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

show more ...

[clang][dataflow] fix bug for transparent ListInitExpr handling

This fixes the handling of "transparent" ListInitExpr, when they're only
used as a copy constructor for records.

Without the fix, the

[clang][dataflow] fix bug for transparent ListInitExpr handling

This fixes the handling of "transparent" ListInitExpr, when they're only
used as a copy constructor for records.

Without the fix, the two tests are crashing the process.

show more ...

[clang][dataflow] Remove checks that test for consistency between `StructValue` and `AggregateStorageLocation`.

Now that the redundancy between these two classes has been eliminated, these
checks ar

[clang][dataflow] Remove checks that test for consistency between `StructValue` and `AggregateStorageLocation`.

Now that the redundancy between these two classes has been eliminated, these
checks aren't needed any more.

Reviewed By: ymandel, xazax.hun

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

show more ...

[clang][dataflow] Eliminate duplication between `AggregateStorageLocation` and `StructValue`.

After this change, `StructValue` is just a wrapper for an `AggregateStorageLocation`. For the wider cont

[clang][dataflow] Eliminate duplication between `AggregateStorageLocation` and `StructValue`.

After this change, `StructValue` is just a wrapper for an `AggregateStorageLocation`. For the wider context, see https://discourse.llvm.org/t/70086.

## How to review

- Start by looking at the comments added / changed in Value.h, StorageLocation.h,
and DataflowEnvironment.h. This will give you a good overview of the semantic
changes.

- Look at the corresponding .cpp files that implement the semantic changes.

- Transfer.cpp, TypeErasedDataflowAnalysis.cpp, and RecordOps.cpp show how the
core of the framework is affected by the semantic changes.

- UncheckedOptionalAccessModel.cpp shows how this complex model is affected by
the changes.

- Many of the changes in the rest of the patch are mechanical in nature.

Reviewed By: ymandel, xazax.hun

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

show more ...

[clang][dataflow] Strengthen flow condition assertions.

Instead of asserting merely that the flow condition doesn't imply that a variable is true, make the stronger assertion that the flow condition

[clang][dataflow] Strengthen flow condition assertions.

Instead of asserting merely that the flow condition doesn't imply that a variable is true, make the stronger assertion that the flow condition implies that the variable is false.

Reviewed By: ymandel, xazax.hun

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

show more ...

[clang][dataflow] Add a test for not explicitly initialized fields in aggregate initialization.

Reviewed By: ymandel

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

[dataflow] Remove deprecated BoolValue flow condition accessors

Use the Formula versions instead, now.

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

[clang][dataflow] Fix initializing a reference field with an `InitListExpr`.

I added a test for this as the ongoing migration to strict handling of value categories (see https://discourse.llvm.org/t

[clang][dataflow] Fix initializing a reference field with an `InitListExpr`.

I added a test for this as the ongoing migration to strict handling of value categories (see https://discourse.llvm.org/t/70086) will change the code that handles this case. It turns out we already didn't handle this correctly, so I fixed the existing implementation.

Depends On D154961

Reviewed By: xazax.hun

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

show more ...

[clang][dataflow] Include fields initialized in an `InitListExpr` in `getModeledFields()`.

Previously, we were including these fields only in the specific instance that was initialized by the `InitL

[clang][dataflow] Include fields initialized in an `InitListExpr` in `getModeledFields()`.

Previously, we were including these fields only in the specific instance that was initialized by the `InitListExpr`, but not in other instances of the same type. This is inconsistent and error-prone.

Depends On D154952

Reviewed By: xazax.hun, gribozavr2

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

show more ...

[clang][dataflow] Use `getFieldValue()` in TransferTest.cpp.

For context, see https://reviews.llvm.org/D154935.

Depends On D154935

Reviewed By: ymandel, xazax.hun

Differential Revision: https://r

[clang][dataflow] Use `getFieldValue()` in TransferTest.cpp.

For context, see https://reviews.llvm.org/D154935.

Depends On D154935

Reviewed By: ymandel, xazax.hun

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

show more ...

Reland "[dataflow] Add dedicated representation of boolean formulas"

This reverts commit 7a72ce98224be76d9328e65eee472381f7c8e7fe.

Test problems were due to unspecified order of function arg evalua

Reland "[dataflow] Add dedicated representation of boolean formulas"

This reverts commit 7a72ce98224be76d9328e65eee472381f7c8e7fe.

Test problems were due to unspecified order of function arg evaluation.

Reland "[dataflow] Replace most BoolValue subclasses with references to Formula (and AtomicBoolValue => Atom and BoolValue => Formula where appropriate)"

This properly frees the Value hierarchy from managing boolean formulas.

We still distinguish AtomicBoolValue; this type is used in client code.
However we expect to convert such uses to BoolValue (where the
distinction is not needed) or Atom (where atomic identity is intended),
and then fold AtomicBoolValue into FormulaBoolValue.

We also distinguish TopBoolValue; this has distinct rules for
widen/join/equivalence, and top-ness is not represented in Formula.
It'd be nice to find a cleaner representation (e.g. the absence of a
formula), but no immediate plans.

For now, BoolValues with the same Formula are deduplicated. This doesn't
seem desirable, as Values are mutable by their creators (properties).
We can probably drop this for FormulaBoolValue immediately (not in this
patch, to isolate changes). For AtomicBoolValue we first need to update
clients to stop using value pointers for atom identity.

The data structures around flow conditions are updated:
- flow condition tokens are Atom, rather than AtomicBoolValue*
- conditions are Formula, rather than BoolValue
Most APIs were changed directly, some with many clients had a
new version added and the existing one deprecated.

The factories for BoolValues in Environment keep their existing
signatures for now (e.g. makeOr(BoolValue, BoolValue) => BoolValue)
and are not deprecated. These have very many clients and finding the
most ergonomic API & migration path still needs some thought.

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

show more ...

[clang][dataflow] Bug fix: `BuiltinFnToFnPtr` cast does not produce a pointer.

See comments in the code for details.

Reviewed By: xazax.hun

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

Revert "Reland "[dataflow] Add dedicated representation of boolean formulas" and followups

These changes are OK, but they break downstream stuff that needs more time to adapt :-(

This reverts commi

Revert "Reland "[dataflow] Add dedicated representation of boolean formulas" and followups

These changes are OK, but they break downstream stuff that needs more time to adapt :-(

This reverts commit 71579569f4399d3cf6bc618dcd449b5388d749cc.
This reverts commit 5e4ad816bf100b0325ed45ab1cfea18deb3ab3d1.
This reverts commit 1c3ac8dfa16c42a631968aadd0396cfe7f7778e0.

show more ...

[dataflow] use true/false literals in formulas, rather than variables

And simplify formulas containing true/false
It's unclear to me how useful this is, it does make formulas more
conveniently self-

[dataflow] use true/false literals in formulas, rather than variables

And simplify formulas containing true/false
It's unclear to me how useful this is, it does make formulas more
conveniently self-contained now (we can usefully print them without
carrying around the "true/false" labels)

(while here, simplify !!X to X, too)

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

show more ...

[dataflow] Replace most BoolValue subclasses with references to Formula (and AtomicBoolValue => Atom and BoolValue => Formula where appropriate)

This properly frees the Value hierarchy from managing

[dataflow] Replace most BoolValue subclasses with references to Formula (and AtomicBoolValue => Atom and BoolValue => Formula where appropriate)

This properly frees the Value hierarchy from managing boolean formulas.

We still distinguish AtomicBoolValue; this type is used in client code.
However we expect to convert such uses to BoolValue (where the
distinction is not needed) or Atom (where atomic identity is intended),
and then fold AtomicBoolValue into FormulaBoolValue.

We also distinguish TopBoolValue; this has distinct rules for
widen/join/equivalence, and top-ness is not represented in Formula.
It'd be nice to find a cleaner representation (e.g. the absence of a
formula), but no immediate plans.

For now, BoolValues with the same Formula are deduplicated. This doesn't
seem desirable, as Values are mutable by their creators (properties).
We can probably drop this for FormulaBoolValue immediately (not in this
patch, to isolate changes). For AtomicBoolValue we first need to update
clients to stop using value pointers for atom identity.

The data structures around flow conditions are updated:
- flow condition tokens are Atom, rather than AtomicBoolValue*
- conditions are Formula, rather than BoolValue
Most APIs were changed directly, some with many clients had a
new version added and the existing one deprecated.

The factories for BoolValues in Environment keep their existing
signatures for now (e.g. makeOr(BoolValue, BoolValue) => BoolValue)
and are not deprecated. These have very many clients and finding the
most ergonomic API & migration path still needs some thought.

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

show more ...

[clang][dataflow] Make `runDataflowReturnError()` a non-template function.

It turns out this didn't need to be a template at all.

Likewise, change callers to they're non-template functions.

Also,

[clang][dataflow] Make `runDataflowReturnError()` a non-template function.

It turns out this didn't need to be a template at all.

Likewise, change callers to they're non-template functions.

Also, correct / clarify some comments in RecordOps.h.

This is in response to post-commit comments on https://reviews.llvm.org/D153006.

Reviewed By: gribozavr2

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

show more ...

[clang][dataflow] Add a test for a struct that is directly self-referential through a reference.

The ongoing migration to strict handling of value
categories (see https://discourse.llvm.org/t/70086)

[clang][dataflow] Add a test for a struct that is directly self-referential through a reference.

The ongoing migration to strict handling of value
categories (see https://discourse.llvm.org/t/70086) will change the way we
handle fields of reference type, and I want to put a test in place that makes
sure we continue to handle this special case correctly.

Depends On D154420

Reviewed By: gribozavr2, xazax.hun

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

show more ...

[clang][dataflow] Model variables / fields / funcs used in default initializers.

The newly added test fails without the other changes in this patch.

Reviewed By: sammccall, gribozavr2

Differential

[clang][dataflow] Model variables / fields / funcs used in default initializers.

The newly added test fails without the other changes in this patch.

Reviewed By: sammccall, gribozavr2

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

show more ...

[clang][dataflow] Don't crash when creating pointers to members.

The newly added tests crash without the other changes in this patch.

Reviewed By: sammccall, xazax.hun, gribozavr2

Differential Rev

[clang][dataflow] Don't crash when creating pointers to members.

The newly added tests crash without the other changes in this patch.

Reviewed By: sammccall, xazax.hun, gribozavr2

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

show more ...

[clang][dataflow] Make `getThisPointeeStorageLocation()` return an `AggregateStorageLocation`.

This avoids the need for casts at callsites.

Depends On D153852

Reviewed By: sammccall, xazax.hun, gr

[clang][dataflow] Make `getThisPointeeStorageLocation()` return an `AggregateStorageLocation`.

This avoids the need for casts at callsites.

Depends On D153852

Reviewed By: sammccall, xazax.hun, gribozavr2

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

show more ...