[clang][dataflow] Use a `PointerValue` for `value` property in optional checker.

The `ReferenceValue` class will be eliminated as part of the ongoing migration
to strict handling of value categories

[clang][dataflow] Use a `PointerValue` for `value` property in optional checker.

The `ReferenceValue` class will be eliminated as part of the ongoing migration
to strict handling of value categories (see https://discourse.llvm.org/t/70086
for details).

Reviewed By: gribozavr2

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

show more ...

[clang][dataflow] Fix a bug in handling of `operator->` for optional checker.

Prior to this patch, `operator->` was being handled like `operator*`: It was
associating a `Value` of type `T` with the

[clang][dataflow] Fix a bug in handling of `operator->` for optional checker.

Prior to this patch, `operator->` was being handled like `operator*`: It was
associating a `Value` of type `T` with the expression result (where `T` is the
template argument of the `optional<T>`). This is correct for `operator*`, which
returns a reference (of some flavor) to `T`, so that the result of the
`CXXOperatorCallExpr` is a glvalue of type `T`. However, `operator*` returns a
`T*`, so the result of the `CXXOperatorCallExpr` is a prvalue `T*`, which should
therefore be associated with `PointerValue` that in turn refers to a `T`.

I noticed this issue while working on the migration to strict handling of
value categories (see https://discourse.llvm.org/t/70086). The current behavior
also seems problematic more generally because it's plausible that the framework
may at some point introduce behavior that assumes an `Expr` of pointer type is
always associated with a `PointerValue`.

As it turns out, this patch fixes an existing FIXME in the test
`OptionalValueInitialization`.

Depends On D150657

Reviewed By: ymandel

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

show more ...

[clang][dataflow] Remove unused parameter from `diagnoseUnwrapCall()`.

Reviewed By: sammccall

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

[ClangTidy] Fix markup in comments

[clang][dataflow] Eliminate `SkipPast::ReferenceThenPointer`.

As a replacement, we provide the accessors `getImplicitObjectLocation()` and
`getBaseObjectLocation()`, which are higher-level construct

[clang][dataflow] Eliminate `SkipPast::ReferenceThenPointer`.

As a replacement, we provide the accessors `getImplicitObjectLocation()` and
`getBaseObjectLocation()`, which are higher-level constructs that cover the use
cases in which `SkipPast::ReferenceThenPointer` was typically used.

Unfortunately, it isn't possible to use these accessors in
UncheckedOptionalAccessModel.cpp; I've added a FIXME to the code explaining the
details. I initially attempted to resolve the issue as part of this patch, but
it turned out to be non-trivial to fix. Instead, I have therefore added a
lower-level replacement for `SkipPast::ReferenceThenPointer` that is used only
within this file.

The wider context of this change is that `SkipPast` will be going away entirely.
See also the RFC at https://discourse.llvm.org/t/70086.

Reviewed By: ymandel, gribozavr2

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

show more ...

[clang][dataflow][NFC] Eliminate unnecessary helper `stripReference()`.

`QualType::getNonReferenceType()` does the same thing.

Reviewed By: sammccall

Differential Revision: https://reviews.llvm.or

[clang][dataflow][NFC] Eliminate unnecessary helper `stripReference()`.

`QualType::getNonReferenceType()` does the same thing.

Reviewed By: sammccall

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

show more ...

[clang][dataflow] Drop optional model's dependency on libc++ internals.

Adjusts the matchers in the optional model to avoid dependency on internal
implementation details of libc++'s `std::optional`.

[clang][dataflow] Drop optional model's dependency on libc++ internals.

Adjusts the matchers in the optional model to avoid dependency on internal
implementation details of libc++'s `std::optional`. In the process, factors out
the code to check the name of these types so that it's shared throughout.

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

show more ...

[clang][dataflow] Refine matching of optional types to anchor at top level.

This patch refines the matching of the relevant optional types to anchor on the
global namespace. Previously, we could mat

[clang][dataflow] Refine matching of optional types to anchor at top level.

This patch refines the matching of the relevant optional types to anchor on the
global namespace. Previously, we could match anything with the right name
(e.g. `base::Optional`) even if nested within other namespaces. This over
matching resulted in an assertion violation when _different_ `base::Optional`
was encountered nested inside another namespace.

Fixes issue #57036.

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

show more ...

[clang-tidy] Allow bugprone-unchecked-optional-access to handle calls to `std::forward`

The check now understands that calling `std::forward`
will not modify the underlying optional value.

This fix

[clang-tidy] Allow bugprone-unchecked-optional-access to handle calls to `std::forward`

The check now understands that calling `std::forward`
will not modify the underlying optional value.

This fixes llvm#59705

Reviewed By: PiotrZSL

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

show more ...

[clang][dataflow] Add `create<T>()` methods to `Environment` and `DataflowAnalysisContext`.

These methods provide a less verbose way of allocating `StorageLocation`s and
`Value`s than the existing `

[clang][dataflow] Add `create<T>()` methods to `Environment` and `DataflowAnalysisContext`.

These methods provide a less verbose way of allocating `StorageLocation`s and
`Value`s than the existing `takeOwnership(make_unique(...))` pattern.

In addition, because allocation of `StorageLocation`s and `Value`s now happens
within the `DataflowAnalysisContext`, the `create<T>()` open up the possibility
of using `BumpPtrAllocator` to allocate these objects if it turns out this
helps performance.

Reviewed By: ymandel, xazax.hun, gribozavr2

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

show more ...

[clang][dataflow] Change `transfer` API to take a reference.

The provided `CFGElement` is never null, so a reference is a more precise type.

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

[clang][dataflow] Relax validity assumptions in `UncheckedOptionalAccessModel`.

Currently, the interpretation of `swap` calls in the optional model assumes the
optional arguments are modeled (and th

[clang][dataflow] Relax validity assumptions in `UncheckedOptionalAccessModel`.

Currently, the interpretation of `swap` calls in the optional model assumes the
optional arguments are modeled (and therefore have valid storage locations and
values). This assumption is incorrect, for example, in the case of unmodeled
optional fields (which can be missing either value or location). This patch
relaxes these assumptions, to return rather than assert when either argument is
not modeled.

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

show more ...

[clang] Use std::optional instead of llvm::Optional (NFC)

This patch replaces (llvm::|)Optional< with std::optional<. I'll post
a separate patch to remove #include "llvm/ADT/Optional.h".

This is p

[clang] Use std::optional instead of llvm::Optional (NFC)

This patch replaces (llvm::|)Optional< with std::optional<. I'll post
a separate patch to remove #include "llvm/ADT/Optional.h".

This is part of an effort to migrate from llvm::Optional to
std::optional:

https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716

show more ...

[clang] Add #include <optional> (NFC)

This patch adds #include <optional> to those files containing
llvm::Optional<...> or Optional<...>.

I'll post a separate patch to actually replace llvm::Option

[clang] Add #include <optional> (NFC)

This patch adds #include <optional> to those files containing
llvm::Optional<...> or Optional<...>.

I'll post a separate patch to actually replace llvm::Optional with
std::optional.

This is part of an effort to migrate from llvm::Optional to
std::optional:

https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716

show more ...

[clang][dataflow] In optional model, implement `widen` and make `compare` sound.

This patch includes two related changes:

1. Rewrite `compare` operation to be sound. Current version checks for equa

[clang][dataflow] In optional model, implement `widen` and make `compare` sound.

This patch includes two related changes:

1. Rewrite `compare` operation to be sound. Current version checks for equality
of `isNonEmptyOptional` on both values, judging the values `Same` when the
results are equal. While that works when both are true, it is problematic when
they are both false, because there are four cases in which that's can occur:
both empty, one empty and one unknown (which is two cases), and both unknown. In
the latter three cases, it is unsound to judge them `Same`. This patch changes
`compare` to explicitly check for case of `both empty` and then judge any other
case `Different`.

2. With the change to `compare`, a number of common cases will no longer
terminate. So, we also implement widening to properly handle those cases and
recover termination.

Drive-by: improve performance of `merge` operation.

Of the new tests, the code before the patch fails
* ReassignValueInLoopToSetUnsafe, and
* ReassignValueInLoopToUnknownUnsafe.

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

show more ...

[clang][dataflow] Fix bug in optional-checker's handling of nullopt constructor.

Currently, the checker only recognizes the nullopt constructor when it is called
without sugar, resulting in a crash

[clang][dataflow] Fix bug in optional-checker's handling of nullopt constructor.

Currently, the checker only recognizes the nullopt constructor when it is called
without sugar, resulting in a crash in the (rare) case where it has been wrapped
in sugar. This relaxes the constraint by checking the constructor decl directly
(which always contains the same, desugared form) rather than the construct
expression (where the spelling depends on the context).

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

show more ...

[clang][dataflow] Simplify handling of nullopt-optionals.

Previously, in the case of an optional constructed from `nullopt`, we relied on
the value constructed for the `nullopt`. This complicates th

[clang][dataflow] Simplify handling of nullopt-optionals.

Previously, in the case of an optional constructed from `nullopt`, we relied on
the value constructed for the `nullopt`. This complicates the implementation and
exposes it to bugs (indeed, one such was found), yet doesn't improve the
engine. Instead, this patch constructs a fresh optional representation, rather
than relying on the underlying nullopt representation.

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

show more ...

[clang][dataflow] Improve optional model's support for ignoring smart pointers.

The optional model has an option to ignore optionals accessed through smart
pointer types (other than optional itself)

[clang][dataflow] Improve optional model's support for ignoring smart pointers.

The optional model has an option to ignore optionals accessed through smart
pointer types (other than optional itself). This patch improves this feature in
two ways:

1. We extend support to optionals accessed directly through the smart pointer,
like `ptr->value()`. Previously, support was limited to cases that went through
an intermediate field.

2. We clean up the implementation by removing the option from the analysis,
leaving it only in the diagnostic phase (where it is relevant).

3. Adjusts a test which was misleading in what it was testing.

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

show more ...

[clang][dataflow] Support (in)equality operators in `optional` model.

This patch adds interpretation of the overloaded equality and inequality
operators available for the optional types.

Fixes issu

[clang][dataflow] Support (in)equality operators in `optional` model.

This patch adds interpretation of the overloaded equality and inequality
operators available for the optional types.

Fixes issue #57253.

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

show more ...

[Analysis] Use std::nullopt instead of None (NFC)

This patch mechanically replaces None with std::nullopt where the
compiler would warn if None were deprecated. The intent is to reduce
the amount o

[Analysis] Use std::nullopt instead of None (NFC)

This patch mechanically replaces None with std::nullopt where the
compiler would warn if None were deprecated. The intent is to reduce
the amount of manual work required in migrating from Optional to
std::optional.

This is part of an effort to migrate from llvm::Optional to
std::optional:

https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716

show more ...

[clang][dataflow] Generalize custom comparison to return tri-value result.

Currently, the API for a model's custom value comparison returns a
boolean. Therefore, models cannot distinguish between si

[clang][dataflow] Generalize custom comparison to return tri-value result.

Currently, the API for a model's custom value comparison returns a
boolean. Therefore, models cannot distinguish between situations where the
values are recognized by the model and different and those where the values are
just not recognized. This patch changes the return value to a tri-valued enum,
allowing models to express "don't know".

This patch is essentially a NFC -- no practical differences result from this
change in this patch. But, it prepares for future patches (particularly,
upcoming patches for widening) which will take advantage of the new flexibility.

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

show more ...

[clang][dataflow] Replace usage of deprecated functions with the optional check

- Update `transfer` and `diagnose` to take `const CFGElement *` as input in `Analysis/FlowSensitive/Models/UncheckedOp

[clang][dataflow] Replace usage of deprecated functions with the optional check

- Update `transfer` and `diagnose` to take `const CFGElement *` as input in `Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel`.
- Update `clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp` accordingly.
- Rename `runDataflowAnalysisOnCFG` to `runDataflowAnalysis` and remove the deprecated `runDataflowAnalysis` (this was only used by the now updated optional check).

Reviewed By: gribozavr2, sgatev

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

show more ...

Revert "[clang][dataflow] Replace `transfer(const Stmt *, ...)` with `transfer(const CFGElement *, ...)` in `Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel`."

This reverts commit 41f235d

Revert "[clang][dataflow] Replace `transfer(const Stmt *, ...)` with `transfer(const CFGElement *, ...)` in `Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel`."

This reverts commit 41f235d26887946f472d71a8417507c35d5f9074.

Details at https://lab.llvm.org/buildbot#builders/139/builds/28171.
Breakage due to API change.

show more ...

[clang][dataflow] Replace `transfer(const Stmt *, ...)` with `transfer(const CFGElement *, ...)` in `Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel`.

Reviewed By: gribozavr2, sgatev

Dif

[clang][dataflow] Replace `transfer(const Stmt *, ...)` with `transfer(const CFGElement *, ...)` in `Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel`.

Reviewed By: gribozavr2, sgatev

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

show more ...

[clang][dataflow] Use NoopLattice in optional model

Followup to D128352. This patch pulls the `NoopLattice` class out from the `NoopAnalysis.h` test file into its own `NoopLattice.h` source file, an

[clang][dataflow] Use NoopLattice in optional model

Followup to D128352. This patch pulls the `NoopLattice` class out from the `NoopAnalysis.h` test file into its own `NoopLattice.h` source file, and uses it to replace usage of `SourceLocationsLattice` in `UncheckedOptionalAccessModel`.

Reviewed By: ymandel, sgatev, gribozavr2, xazax.hun

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

show more ...