[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](google/crubit@4b01ed0),
[fix for the
fix](google/crubit@e0c5d8d)).

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.

clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h

@@ -32,7 +32,8 @@ namespace dataflow {
 class ControlFlowContext {
 public:
   /// Builds a ControlFlowContext from a `FunctionDecl`.
-  /// `Func.hasBody()` must be true, and `Func.isTemplated()` must be false.
+  /// `Func.doesThisDeclarationHaveABody()` must be true, and
+  /// `Func.isTemplated()` must be false.
   static llvm::Expected<ControlFlowContext> build(const FunctionDecl &Func);
 
   /// Builds a ControlFlowContext from an AST node. `D` is the function in which

clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h

@@ -172,7 +172,8 @@ class Environment {
   ///
   /// Requirements:
   ///
-  ///  The function must have a body.
+  ///  The function must have a body, i.e.
+  ///  `FunctionDecl::doesThisDecalarationHaveABody()` must be true.
   void initialize();
 
   /// Returns a new environment that is a copy of this one.

clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp

@@ -69,7 +69,7 @@ static llvm::BitVector findReachableBlocks(const CFG &Cfg) {
 
 llvm::Expected<ControlFlowContext>
 ControlFlowContext::build(const FunctionDecl &Func) {
-  if (!Func.hasBody())
+  if (!Func.doesThisDeclarationHaveABody())
     return llvm::createStringError(
         std::make_error_code(std::errc::invalid_argument),
         "Cannot analyze function without a body");

clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp

@@ -298,7 +298,7 @@ DataflowAnalysisContext::getControlFlowContext(const FunctionDecl *F) {
   if (It != FunctionContexts.end())
     return &It->second;
 
-  if (F->hasBody()) {
+  if (F->doesThisDeclarationHaveABody()) {
     auto CFCtx = ControlFlowContext::build(*F);
     // FIXME: Handle errors.
     assert(CFCtx);

clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

@@ -386,7 +386,7 @@ void Environment::initialize() {
     return;
 
   if (const auto *FuncDecl = dyn_cast<FunctionDecl>(DeclCtx)) {
-    assert(FuncDecl->getBody() != nullptr);
+    assert(FuncDecl->doesThisDeclarationHaveABody());
 
     initFieldsGlobalsAndFuncs(FuncDecl);
 
@@ -426,7 +426,7 @@ void Environment::initialize() {
 // FIXME: Add support for resetting globals after function calls to enable
 // the implementation of sound analyses.
 void Environment::initFieldsGlobalsAndFuncs(const FunctionDecl *FuncDecl) {
-  assert(FuncDecl->getBody() != nullptr);
+  assert(FuncDecl->doesThisDeclarationHaveABody());
 
   FieldSet Fields;
   llvm::DenseSet<const VarDecl *> Vars;