[llvm] [clang] [clang][CoverageMapping] Refactor when setting MC/DC True/False (PR #78202)

NAKAMURA Takumi via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 16 22:57:38 PST 2024


================
@@ -730,68 +723,70 @@ struct MCDCCoverageBuilder {
       return;
 
     // If binary expression is disqualified, don't do mapping.
-    if (NestLevel.empty() &&
-        !MCDCBitmapMap.contains(CodeGenFunction::stripCond(E)))
+    if (!isBuilding() && !MCDCBitmapMap.contains(CodeGenFunction::stripCond(E)))
       NotMapped = true;
 
-    // Push Stmt on 'NestLevel' stack to keep track of nest location.
-    NestLevel.push_back(E);
-
     // Don't go any further if we don't need to map condition IDs.
     if (NotMapped)
       return;
 
+    assert(DecisionStack.size() >= 1);
+    const DecisionIDPair &ParentDecision = DecisionStack.back();
+
     // If the operator itself has an assigned ID, this means it represents a
-    // larger subtree.  In this case, pop its ID out of the RHS stack and
-    // assign that ID to its LHS node.  Its RHS will receive a new ID.
-    if (CondIDs.contains(CodeGenFunction::stripCond(E))) {
-      // If Stmt has an ID, assign its ID to LHS
-      CondIDs[CodeGenFunction::stripCond(E->getLHS())] = CondIDs[E];
-
-      // Since the operator's LHS assumes the operator's same ID, pop the
-      // operator from the RHS stack so that if LHS short-circuits, it won't be
-      // incorrectly re-used as the node executed next.
-      popRHSifTop(E);
-    } else {
-      // Otherwise, assign ID+1 to LHS.
-      CondIDs[CodeGenFunction::stripCond(E->getLHS())] = NextID++;
-    }
+    // larger subtree.  In this case, assign that ID to its LHS node.  Its RHS
+    // will receive a new ID below. Otherwise, assign ID+1 to LHS.
+    if (CondIDs.contains(CodeGenFunction::stripCond(E)))
+      setCondID(E->getLHS(), getCondID(E));
+    else
+      setCondID(E->getLHS(), NextID++);
+
+    // Assign a ID+1 for the RHS.
+    MCDCConditionID RHSid = NextID++;
+    setCondID(E->getRHS(), RHSid);
+
+    // Push the LHS decision IDs onto the DecisionStack.
+    if (isLAnd(E))
+      DecisionStack.push_back({RHSid, ParentDecision.FalseID});
+    else
+      DecisionStack.push_back({ParentDecision.TrueID, RHSid});
+  }
 
-    // Assign ID+1 to RHS.
-    CondIDs[CodeGenFunction::stripCond(E->getRHS())] = NextID++;
+  /// Pop and return the LHS Decision ({0,0} if not set).
+  DecisionIDPair pop() {
+    assert(DecisionStack.size() >= 1);
 
-    // Push ID of Stmt's RHS so that LHS nodes know about it
-    pushRHS(E);
+    if (!CGM.getCodeGenOpts().MCDCCoverage || NotMapped)
+      return DecisionStack.front();
+
+    assert(DecisionStack.size() > 1);
+    DecisionIDPair D = DecisionStack.back();
+    DecisionStack.pop_back();
+    return D;
   }
 
-  /// Pop the binary operator from the next level. If the walk is at the top of
-  /// the next, assign the total number of conditions.
-  unsigned popAndReturnCondCount(const BinaryOperator *E) {
+  /// Return the total number of conditions and reset the state. The number of
+  /// conditions is zero if the expression isn't mapped.
+  unsigned getTotalConditionsAndReset(const BinaryOperator *E) {
     if (!CGM.getCodeGenOpts().MCDCCoverage)
       return 0;
 
-    unsigned TotalConds = 0;
-
-    // Pop Stmt from 'NestLevel' stack.
-    assert(NestLevel.back() == E);
-    NestLevel.pop_back();
+    assert(!isIdle());
+    assert(DecisionStack.size() == 1);
----------------
chapuni wrote:

This is also better to hold, I think.

https://github.com/llvm/llvm-project/pull/78202


More information about the cfe-commits mailing list