[clang] [Coverage][Expansion] handle nested macros in scratch space (PR #89869)
Wentao Zhang via cfe-commits
cfe-commits at lists.llvm.org
Thu May 23 09:11:43 PDT 2024
https://github.com/whentojump updated https://github.com/llvm/llvm-project/pull/89869
>From 510e0f131ad11d7924f33fc4c4130dcf866bd8da Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi <geek4civic at gmail.com>
Date: Fri, 19 Apr 2024 15:16:05 +0900
Subject: [PATCH 1/9] [MC/DC][Coverage] Workaround for `##` conditions
A synthesized identifier with `##` is emitted to `<scratch space>`.
`llvm-cov` cannot handle `<scratch space> since it doesn't have actual files.
As a workaround, peel `<scratch space>` to the actual definition if the definition is present.
This affects predefined built-in macros, like __LINE__.
Fixes #87000
---
clang/lib/CodeGen/CoverageMappingGen.cpp | 9 +++++++
clang/test/CoverageMapping/builtinmacro.c | 2 +-
clang/test/CoverageMapping/macros.c | 8 +++---
.../test/CoverageMapping/mcdc-scratch-space.c | 26 ++++++++++++++-----
4 files changed, 34 insertions(+), 11 deletions(-)
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 993d7cc7e21fa..d6b0145d19fea 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -344,6 +344,15 @@ class CoverageMappingBuilder {
for (auto &Region : SourceRegions) {
SourceLocation Loc = Region.getBeginLoc();
+ // Replace Region with its definition if it is in <scratch space>.
+ while (Loc.isMacroID() &&
+ SM.isWrittenInScratchSpace(SM.getSpellingLoc(Loc))) {
+ auto ExpansionRange = SM.getImmediateExpansionRange(Loc);
+ Loc = ExpansionRange.getBegin();
+ Region.setStartLoc(Loc);
+ Region.setEndLoc(ExpansionRange.getEnd());
+ }
+
// Replace Loc with FileLoc if it is expanded with system headers.
if (!SystemHeadersCoverage && SM.isInSystemMacro(Loc)) {
auto BeginLoc = SM.getSpellingLoc(Loc);
diff --git a/clang/test/CoverageMapping/builtinmacro.c b/clang/test/CoverageMapping/builtinmacro.c
index abcdc191523a5..5d5a176aa7d87 100644
--- a/clang/test/CoverageMapping/builtinmacro.c
+++ b/clang/test/CoverageMapping/builtinmacro.c
@@ -4,7 +4,7 @@
// CHECK: filename
const char *filename (const char *name) { // CHECK-NEXT: File 0, [[@LINE]]:41 -> [[@LINE+3]]:2 = #0
- static const char this_file[] = __FILE__;
+ static const char this_file[] = __FILE__; // CHECK-NEXT: File 0, [[@LINE]]:35 -> [[@LINE]]:35 = #0
return this_file;
}
diff --git a/clang/test/CoverageMapping/macros.c b/clang/test/CoverageMapping/macros.c
index 6bd3be434139a..fcf21170ef135 100644
--- a/clang/test/CoverageMapping/macros.c
+++ b/clang/test/CoverageMapping/macros.c
@@ -80,12 +80,14 @@ void func7(void) { // CHECK-NEXT: File 0, [[@LINE]]:18 -> [[@LINE+6]]:2 = #0
int kk,ll; // CHECK-NEXT: File 0, [[@LINE+1]]:7 -> [[@LINE+1]]:8 = #0
if (k) // CHECK-NEXT: Branch,File 0, [[@LINE]]:7 -> [[@LINE]]:8 = #1
m(k); // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:9 -> [[@LINE]]:5 = #1
- else // CHECK-NEXT: Expansion,File 0, [[@LINE-1]]:5 -> [[@LINE-1]]:6 = #0
+ else // CHECK-NEXT: Expansion,File 0, [[@LINE-1]]:5 -> [[@LINE-1]]:6 = #1
l = m(l); // CHECK-NEXT: Gap,File 0, [[@LINE-2]]:7 -> [[@LINE]]:5 = (#0 - #1)
} // CHECK-NEXT: File 0, [[@LINE-1]]:5 -> [[@LINE-1]]:10 = (#0 - #1)
// CHECK-NEXT: Expansion,File 0, [[@LINE-2]]:9 -> [[@LINE-2]]:10 = (#0 - #1)
- // CHECK-NEXT: File 1, [[@LINE-9]]:14 -> [[@LINE-9]]:18 = #0
- // CHECK-NEXT: File 2, [[@LINE-10]]:14 -> [[@LINE-10]]:15 = (#0 - #1)
+ // CHECK-NEXT: File 1, [[@LINE-9]]:14 -> [[@LINE-9]]:17 = #1
+ // CHECK-NEXT: File 1, [[@LINE-10]]:14 -> [[@LINE-10]]:18 = #0
+ // CHECK-NEXT: File 2, [[@LINE-11]]:14 -> [[@LINE-11]]:17 = (#0 - #1)
+ // CHECK-NEXT: File 2, [[@LINE-12]]:14 -> [[@LINE-12]]:15 = (#0 - #1)
int main(int argc, const char *argv[]) {
func();
diff --git a/clang/test/CoverageMapping/mcdc-scratch-space.c b/clang/test/CoverageMapping/mcdc-scratch-space.c
index 962d10653a028..1b8735cd27445 100644
--- a/clang/test/CoverageMapping/mcdc-scratch-space.c
+++ b/clang/test/CoverageMapping/mcdc-scratch-space.c
@@ -1,27 +1,39 @@
-// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c99 -fcoverage-mcdc -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only %s
-// XFAIL: *
-// REQUIRES: asserts
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c99 -fcoverage-mcdc -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only %s | FileCheck %s
+// CHECK: builtin_macro0:
int builtin_macro0(int a) {
- return (__LINE__
- && a);
+ // CHECK: Decision,File 0, [[@LINE+1]]:11 -> [[@LINE+2]]:15 = M:0, C:2
+ return (__LINE__ // CHECK: Branch,File 0, [[@LINE]]:11 -> [[@LINE]]:11 = 0, 0 [1,2,0]
+ && a); // CHECK: Branch,File 0, [[@LINE]]:14 -> [[@LINE]]:15 = #2, (#1 - #2) [2,0,0]
}
+// CHECK: builtin_macro1:
int builtin_macro1(int a) {
- return (a
- || __LINE__);
+ // CHECK: Decision,File 0, [[@LINE+1]]:11 -> [[@LINE+2]]:22 = M:0, C:2
+ return (a // CHECK: Branch,File 0, [[@LINE]]:11 -> [[@LINE]]:12 = (#0 - #1), #1 [1,0,2]
+ || __LINE__); // CHECK: Branch,File 0, [[@LINE]]:14 -> [[@LINE]]:14 = 0, 0 [2,0,0]
}
#define PRE(x) pre_##x
+// CHECK: pre0:
int pre0(int pre_a, int b_post) {
+ // CHECK: Decision,File 0, [[@LINE+2]]:11 -> [[@LINE+3]]:20 = M:0, C:2
+ // CHECK: Expansion,File 0, [[@LINE+1]]:11 -> [[@LINE+1]]:14 = #0 (Expanded file = 1)
return (PRE(a)
&& b_post);
+ // CHECK: Branch,File 0, [[@LINE-1]]:14 -> [[@LINE-1]]:20 = #2, (#1 - #2) [2,0,0]
+ // CHECK: Branch,File 1, [[@LINE-9]]:16 -> [[@LINE-9]]:22 = #1, (#0 - #1) [1,2,0]
}
#define POST(x) x##_post
+// CHECK: post0:
int post0(int pre_a, int b_post) {
+ // CHECK: Decision,File 0, [[@LINE+2]]:11 -> [[@LINE+3]]:18 = M:0, C:2
+ // CHECK: Branch,File 0, [[@LINE+1]]:11 -> [[@LINE+1]]:16 = (#0 - #1), #1 [1,0,2]
return (pre_a
|| POST(b));
+ // CHECK: Expansion,File 0, [[@LINE-1]]:14 -> [[@LINE-1]]:18 = #1 (Expanded file = 1)
+ // CHECK: Branch,File 1, [[@LINE-9]]:17 -> [[@LINE-9]]:20 = (#1 - #2), #2 [2,0,0]
}
>From 0c22a2491e65a64ef1c168a2626b246ffecfe6ba Mon Sep 17 00:00:00 2001
From: Wentao Zhang <zhangwt1997 at gmail.com>
Date: Tue, 23 Apr 2024 22:46:54 -0500
Subject: [PATCH 2/9] [Coverage][Expansion] handle nested macros in scratch
space
---
clang/lib/CodeGen/CoverageMappingGen.cpp | 19 +++++++++++++-
.../test/CoverageMapping/mcdc-scratch-space.c | 26 +++++++++++++++++++
2 files changed, 44 insertions(+), 1 deletion(-)
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index d6b0145d19fea..2dd7f43b44c6b 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -300,6 +300,22 @@ class CoverageMappingBuilder {
: SM.getIncludeLoc(SM.getFileID(Loc));
}
+ /// Find out where the current file is included or macro is expanded. If the
+ /// found expansion is a <scratch space>, keep looking.
+ SourceLocation getIncludeOrNonScratchExpansionLoc(SourceLocation Loc) {
+ if (Loc.isMacroID()) {
+ Loc = SM.getImmediateExpansionRange(Loc).getBegin();
+ while (Loc.isMacroID() &&
+ SM.isWrittenInScratchSpace(SM.getSpellingLoc(Loc))) {
+ auto ExpansionRange = SM.getImmediateExpansionRange(Loc);
+ Loc = ExpansionRange.getBegin();
+ }
+ } else {
+ Loc = SM.getIncludeLoc(SM.getFileID(Loc));
+ }
+ return Loc;
+ }
+
/// Return true if \c Loc is a location in a built-in macro.
bool isInBuiltin(SourceLocation Loc) {
return SM.getBufferName(SM.getSpellingLoc(Loc)) == "<built-in>";
@@ -547,7 +563,8 @@ class CoverageMappingBuilder {
SourceRegionFilter Filter;
for (const auto &FM : FileIDMapping) {
SourceLocation ExpandedLoc = FM.second.second;
- SourceLocation ParentLoc = getIncludeOrExpansionLoc(ExpandedLoc);
+ SourceLocation ParentLoc =
+ getIncludeOrNonScratchExpansionLoc(ExpandedLoc);
if (ParentLoc.isInvalid())
continue;
diff --git a/clang/test/CoverageMapping/mcdc-scratch-space.c b/clang/test/CoverageMapping/mcdc-scratch-space.c
index 1b8735cd27445..2b5b12d9dcad6 100644
--- a/clang/test/CoverageMapping/mcdc-scratch-space.c
+++ b/clang/test/CoverageMapping/mcdc-scratch-space.c
@@ -26,6 +26,19 @@ int pre0(int pre_a, int b_post) {
// CHECK: Branch,File 1, [[@LINE-9]]:16 -> [[@LINE-9]]:22 = #1, (#0 - #1) [1,2,0]
}
+#define pre_foo pre_a
+
+// CHECK: pre1:
+int pre1(int pre_a, int b_post) {
+ // CHECK: Decision,File 0, [[@LINE+3]]:11 -> [[@LINE+4]]:20 = M:0, C:2
+ // CHECK: Expansion,File 0, [[@LINE+2]]:11 -> [[@LINE+2]]:14 = #0 (Expanded file = 1)
+ // CHECK: Branch,File 0, [[@LINE+2]]:14 -> [[@LINE+2]]:20 = #2, (#1 - #2) [2,0,0]
+ return (PRE(foo)
+ && b_post);
+ // CHECK: Expansion,File 1, 17:16 -> 17:20 = #0 (Expanded file = 2)
+ // CHECK: Branch,File 2, 29:17 -> 29:22 = #1, (#0 - #1) [1,2,0]
+}
+
#define POST(x) x##_post
// CHECK: post0:
@@ -37,3 +50,16 @@ int post0(int pre_a, int b_post) {
// CHECK: Expansion,File 0, [[@LINE-1]]:14 -> [[@LINE-1]]:18 = #1 (Expanded file = 1)
// CHECK: Branch,File 1, [[@LINE-9]]:17 -> [[@LINE-9]]:20 = (#1 - #2), #2 [2,0,0]
}
+
+#define bar_post b_post
+
+// CHECK: post1:
+int post1(int pre_a, int b_post) {
+ // CHECK: Decision,File 0, [[@LINE+3]]:11 -> [[@LINE+4]]:18 = M:0, C:2
+ // CHECK: Branch,File 0, [[@LINE+2]]:11 -> [[@LINE+2]]:16 = (#0 - #1), #1 [1,0,2]
+ // CHECK: Expansion,File 0, [[@LINE+2]]:14 -> [[@LINE+2]]:18 = 0 (Expanded file = 1)
+ return (pre_a
+ || POST(bar));
+ // CHECK: Expansion,File 1, 42:17 -> 42:18 = #1 (Expanded file = 2)
+ // CHECK: Branch,File 2, 54:18 -> 54:24 = (#1 - #2), #2 [2,0,0]
+}
>From 17b6484263285f7c8ff0de1c7ca064ba27e2db48 Mon Sep 17 00:00:00 2001
From: Wentao Zhang <zhangwt1997 at gmail.com>
Date: Wed, 24 Apr 2024 14:05:45 -0500
Subject: [PATCH 3/9] address comments: one getIncludeOrExpansionLoc() function
controlled by a flag
---
clang/lib/CodeGen/CoverageMappingGen.cpp | 31 ++++++++++--------------
1 file changed, 13 insertions(+), 18 deletions(-)
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 2dd7f43b44c6b..4ca2c220dcbe1 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -294,25 +294,21 @@ class CoverageMappingBuilder {
return SM.getLocForEndOfFile(SM.getFileID(Loc));
}
- /// Find out where the current file is included or macro is expanded.
- SourceLocation getIncludeOrExpansionLoc(SourceLocation Loc) {
- return Loc.isMacroID() ? SM.getImmediateExpansionRange(Loc).getBegin()
- : SM.getIncludeLoc(SM.getFileID(Loc));
- }
-
- /// Find out where the current file is included or macro is expanded. If the
- /// found expansion is a <scratch space>, keep looking.
- SourceLocation getIncludeOrNonScratchExpansionLoc(SourceLocation Loc) {
+ /// Find out where the current file is included or macro is expanded. If
+ /// \c AcceptScratch is set to false, keep looking for expansions until the
+ /// found sloc is not a <scratch space>.
+ SourceLocation getIncludeOrExpansionLoc(SourceLocation Loc,
+ bool AcceptScratch = true) {
if (Loc.isMacroID()) {
Loc = SM.getImmediateExpansionRange(Loc).getBegin();
- while (Loc.isMacroID() &&
- SM.isWrittenInScratchSpace(SM.getSpellingLoc(Loc))) {
- auto ExpansionRange = SM.getImmediateExpansionRange(Loc);
- Loc = ExpansionRange.getBegin();
- }
- } else {
+ if (!AcceptScratch)
+ while (Loc.isMacroID() &&
+ SM.isWrittenInScratchSpace(SM.getSpellingLoc(Loc))) {
+ auto ExpansionRange = SM.getImmediateExpansionRange(Loc);
+ Loc = ExpansionRange.getBegin();
+ }
+ } else
Loc = SM.getIncludeLoc(SM.getFileID(Loc));
- }
return Loc;
}
@@ -563,8 +559,7 @@ class CoverageMappingBuilder {
SourceRegionFilter Filter;
for (const auto &FM : FileIDMapping) {
SourceLocation ExpandedLoc = FM.second.second;
- SourceLocation ParentLoc =
- getIncludeOrNonScratchExpansionLoc(ExpandedLoc);
+ SourceLocation ParentLoc = getIncludeOrExpansionLoc(ExpandedLoc, false);
if (ParentLoc.isInvalid())
continue;
>From 7250b77404d54aeb9aac2653b602121d38d9ece5 Mon Sep 17 00:00:00 2001
From: Wentao Zhang <zhangwt1997 at gmail.com>
Date: Wed, 24 Apr 2024 16:21:24 -0500
Subject: [PATCH 4/9] address comments: extract common code
---
clang/lib/CodeGen/CoverageMappingGen.cpp | 38 ++++++++++++++++--------
1 file changed, 26 insertions(+), 12 deletions(-)
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 4ca2c220dcbe1..439724fe6c0e1 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -294,6 +294,25 @@ class CoverageMappingBuilder {
return SM.getLocForEndOfFile(SM.getFileID(Loc));
}
+ /// Find out where a macro is expanded. If the immediate result is a
+ /// <scratch space>, keep looking until the result isn't. Return a pair of
+ /// \c SourceLocation. The first object is always the begin sloc of found
+ /// result. The second should be checked by the caller: if it has value, it's
+ /// the end sloc of the found result. Otherwise the while loop didn't get
+ /// executed, which means the location wasn't changed and the caller has to
+ /// learn the end sloc from somewhere else.
+ std::pair<SourceLocation, std::optional<SourceLocation>>
+ getNonScratchExpansionLoc(SourceLocation Loc) {
+ std::optional<SourceLocation> EndLoc = std::nullopt;
+ while (Loc.isMacroID() &&
+ SM.isWrittenInScratchSpace(SM.getSpellingLoc(Loc))) {
+ auto ExpansionRange = SM.getImmediateExpansionRange(Loc);
+ Loc = ExpansionRange.getBegin();
+ EndLoc = ExpansionRange.getEnd();
+ }
+ return std::make_pair(Loc, EndLoc);
+ }
+
/// Find out where the current file is included or macro is expanded. If
/// \c AcceptScratch is set to false, keep looking for expansions until the
/// found sloc is not a <scratch space>.
@@ -302,11 +321,7 @@ class CoverageMappingBuilder {
if (Loc.isMacroID()) {
Loc = SM.getImmediateExpansionRange(Loc).getBegin();
if (!AcceptScratch)
- while (Loc.isMacroID() &&
- SM.isWrittenInScratchSpace(SM.getSpellingLoc(Loc))) {
- auto ExpansionRange = SM.getImmediateExpansionRange(Loc);
- Loc = ExpansionRange.getBegin();
- }
+ Loc = getNonScratchExpansionLoc(Loc).first;
} else
Loc = SM.getIncludeLoc(SM.getFileID(Loc));
return Loc;
@@ -357,13 +372,12 @@ class CoverageMappingBuilder {
SourceLocation Loc = Region.getBeginLoc();
// Replace Region with its definition if it is in <scratch space>.
- while (Loc.isMacroID() &&
- SM.isWrittenInScratchSpace(SM.getSpellingLoc(Loc))) {
- auto ExpansionRange = SM.getImmediateExpansionRange(Loc);
- Loc = ExpansionRange.getBegin();
- Region.setStartLoc(Loc);
- Region.setEndLoc(ExpansionRange.getEnd());
- }
+ auto NonScratchExpansionLoc = getNonScratchExpansionLoc(Loc);
+ Loc = NonScratchExpansionLoc.first;
+ auto EndLoc = NonScratchExpansionLoc.second;
+ Region.setStartLoc(Loc);
+ Region.setEndLoc(EndLoc.has_value() ? EndLoc.value()
+ : Region.getEndLoc());
// Replace Loc with FileLoc if it is expanded with system headers.
if (!SystemHeadersCoverage && SM.isInSystemMacro(Loc)) {
>From 819103d82bc1bae0eec2497f39d97fa4a53a26ae Mon Sep 17 00:00:00 2001
From: Wentao Zhang <zhangwt1997 at gmail.com>
Date: Wed, 24 Apr 2024 16:33:55 -0500
Subject: [PATCH 5/9] address comments: return early
---
clang/lib/CodeGen/CoverageMappingGen.cpp | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 439724fe6c0e1..3df2fcd7e4d2f 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -318,13 +318,12 @@ class CoverageMappingBuilder {
/// found sloc is not a <scratch space>.
SourceLocation getIncludeOrExpansionLoc(SourceLocation Loc,
bool AcceptScratch = true) {
- if (Loc.isMacroID()) {
- Loc = SM.getImmediateExpansionRange(Loc).getBegin();
- if (!AcceptScratch)
- Loc = getNonScratchExpansionLoc(Loc).first;
- } else
- Loc = SM.getIncludeLoc(SM.getFileID(Loc));
- return Loc;
+ if (!Loc.isMacroID())
+ return SM.getIncludeLoc(SM.getFileID(Loc));
+ Loc = SM.getImmediateExpansionRange(Loc).getBegin();
+ if (AcceptScratch)
+ return Loc;
+ return getNonScratchExpansionLoc(Loc).first;
}
/// Return true if \c Loc is a location in a built-in macro.
>From 14e52c3ffeb39ee576683afe28fb7fafcf6e7b26 Mon Sep 17 00:00:00 2001
From: Wentao Zhang <zhangwt1997 at gmail.com>
Date: Wed, 24 Apr 2024 22:47:06 -0500
Subject: [PATCH 6/9] address comments: return optional<SourceRange>; update
region only when needed
---
clang/lib/CodeGen/CoverageMappingGen.cpp | 32 ++++++++++++------------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 3df2fcd7e4d2f..47009730b7984 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -295,14 +295,10 @@ class CoverageMappingBuilder {
}
/// Find out where a macro is expanded. If the immediate result is a
- /// <scratch space>, keep looking until the result isn't. Return a pair of
- /// \c SourceLocation. The first object is always the begin sloc of found
- /// result. The second should be checked by the caller: if it has value, it's
- /// the end sloc of the found result. Otherwise the while loop didn't get
- /// executed, which means the location wasn't changed and the caller has to
- /// learn the end sloc from somewhere else.
- std::pair<SourceLocation, std::optional<SourceLocation>>
- getNonScratchExpansionLoc(SourceLocation Loc) {
+ /// <scratch space>, keep looking until the result isn't. Return the source
+ /// range of the found result, or std::nullopt if the while loop didn't get
+ /// executed, which means the location wasn't changed.
+ std::optional<SourceRange> getNonScratchExpansion(SourceLocation Loc) {
std::optional<SourceLocation> EndLoc = std::nullopt;
while (Loc.isMacroID() &&
SM.isWrittenInScratchSpace(SM.getSpellingLoc(Loc))) {
@@ -310,7 +306,9 @@ class CoverageMappingBuilder {
Loc = ExpansionRange.getBegin();
EndLoc = ExpansionRange.getEnd();
}
- return std::make_pair(Loc, EndLoc);
+ if (EndLoc.has_value())
+ return SourceRange(Loc, EndLoc.value());
+ return std::nullopt;
}
/// Find out where the current file is included or macro is expanded. If
@@ -323,7 +321,9 @@ class CoverageMappingBuilder {
Loc = SM.getImmediateExpansionRange(Loc).getBegin();
if (AcceptScratch)
return Loc;
- return getNonScratchExpansionLoc(Loc).first;
+ auto NonScratchExpansion = getNonScratchExpansion(Loc);
+ return NonScratchExpansion.has_value() ? NonScratchExpansion->getBegin()
+ : Loc;
}
/// Return true if \c Loc is a location in a built-in macro.
@@ -371,12 +371,12 @@ class CoverageMappingBuilder {
SourceLocation Loc = Region.getBeginLoc();
// Replace Region with its definition if it is in <scratch space>.
- auto NonScratchExpansionLoc = getNonScratchExpansionLoc(Loc);
- Loc = NonScratchExpansionLoc.first;
- auto EndLoc = NonScratchExpansionLoc.second;
- Region.setStartLoc(Loc);
- Region.setEndLoc(EndLoc.has_value() ? EndLoc.value()
- : Region.getEndLoc());
+ auto NonScratchExpansion = getNonScratchExpansion(Loc);
+ if (NonScratchExpansion.has_value()) {
+ Loc = NonScratchExpansion->getBegin();
+ Region.setStartLoc(Loc);
+ Region.setEndLoc(NonScratchExpansion->getEnd());
+ }
// Replace Loc with FileLoc if it is expanded with system headers.
if (!SystemHeadersCoverage && SM.isInSystemMacro(Loc)) {
>From 108a9ac160099719dd777c62b403ac6559d93bb2 Mon Sep 17 00:00:00 2001
From: Wentao Zhang <zhangwt1997 at gmail.com>
Date: Tue, 21 May 2024 16:47:49 -0500
Subject: [PATCH 7/9] Revert "address comments: return optional<SourceRange>;
update region only when needed"
This reverts commit 0eeb0d3e392b1ea1ae47bc8c977eb709cd1c264c.
---
clang/lib/CodeGen/CoverageMappingGen.cpp | 32 ++++++++++++------------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 47009730b7984..3df2fcd7e4d2f 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -295,10 +295,14 @@ class CoverageMappingBuilder {
}
/// Find out where a macro is expanded. If the immediate result is a
- /// <scratch space>, keep looking until the result isn't. Return the source
- /// range of the found result, or std::nullopt if the while loop didn't get
- /// executed, which means the location wasn't changed.
- std::optional<SourceRange> getNonScratchExpansion(SourceLocation Loc) {
+ /// <scratch space>, keep looking until the result isn't. Return a pair of
+ /// \c SourceLocation. The first object is always the begin sloc of found
+ /// result. The second should be checked by the caller: if it has value, it's
+ /// the end sloc of the found result. Otherwise the while loop didn't get
+ /// executed, which means the location wasn't changed and the caller has to
+ /// learn the end sloc from somewhere else.
+ std::pair<SourceLocation, std::optional<SourceLocation>>
+ getNonScratchExpansionLoc(SourceLocation Loc) {
std::optional<SourceLocation> EndLoc = std::nullopt;
while (Loc.isMacroID() &&
SM.isWrittenInScratchSpace(SM.getSpellingLoc(Loc))) {
@@ -306,9 +310,7 @@ class CoverageMappingBuilder {
Loc = ExpansionRange.getBegin();
EndLoc = ExpansionRange.getEnd();
}
- if (EndLoc.has_value())
- return SourceRange(Loc, EndLoc.value());
- return std::nullopt;
+ return std::make_pair(Loc, EndLoc);
}
/// Find out where the current file is included or macro is expanded. If
@@ -321,9 +323,7 @@ class CoverageMappingBuilder {
Loc = SM.getImmediateExpansionRange(Loc).getBegin();
if (AcceptScratch)
return Loc;
- auto NonScratchExpansion = getNonScratchExpansion(Loc);
- return NonScratchExpansion.has_value() ? NonScratchExpansion->getBegin()
- : Loc;
+ return getNonScratchExpansionLoc(Loc).first;
}
/// Return true if \c Loc is a location in a built-in macro.
@@ -371,12 +371,12 @@ class CoverageMappingBuilder {
SourceLocation Loc = Region.getBeginLoc();
// Replace Region with its definition if it is in <scratch space>.
- auto NonScratchExpansion = getNonScratchExpansion(Loc);
- if (NonScratchExpansion.has_value()) {
- Loc = NonScratchExpansion->getBegin();
- Region.setStartLoc(Loc);
- Region.setEndLoc(NonScratchExpansion->getEnd());
- }
+ auto NonScratchExpansionLoc = getNonScratchExpansionLoc(Loc);
+ Loc = NonScratchExpansionLoc.first;
+ auto EndLoc = NonScratchExpansionLoc.second;
+ Region.setStartLoc(Loc);
+ Region.setEndLoc(EndLoc.has_value() ? EndLoc.value()
+ : Region.getEndLoc());
// Replace Loc with FileLoc if it is expanded with system headers.
if (!SystemHeadersCoverage && SM.isInSystemMacro(Loc)) {
>From 6629cf1b84b638a0289bcd033d05074cc5bc7469 Mon Sep 17 00:00:00 2001
From: Wentao Zhang <zhangwt1997 at gmail.com>
Date: Tue, 21 May 2024 17:23:15 -0500
Subject: [PATCH 8/9] update region only when needed
---
clang/lib/CodeGen/CoverageMappingGen.cpp | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 3df2fcd7e4d2f..932f8c0c4a826 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -374,9 +374,10 @@ class CoverageMappingBuilder {
auto NonScratchExpansionLoc = getNonScratchExpansionLoc(Loc);
Loc = NonScratchExpansionLoc.first;
auto EndLoc = NonScratchExpansionLoc.second;
- Region.setStartLoc(Loc);
- Region.setEndLoc(EndLoc.has_value() ? EndLoc.value()
- : Region.getEndLoc());
+ if (EndLoc.has_value()) {
+ Region.setStartLoc(Loc);
+ Region.setEndLoc(EndLoc.value());
+ }
// Replace Loc with FileLoc if it is expanded with system headers.
if (!SystemHeadersCoverage && SM.isInSystemMacro(Loc)) {
>From 0eace69a28ea97e7c77666f9855aa6b0213442fd Mon Sep 17 00:00:00 2001
From: Wentao Zhang <zhangwt1997 at gmail.com>
Date: Thu, 23 May 2024 11:10:00 -0500
Subject: [PATCH 9/9] address comments: sink updating Loc should into then
clause
---
clang/lib/CodeGen/CoverageMappingGen.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 932f8c0c4a826..05de50e3f1216 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -372,9 +372,9 @@ class CoverageMappingBuilder {
// Replace Region with its definition if it is in <scratch space>.
auto NonScratchExpansionLoc = getNonScratchExpansionLoc(Loc);
- Loc = NonScratchExpansionLoc.first;
auto EndLoc = NonScratchExpansionLoc.second;
if (EndLoc.has_value()) {
+ Loc = NonScratchExpansionLoc.first;
Region.setStartLoc(Loc);
Region.setEndLoc(EndLoc.value());
}
More information about the cfe-commits
mailing list