li.zhe.hua added inline comments.

================
Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:180
+      assert(Block->pred_size() == 2);
+      BackEdge = Pred;
+    }
----------------
ymandel wrote:
> Might it be worth simply returning the backedge when you find it? Or is the 
> assertion (above) sufficiently important to keep it as is?
So, I couldn't prove to myself that a block couldn't have two backedge 
predecessors from staring at the CFG code, but it seems conceptually unlikely 
enough that I'm OK simplifying this.


================
Comment at: 
clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp:275
+  )";
+  auto BlockStates = llvm::cantFail(runAnalysis<ConvergesOnWidenAnalysis>(
+      Code, [](ASTContext &C) { return ConvergesOnWidenAnalysis(C); }));
----------------
ymandel wrote:
> Might a (googletest) assertion here be better than `llvm::cantFail`? I would 
> think that this line is the crux of checking whether it converges or not.
Ah, yes. I hadn't thought to look for `llvm::Expected` matchers; good call.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131646/new/

https://reviews.llvm.org/D131646

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to