Skip to content

Commit

Permalink
[coverage] skipping code coverage for 'if constexpr' and 'if consteval'
Browse files Browse the repository at this point in the history
  • Loading branch information
hanickadot committed Jan 22, 2024
1 parent a43c192 commit e2e0eca
Show file tree
Hide file tree
Showing 5 changed files with 317 additions and 79 deletions.
212 changes: 180 additions & 32 deletions clang/lib/CodeGen/CoverageMappingGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -119,26 +119,31 @@ class SourceMappingRegion {
/// as the line execution count if there are no other regions on the line.
bool GapRegion;

/// Whetever this region is skipped ('if constexpr' or 'if consteval' untaken
/// branch, or anything skipped but not empty line / comments)
bool SkippedRegion;

public:
SourceMappingRegion(Counter Count, std::optional<SourceLocation> LocStart,
std::optional<SourceLocation> LocEnd,
bool GapRegion = false)
: Count(Count), LocStart(LocStart), LocEnd(LocEnd), GapRegion(GapRegion) {
}
: Count(Count), LocStart(LocStart), LocEnd(LocEnd), GapRegion(GapRegion),
SkippedRegion(false) {}

SourceMappingRegion(Counter Count, std::optional<Counter> FalseCount,
MCDCParameters MCDCParams,
std::optional<SourceLocation> LocStart,
std::optional<SourceLocation> LocEnd,
bool GapRegion = false)
: Count(Count), FalseCount(FalseCount), MCDCParams(MCDCParams),
LocStart(LocStart), LocEnd(LocEnd), GapRegion(GapRegion) {}
LocStart(LocStart), LocEnd(LocEnd), GapRegion(GapRegion),
SkippedRegion(false) {}

SourceMappingRegion(MCDCParameters MCDCParams,
std::optional<SourceLocation> LocStart,
std::optional<SourceLocation> LocEnd)
: MCDCParams(MCDCParams), LocStart(LocStart), LocEnd(LocEnd),
GapRegion(false) {}
GapRegion(false), SkippedRegion(false) {}

const Counter &getCounter() const { return Count; }

Expand Down Expand Up @@ -174,6 +179,10 @@ class SourceMappingRegion {

void setGap(bool Gap) { GapRegion = Gap; }

bool isSkipped() const { return SkippedRegion; }

void setSkipped(bool Skipped) { SkippedRegion = Skipped; }

bool isBranch() const { return FalseCount.has_value(); }

bool isMCDCDecision() const { return MCDCParams.NumConditions != 0; }
Expand Down Expand Up @@ -468,6 +477,10 @@ class CoverageMappingBuilder {
MappingRegions.push_back(CounterMappingRegion::makeGapRegion(
Region.getCounter(), *CovFileID, SR.LineStart, SR.ColumnStart,
SR.LineEnd, SR.ColumnEnd));
} else if (Region.isSkipped()) {
MappingRegions.push_back(CounterMappingRegion::makeSkipped(
*CovFileID, SR.LineStart, SR.ColumnStart, SR.LineEnd,
SR.ColumnEnd));
} else if (Region.isBranch()) {
MappingRegions.push_back(CounterMappingRegion::makeBranchRegion(
Region.getCounter(), Region.getFalseCounter(),
Expand Down Expand Up @@ -1251,6 +1264,70 @@ struct CounterCoverageMappingBuilder
popRegions(Index);
}

/// Find a valid range starting with \p StartingLoc and ending before \p
/// BeforeLoc.
std::optional<SourceRange> findAreaStartingFromTo(SourceLocation StartingLoc,
SourceLocation BeforeLoc) {
// If StartingLoc is in function-like macro, use its start location.
if (StartingLoc.isMacroID()) {
FileID FID = SM.getFileID(StartingLoc);
const SrcMgr::ExpansionInfo *EI = &SM.getSLocEntry(FID).getExpansion();
if (EI->isFunctionMacroExpansion())
StartingLoc = EI->getExpansionLocStart();
}

size_t StartDepth = locationDepth(StartingLoc);
size_t EndDepth = locationDepth(BeforeLoc);
while (!SM.isWrittenInSameFile(StartingLoc, BeforeLoc)) {
bool UnnestStart = StartDepth >= EndDepth;
bool UnnestEnd = EndDepth >= StartDepth;
if (UnnestEnd) {
assert(SM.isWrittenInSameFile(getStartOfFileOrMacro(BeforeLoc),
BeforeLoc));

BeforeLoc = getIncludeOrExpansionLoc(BeforeLoc);
assert(BeforeLoc.isValid());
EndDepth--;
}
if (UnnestStart) {
assert(SM.isWrittenInSameFile(StartingLoc,
getStartOfFileOrMacro(StartingLoc)));

StartingLoc = getIncludeOrExpansionLoc(StartingLoc);
assert(StartingLoc.isValid());
StartDepth--;
}
}
// If the start and end locations of the gap are both within the same macro
// file, the range may not be in source order.
if (StartingLoc.isMacroID() || BeforeLoc.isMacroID())
return std::nullopt;
if (!SM.isWrittenInSameFile(StartingLoc, BeforeLoc) ||
!SpellingRegion(SM, StartingLoc, BeforeLoc).isInSourceOrder())
return std::nullopt;
return {{StartingLoc, BeforeLoc}};
}

void markSkipped(SourceLocation StartLoc, SourceLocation BeforeLoc) {
const auto Skipped = findAreaStartingFromTo(StartLoc, BeforeLoc);

if (!Skipped) {
return;
}

const auto NewStartLoc = Skipped->getBegin();
const auto EndLoc = Skipped->getEnd();

if (NewStartLoc == EndLoc)
return;
assert(SpellingRegion(SM, NewStartLoc, EndLoc).isInSourceOrder());
handleFileExit(NewStartLoc);
size_t Index = pushRegion({}, NewStartLoc, EndLoc);
getRegion().setSkipped(true);
handleFileExit(EndLoc);
popRegions(Index);
}

/// Keep counts of breaks and continues inside loops.
struct BreakContinue {
Counter BreakCount;
Expand Down Expand Up @@ -1700,43 +1777,116 @@ struct CounterCoverageMappingBuilder
Visit(S->getSubStmt());
}

void CoverIfConsteval(const IfStmt *S) {
assert(S->isConsteval());

const auto *Then = S->getThen();
const auto *Else = S->getElse();

// I'm using 'propagateCounts' later as new region is better and allows me
// to properly calculate line coverage in llvm-cov utility
const Counter ParentCount = getRegion().getCounter();

extendRegion(S);

if (S->isNegatedConsteval()) {
// ignore 'if consteval'
markSkipped(S->getIfLoc(), getStart(Then));
propagateCounts(ParentCount, Then);

if (Else) {
// ignore 'else <else>'
markSkipped(getEnd(Then), getEnd(Else));
}
} else {
assert(S->isNonNegatedConsteval());
// ignore 'if consteval <then> [else]'
markSkipped(S->getIfLoc(), Else ? getStart(Else) : getEnd(Then));

if (Else)
propagateCounts(ParentCount, Else);
}
}

void CoverIfConstexpr(const IfStmt *S) {
assert(S->isConstexpr());

// evaluate constant condition...
const auto *E = dyn_cast<ConstantExpr>(S->getCond());
assert(E != nullptr);
const bool isTrue = E->getResultAsAPSInt().getExtValue();

extendRegion(S);

const auto *Init = S->getInit();
const auto *Then = S->getThen();
const auto *Else = S->getElse();

// I'm using 'propagateCounts' later as new region is better and allows me
// to properly calculate line coverage in llvm-cov utility
const Counter ParentCount = getRegion().getCounter();

// ignore 'if constexpr ('
SourceLocation startOfSkipped = S->getIfLoc();

if (Init) {
// don't mark initialisation as ignored
markSkipped(startOfSkipped, getStart(Init));
propagateCounts(ParentCount, Init);
// ignore after initialisation: '; <condition>)'...
startOfSkipped = getEnd(Init);
}

if (isTrue) {
// ignore '<condition>)'
markSkipped(startOfSkipped, getStart(Then));
propagateCounts(ParentCount, Then);

if (Else)
// ignore 'else <else>'
markSkipped(getEnd(Then), getEnd(Else));
} else {
// ignore '<condition>) <then> [else]'
markSkipped(startOfSkipped, Else ? getStart(Else) : getEnd(Then));

if (Else) {
propagateCounts(ParentCount, Else);
}
}
}

void VisitIfStmt(const IfStmt *S) {
// "if constexpr" and "if consteval" are not normal conditional statements,
// they should behave more like a preprocessor conditions
if (S->isConsteval())
return CoverIfConsteval(S);
else if (S->isConstexpr())
return CoverIfConstexpr(S);

extendRegion(S);
if (S->getInit())
Visit(S->getInit());

// Extend into the condition before we propagate through it below - this is
// needed to handle macros that generate the "if" but not the condition.
if (!S->isConsteval())
extendRegion(S->getCond());
extendRegion(S->getCond());

Counter ParentCount = getRegion().getCounter();
Counter ThenCount = getRegionCounter(S);

// If this is "if !consteval" the then-branch will never be taken, we don't
// need to change counter
Counter ThenCount =
S->isNegatedConsteval() ? ParentCount : getRegionCounter(S);
// Emitting a counter for the condition makes it easier to interpret the
// counter for the body when looking at the coverage.
propagateCounts(ParentCount, S->getCond());

if (!S->isConsteval()) {
// Emitting a counter for the condition makes it easier to interpret the
// counter for the body when looking at the coverage.
propagateCounts(ParentCount, S->getCond());

// The 'then' count applies to the area immediately after the condition.
std::optional<SourceRange> Gap =
findGapAreaBetween(S->getRParenLoc(), getStart(S->getThen()));
if (Gap)
fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), ThenCount);
}
// The 'then' count applies to the area immediately after the condition.
std::optional<SourceRange> Gap =
findGapAreaBetween(S->getRParenLoc(), getStart(S->getThen()));
if (Gap)
fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), ThenCount);

extendRegion(S->getThen());
Counter OutCount = propagateCounts(ThenCount, S->getThen());

// If this is "if consteval" the else-branch will never be taken, we don't
// need to change counter
Counter ElseCount = S->isNonNegatedConsteval()
? ParentCount
: subtractCounters(ParentCount, ThenCount);
Counter ElseCount = subtractCounters(ParentCount, ThenCount);

if (const Stmt *Else = S->getElse()) {
bool ThenHasTerminateStmt = HasTerminateStmt;
Expand All @@ -1759,11 +1909,9 @@ struct CounterCoverageMappingBuilder
GapRegionCounter = OutCount;
}

if (!S->isConsteval()) {
// Create Branch Region around condition.
createBranchRegion(S->getCond(), ThenCount,
subtractCounters(ParentCount, ThenCount));
}
// Create Branch Region around condition.
createBranchRegion(S->getCond(), ThenCount,
subtractCounters(ParentCount, ThenCount));
}

