[all-commits] [llvm/llvm-project] 597a31: [clang][dataflow] Don't propagate result objects i...

[martinboehme via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 597a3150e932a9423c65b5ea4b53dd431aff5865
      https://github.com/llvm/llvm-project/commit/597a3150e932a9423c65b5ea4b53dd431aff5865
  Author: martinboehme <mboehme at google.com>
  Date:   2024-05-02 (Thu, 02 May 2024)

  Changed paths:
    M clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
    M clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

  Log Message:
  -----------
  [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).



To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications