[all-commits] [llvm/llvm-project] 71f2ec: [clang][dataflow] Add synthetic fields to `RecordS...

[martinboehme via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 71f2ec2db1295462d61e1407fcc1e715ba5d458b
      https://github.com/llvm/llvm-project/commit/71f2ec2db1295462d61e1407fcc1e715ba5d458b
  Author: martinboehme <mboehme at google.com>
  Date:   2023-12-04 (Mon, 04 Dec 2023)

  Changed paths:
    M clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
    M clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
    M clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
    M clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
    M clang/include/clang/Analysis/FlowSensitive/RecordOps.h
    M clang/include/clang/Analysis/FlowSensitive/StorageLocation.h
    M clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
    M clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
    M clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
    M clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
    M clang/lib/Analysis/FlowSensitive/RecordOps.cpp
    M clang/lib/Analysis/FlowSensitive/Transfer.cpp
    M clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
    M clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
    M clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
    M clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
    M clang/unittests/Analysis/FlowSensitive/TestingSupport.h
    M clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

  Log Message:
  -----------
  [clang][dataflow] Add synthetic fields to `RecordStorageLocation` (#73860)

Synthetic fields are intended to model the internal state of a class
(e.g. the value stored in a `std::optional`) without having to depend on
that class's implementation details.

Today, this is typically done with properties on `RecordValue`s, but
these have several drawbacks:

* Care must be taken to call `refreshRecordValue()` before modifying a
property so that the modified property values aren’t seen by other
environments that may have access to the same `RecordValue`.

* Properties aren’t associated with a storage location. If an analysis
needs to associate a location with the value stored in a property (e.g.
to model the reference returned by `std::optional::value()`), it needs
to manually add an indirection using a `PointerValue`. (See for example
the way this is done in UncheckedOptionalAccessModel.cpp, specifically
in `maybeInitializeOptionalValueMember()`.)

* Properties don’t participate in the builtin compare, join, and widen
operations. If an analysis needs to apply these operations to
properties, it needs to override the corresponding methods of
`ValueModel`.

* Longer-term, we plan to eliminate `RecordValue`, as by-value
operations on records aren’t really “a thing” in C++ (see
https://discourse.llvm.org/t/70086#changed-structvalue-api-14). This
would obviously eliminate the ability to set properties on
`RecordValue`s.

To demonstrate the advantages of synthetic fields, this patch converts
UncheckedOptionalAccessModel.cpp to synthetic fields. This greatly
simplifies the implementation of the check.

This PR is pretty big; to make it easier to review, I have broken it
down into a stack of three commits, each of which contains a set of
logically related changes. I considered submitting each of these as a
separate PR, but the commits only really make sense when taken together.

To review, I suggest first looking at the changes in
UncheckedOptionalAccessModel.cpp. This gives a flavor for how the
various API changes work together in the context of an analysis. Then,
review the rest of the changes.