void VisitCXXTryStmt(const CXXTryStmt *S) {
Expand Down
8 changes: 4 additions & 4 deletions clang/test/CoverageMapping/branch-constfolded.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,10 @@ bool for_7(bool a) { // MCDC: Decision,File 0, [[@LINE+1]]:10 -> [[@LINE+1
} // CHECK: Branch,File 0, [[@LINE-1]]:15 -> [[@LINE-1]]:19 = 0, 0

// CHECK-LABEL: _Z5for_8b:
bool for_8(bool a) { // MCDC: Decision,File 0, [[@LINE+3]]:17 -> [[@LINE+3]]:30 = M:0, C:2
// CHECK: Branch,File 0, [[@LINE+2]]:17 -> [[@LINE+2]]:21 = 0, 0
// CHECK: Branch,File 0, [[@LINE+1]]:25 -> [[@LINE+1]]:30 = 0, 0
if constexpr (true && false)
bool for_8(bool a) { // MCDC: Decision,File 0, [[@LINE+3]]:7 -> [[@LINE+3]]:20 = M:0, C:2
// CHECK: Branch,File 0, [[@LINE+2]]:7 -> [[@LINE+2]]:11 = 0, 0
// CHECK: Branch,File 0, [[@LINE+1]]:15 -> [[@LINE+1]]:20 = 0, 0
if (true && false)
return true;
else
return false;
Expand Down
Loading

0 comments on commit e2e0eca

Please sign in to comment.