llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Hana Dusíková (hanickadot) <details> <summary>Changes</summary> It was a while since I noticed coverage report is broken for "if constexpr" and "if consteval" (as shown on first picture). <img width="453" alt="Screenshot 2024-01-07 at 00 29 17" src="https://github.com/llvm/llvm-project/assets/6557263/dbdbc8a6-ad16-44da-882d-8e229ee69093"> (notice wrong coverage "if constexpr" for positive one, and completely missing for negative one, also notice "if consteval" marking always the same branch as uncovered) Report after this change: <img width="453" alt="Screenshot 2024-01-07 at 00 25 32" src="https://github.com/llvm/llvm-project/assets/6557263/d7ca85b0-34c7-40b5-9cc7-4efd8c18649b"> Main problem was replacement of non-taken "if constexpr" branch with `NullStmt` but such object doesn't have begin/end for source location properly. So I introduce a new constructor for empty `CompoundStmt` and used it. With "if consteval" I'm no longer introducing new branch counter for non-taken "branch". But in future it would be useful to mark whole gap there as skipped instead. If there is interest I would do it in another PR. --- Full diff: https://github.com/llvm/llvm-project/pull/77214.diff 3 Files Affected: - (modified) clang/include/clang/AST/Stmt.h (+4-2) - (modified) clang/lib/CodeGen/CoverageMappingGen.cpp (+11-2) - (modified) clang/lib/Sema/TreeTransform.h (+6-1) ``````````diff diff --git a/clang/include/clang/AST/Stmt.h b/clang/include/clang/AST/Stmt.h index da7b37ce0e1211..fb50212083316e 100644 --- a/clang/include/clang/AST/Stmt.h +++ b/clang/include/clang/AST/Stmt.h @@ -1631,8 +1631,10 @@ class CompoundStmt final SourceLocation RB); // Build an empty compound statement with a location. - explicit CompoundStmt(SourceLocation Loc) - : Stmt(CompoundStmtClass), LBraceLoc(Loc), RBraceLoc(Loc) { + explicit CompoundStmt(SourceLocation Loc) : CompoundStmt(Loc, Loc) {} + + explicit CompoundStmt(SourceLocation Loc, SourceLocation EndLoc) + : Stmt(CompoundStmtClass), LBraceLoc(Loc), RBraceLoc(EndLoc) { CompoundStmtBits.NumStmts = 0; CompoundStmtBits.HasFPFeatures = 0; } diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp index bf227386a71b78..b245abd16c3f4a 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -1712,7 +1712,11 @@ struct CounterCoverageMappingBuilder 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); if (!S->isConsteval()) { // Emitting a counter for the condition makes it easier to interpret the @@ -1729,7 +1733,12 @@ struct CounterCoverageMappingBuilder extendRegion(S->getThen()); Counter OutCount = propagateCounts(ThenCount, S->getThen()); - Counter ElseCount = subtractCounters(ParentCount, ThenCount); + // 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); + if (const Stmt *Else = S->getElse()) { bool ThenHasTerminateStmt = HasTerminateStmt; HasTerminateStmt = false; diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h index c8c5a51bf9f94e..0033c851b618a1 100644 --- a/clang/lib/Sema/TreeTransform.h +++ b/clang/lib/Sema/TreeTransform.h @@ -7732,7 +7732,8 @@ TreeTransform<Derived>::TransformIfStmt(IfStmt *S) { if (Then.isInvalid()) return StmtError(); } else { - Then = new (getSema().Context) NullStmt(S->getThen()->getBeginLoc()); + Then = new (getSema().Context) + CompoundStmt(S->getThen()->getBeginLoc(), S->getThen()->getEndLoc()); } // Transform the "else" branch. @@ -7741,6 +7742,10 @@ TreeTransform<Derived>::TransformIfStmt(IfStmt *S) { Else = getDerived().TransformStmt(S->getElse()); if (Else.isInvalid()) return StmtError(); + } else if (S->getElse() && ConstexprConditionValue && + *ConstexprConditionValue) { + Else = new (getSema().Context) + CompoundStmt(S->getElse()->getBeginLoc(), S->getElse()->getEndLoc()); } if (!getDerived().AlwaysRebuild() && `````````` </details> https://github.com/llvm/llvm-project/pull/77214 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits