[clang] [llvm] [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);
----------------
chapuni wrote:

I found another place to hold the assertion. Overall I suggest holding two place (including this).

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


More information about the cfe-commits mailing list