[clang][dataflow] Allow disabling built-in transfer functions for CFG terminators

[ymandel]

Summary

Terminators are handled specially in the transfer functions so we need an
additional check on whether the analysis has disabled built-in transfer
functions.

[xazax.hun]

The change itself looks good. But out of curiosity, could you give me an example when we do not want to use the builtin transfer functions?

[ymandel]

In D121694#3382473, @xazax.hun wrote:

The change itself looks good. But out of curiosity, could you give me an example when we do not want to use the builtin transfer functions?

Sure! Pretty much any plain-vanilla dataflow analysis that sticks to its own lattice and doesn't care about the environment. The demo constant-propagation analyses are like this, but we have additional real analyses using the framework in this way. Examples include an analysis to detect raw pointers that could be unique pointers and one that detects missed opportunies to use std::move.

[xazax.hun]

In D121694#3382587, @ymandel wrote:

In D121694#3382473, @xazax.hun wrote:

The change itself looks good. But out of curiosity, could you give me an example when we do not want to use the builtin transfer functions?

Sure! Pretty much any plain-vanilla dataflow analysis that sticks to its own lattice and doesn't care about the environment. The demo constant-propagation analyses are like this, but we have additional real analyses using the framework in this way. Examples include an analysis to detect raw pointers that could be unique pointers and one that detects missed opportunies to use std::move.

Makes sense! Although, I wonder if we would want an alternative API where the transfer function would not even get Env as an argument. So users could pick the right class to derive from for their needs and the decision whether use the built-in transfer functions would be compile time.

[ymandel]

In D121694#3382683, @xazax.hun wrote:

In D121694#3382587, @ymandel wrote:

In D121694#3382473, @xazax.hun wrote:

The change itself looks good. But out of curiosity, could you give me an example when we do not want to use the builtin transfer functions?

Sure! Pretty much any plain-vanilla dataflow analysis that sticks to its own lattice and doesn't care about the environment. The demo constant-propagation analyses are like this, but we have additional real analyses using the framework in this way. Examples include an analysis to detect raw pointers that could be unique pointers and one that detects missed opportunies to use std::move.

Makes sense! Although, I wonder if we would want an alternative API where the transfer function would not even get Env as an argument. So users could pick the right class to derive from for their needs and the decision whether use the built-in transfer functions would be compile time.

That would make sense. We definitely don't want to stick w/ the current setup. But, we've been thinking that we can split out Env and its transfer functions as a standalone "model" that users can choose to compose with their own (more specialized) model. I'm planning to send a patch shortly with an initial API for "models" (basically, a virtual API corresponding to DataflowAnalysis as of now).

clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp

if (Block.getTerminator().isTemporaryDtorsBranch()) {
     if (Block.succ_begin()->getReachableBlock()->hasNoReturnElement()) {
       auto StmtBlock = CFCtx.getStmtToBlock().find(Block.getTerminatorStmt());
       assert(StmtBlock != CFCtx.getStmtToBlock().end());
       Preds.erase(StmtBlock->getSecond());
     }
   }
 
   llvm::Optional<TypeErasedDataflowAnalysisState> MaybeState;
+  bool ApplyBuiltinTransfer = Analysis.applyBuiltinTransfer();
+
   for (const CFGBlock *Pred : Preds) {
     // Skip if the `Block` is unreachable or control flow cannot get past it.
     if (!Pred || Pred->hasNoReturnElement())
       continue;
 
     // Skip if `Pred` was not evaluated yet. This could happen if `Pred` has a
     // loop back edge to `Block`.
     const llvm::Optional<TypeErasedDataflowAnalysisState> &MaybePredState =
         BlockStates[Pred->getBlockID()];
     if (!MaybePredState.hasValue())
       continue;
 
     TypeErasedDataflowAnalysisState PredState = MaybePredState.getValue();
+    if (ApplyBuiltinTransfer) {
       if (const Stmt *PredTerminatorStmt = Pred->getTerminatorStmt()) {
         const StmtToEnvMapImpl StmtToEnv(CFCtx, BlockStates);
         TerminatorVisitor(StmtToEnv, PredState.Env,
                           blockIndexInPredecessor(*Pred, Block))
             .Visit(PredTerminatorStmt);
       }
+    }
 
     if (MaybeState.hasValue()) {
       Analysis.joinTypeErased(MaybeState->Lattice, PredState.Lattice);
       MaybeState->Env.join(PredState.Env, Analysis);
     } else {
       MaybeState = std::move(PredState);
     }
   }