search

llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.78k stars 11.9k forks source link

[Coverage][NFC] Avoid visiting non-unique `OpaqueValueExpr` #88910

Closed bolshakov-a closed 6 months ago

bolshakov-a commented 6 months ago

Only unique OpaqueValueExprs should be handled in the mapping builder, as discussed in #85837. However, getCond() returns non-unique OpaqueValueExpr for BinaryConditionalOperator (because it is also used as the "true" branch expression). Use getCommon() instead so as to bypass the OpaqueValueExpr.

@efriedma-quic

llvmbot commented 6 months ago

@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Author: Andrey Ali Khan Bolshakov (bolshakov-a)

Changes Only unique `OpaqueValueExpr`s should be handled in the mapping builder, as [discussed](https://github.com/llvm/llvm-project/pull/85837#discussion_r1542056451) in #85837. However, `getCond()` returns non-unique `OpaqueValueExpr` for `BinaryConditionalOperator` (because it is also used as the "true" branch expression). Use `getCommon()` instead so as to bypass the `OpaqueValueExpr`. @efriedma-quic --- Full diff: https://github.com/llvm/llvm-project/pull/88910.diff 1 Files Affected: - (modified) clang/lib/CodeGen/CoverageMappingGen.cpp (+5-5) ``````````diff diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp index 71215da362d3d0..64c39c5de351c7 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -2011,11 +2011,13 @@ struct CounterCoverageMappingBuilder Counter TrueCount = llvm::EnableSingleByteCoverage ? getRegionCounter(E->getTrueExpr()) : getRegionCounter(E); - - propagateCounts(ParentCount, E->getCond()); Counter OutCount; - if (!isa(E)) { + if (const auto *BCO = dyn_cast(E)) { + propagateCounts(ParentCount, BCO->getCommon()); + OutCount = TrueCount; + } else { + propagateCounts(ParentCount, E->getCond()); // The 'then' count applies to the area immediately after the condition. auto Gap = findGapAreaBetween(E->getQuestionLoc(), getStart(E->getTrueExpr())); @@ -2024,8 +2026,6 @@ struct CounterCoverageMappingBuilder extendRegion(E->getTrueExpr()); OutCount = propagateCounts(TrueCount, E->getTrueExpr()); - } else { - OutCount = TrueCount; } extendRegion(E->getFalseExpr()); ``````````
bolshakov-a commented 6 months ago

Could you merge it please?

efriedma-quic commented 6 months ago

Looks like automation didn't trigger for some reason... but quoting the automated message that's supposed to trigger:

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo. Please turn off Keep my email addresses private setting in your account. See LLVM Discourse for more information.

bolshakov-a commented 6 months ago

Please turn off Keep my email addresses private setting in your account.

Done.