[all-commits] [llvm/llvm-project] c19cac: [clang][dataflow] Tighten checking for existence o...

[martinboehme via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: c19cacfa34f52b65addeb7239d564b20e3cf2c61
      https://github.com/llvm/llvm-project/commit/c19cacfa34f52b65addeb7239d564b20e3cf2c61
  Author: martinboehme <mboehme at google.com>
  Date:   2024-01-16 (Tue, 16 Jan 2024)

  Changed paths:
    M clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
    M clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
    M clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
    M clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
    M clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

  Log Message:
  -----------
  [clang][dataflow] Tighten checking for existence of a function body. (#78163)

In various places, we would previously call `FunctionDecl::hasBody()`
(which
checks whether any redeclaration of the function has a body, not
necessarily the
one on which `hasBody()` is being called).

This is bug-prone, as a recent bug in Crubit's nullability checker has
shown

([fix](https://github.com/google/crubit/commit/4b01ed0f14d953cda20f92d62256e7365d206b2e),
[fix for the
fix](https://github.com/google/crubit/commit/e0c5d8ddd7d647da483c2ae198ff91d131c12055)).

Instead, we now use `FunctionDecl::doesThisDeclarationHaveABody()`
which, as the
name implies, checks whether the specific redeclaration it is being
called on
has a body.

Alternatively, I considered being more lenient and "canonicalizing" to
the
`FunctionDecl` that has the body if the `FunctionDecl` being passed is a
different redeclaration. However, this also risks hiding bugs: A caller
might
inadverently perform the analysis for all redeclarations of a function
and end
up duplicating work without realizing it. By accepting only the
redeclaration
that contains the body, we prevent this.

I've checked, and all clients that I'm aware of do currently pass in the
redeclaration that contains the function body. Typically this is because
they
use the `ast_matchers::hasBody()` matcher which, unlike
`FunctionDecl::hasBody()`, only matches for the redeclaration containing
the
body.