[PATCH] D84467: Add support for Branch Coverage in LLVM Source-Based Code Coverage

[Vedant Kumar via Phabricator via llvm-commits]

vsk added a reviewer: zequanwu.
vsk added subscribers: ikudrin, arphaman.
vsk added a comment.

@alanphipps thanks for the patch! I'm a bit swamped at the moment, but I hope to give a detailed review by this Wednesday.



================
Comment at: clang/lib/CodeGen/CodeGenFunction.h:4359
+  /// condition (i.e. no "&&" or "||").
+  static bool isLeafCondition(const Expr *C);
+
----------------
It might be helpful to either require that `C` be the RHS of a logical binop (e.g. by passing the binop expr in), or to document that requirement.


================
Comment at: clang/lib/CodeGen/CodeGenFunction.h:4359
+  /// condition (i.e. no "&&" or "||").
+  static bool isLeafCondition(const Expr *C);
+
----------------
vsk wrote:
> It might be helpful to either require that `C` be the RHS of a logical binop (e.g. by passing the binop expr in), or to document that requirement.
Given a logical binop like `E = a && !(b || c)`, `isLeafCondition(E->getRHS())` is true. That seems a bit counter-intuitive, because `E->getRHS()` contains another leaf condition. Would it make sense to rename the condition (perhaps to something like 'InstrumentedCondition')? Have I misunderstood what a leaf condition is?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84467