Skip to content

Conversation

@Syzygy106
Copy link
Contributor

Motivation

Closes #12657

LCOV branch coverage produced by forge coverage was misleading and formatting-dependent for compound boolean conditions. In particular, short-circuit chains (&& / ||) could appear as 100% branch coverage when written inline, even if most sub-conditions were never evaluated. Multi-line if (...) conditions could also yield confusing BRDA attribution (branches reported on the body line instead of the condition line). This is tracked in #12657 (rooted in the POC from #12508).

Solution

This PR improves how forge coverage generates LCOV branch data so that short-circuit boolean conditions are reflected in branch coverage and the result is independent of formatting (inline vs multi-line). Previously, LCOV would typically only see the top-level construct (if true/false, require pass/fail), which could report deceptively high branch coverage even when later operands in &&/|| chains were never evaluated due to short-circuiting.
To address this, we extend the coverage source analysis to treat each && / || chain as a sequence of short-circuit decision points and emit corresponding LCOV branch items. This makes LCOV’s BRF/BRH counts represent whether each boolean operand was actually evaluated across execution paths, rather than only whether the outer statement was taken.
The same condition-branch modeling is applied consistently across if conditions, require(...) conditions, and loop conditions (while, for, and do-while), so condition coverage behaves the same regardless of where the boolean expression appears.
Finally, the PR fixes LCOV BRDA line attribution for multi-line if (...) statements by ensuring branches are reported on the condition line (while still anchoring to the correct bytecode range for execution mapping). This avoids confusing reports where branch entries appear on the body line purely due to formatting and keeps line attribution stable.

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

Copy link
Collaborator

@grandizzy grandizzy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you! However it doesn't seem to fix the reported issue, for example, given the contract

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

contract Counter {
    uint256 public number = 1;

    function setNumber(uint256 newNumber) public {
        if (newNumber != 0 || newNumber != 1 || newNumber != 2 || newNumber != 3 || newNumber != 4) {
            number = newNumber;
        }
        number = newNumber;
    }

    function increment() public {
        number++;
    }
}

forge coverage reports 22.22% (2/9) but there are only 5 branches and not 9. Also the lcov report shows duped branches (pairs of (branch 0 / branch 1) instead single (branch 0 / branch 1 / branch 2 / branch 3)) which is confusing

Image

@Syzygy106
Copy link
Contributor Author

thank you! However it doesn't seem to fix the reported issue, for example, given the contract

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

contract Counter {
    uint256 public number = 1;

    function setNumber(uint256 newNumber) public {
        if (newNumber != 0 || newNumber != 1 || newNumber != 2 || newNumber != 3 || newNumber != 4) {
            number = newNumber;
        }
        number = newNumber;
    }

    function increment() public {
        number++;
    }
}

forge coverage reports 22.22% (2/9) but there are only 5 branches and not 9. Also the lcov report shows duped branches (pairs of (branch 0 / branch 1) instead single (branch 0 / branch 1 / branch 2 / branch 3)) which is confusing

Image

But LCOV BRF/BRH counts branch paths, not branch points. For an || chain with 5 operands, the short-circuit modeling yields 4 decision points, each with 2 outcomes -> 8 branch paths. In Foundry’s LCOV branch model (if I’ve got it right), an if without an else contributes one reported branch path (the “then” path only), so the total is BRF = 8 + 1 = 9. The “duplicated” (0/1) branches are an artifact of genhtml displaying the branch index without the block component; each 0/1 pair belongs to a different block (decision point)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

feature(forge): improve lcov condition coverage

2 participants