[all-commits] [llvm/llvm-project] bfbe13: [clang][dataflow] Eliminate intermediate `Referenc...

[martinboehme via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: bfbe137888151dfd506df6b3319d08c4de0e00f5
      https://github.com/llvm/llvm-project/commit/bfbe137888151dfd506df6b3319d08c4de0e00f5
  Author: Martin Braenne <mboehme at google.com>
  Date:   2023-05-04 (Thu, 04 May 2023)

  Changed paths:
    M clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
    M clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
    M clang/lib/Analysis/FlowSensitive/Transfer.cpp
    M clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
    M clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

  Log Message:
  -----------
  [clang][dataflow] Eliminate intermediate `ReferenceValue`s from `Environment::DeclToLoc`.

For the wider context of this change, see the RFC at
https://discourse.llvm.org/t/70086.

After this change, global and local variables of reference type are associated
directly with the `StorageLocation` of the referenced object instead of the
`StorageLocation` of a `ReferenceValue`.

Some tests that explicitly check for an existence of `ReferenceValue` for a
variable of reference type have been modified accordingly.

As discussed in the RFC, I have added an assertion to `Environment::join()` to
check that if both environments contain an entry for the same declaration in
`DeclToLoc`, they both map to the same `StorageLocation`. As discussed in
https://discourse.llvm.org/t/70086/5, this also necessitates removing
declarations from `DeclToLoc` when they go out of scope.

In the RFC, I proposed a gradual migration for this change, but it appears
that all of the callers of `Environment::setStorageLocation(const ValueDecl &,
SkipPast` are in the dataflow framework itself, and that there are only a few of
them.

As this is the function whose semantics are changing in a way that callers
potentially need to adapt to, I've decided to change the semantics of the
function directly.

The semantics of `getStorageLocation(const ValueDecl &, SkipPast SP` now no
longer depend on the behavior of the `SP` parameter. (There don't appear to be
any callers that use `SkipPast::ReferenceThenPointer`, so I've added an
assertion that forbids this usage.)

This patch adds a default argument for the `SP` parameter and removes the
explicit `SP` argument at the callsites that are touched by this change. A
followup patch will remove the argument from the remaining callsites,
allowing the `SkipPast` parameter to be removed entirely. (I don't want to do
that in this patch so that semantics-changing changes can be reviewed separately
from semantics-neutral changes.)

Reviewed By: ymandel, xazax.hun, gribozavr2

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