https://github.com/whentojump updated https://github.com/llvm/llvm-project/pull/89869
>From 968ef430dd09ee4545323426d0c5b550c6a0f690 Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi <geek4ci...@gmail.com> Date: Fri, 19 Apr 2024 15:16:05 +0900 Subject: [PATCH 1/8] [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 | 12 +++++- clang/test/CoverageMapping/builtinmacro.c | 2 +- clang/test/CoverageMapping/macros.c | 8 ++-- .../test/CoverageMapping/mcdc-scratch-space.c | 39 +++++++++++++++++++ 4 files changed, 56 insertions(+), 5 deletions(-) create mode 100644 clang/test/CoverageMapping/mcdc-scratch-space.c diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp index 733686d4946b3..f92706f579a7b 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -339,8 +339,18 @@ class CoverageMappingBuilder { llvm::SmallSet<FileID, 8> Visited; SmallVector<std::pair<SourceLocation, unsigned>, 8> FileLocs; - for (const auto &Region : SourceRegions) { + 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()); + } + FileID File = SM.getFileID(Loc); if (!Visited.insert(File).second) continue; 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 new file mode 100644 index 0000000000000..1b8735cd27445 --- /dev/null +++ b/clang/test/CoverageMapping/mcdc-scratch-space.c @@ -0,0 +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 | FileCheck %s + +// CHECK: builtin_macro0: +int builtin_macro0(int 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) { + // 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 ee6c8b668aac575f7dfb52319703f9ab3006921a Mon Sep 17 00:00:00 2001 From: Wentao Zhang <zhangwt1...@gmail.com> Date: Tue, 23 Apr 2024 22:46:54 -0500 Subject: [PATCH 2/8] [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 f92706f579a7b..9268ac5b4c61a 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -298,6 +298,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>"; @@ -527,7 +543,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 145c807a177db8e649f7f3343d640a000e65cebb Mon Sep 17 00:00:00 2001 From: Wentao Zhang <zhangwt1...@gmail.com> Date: Wed, 24 Apr 2024 14:05:45 -0500 Subject: [PATCH 3/8] 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 9268ac5b4c61a..068f90846d89f 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -292,25 +292,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; } @@ -543,8 +539,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 93ace1a2b2317ed1f040e30ff7a4e2f01beb3a9b Mon Sep 17 00:00:00 2001 From: Wentao Zhang <zhangwt1...@gmail.com> Date: Wed, 24 Apr 2024 16:21:24 -0500 Subject: [PATCH 4/8] 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 068f90846d89f..be2fa16cee66c 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -292,6 +292,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>. @@ -300,11 +319,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; @@ -355,13 +370,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()); FileID File = SM.getFileID(Loc); if (!Visited.insert(File).second) >From 8a40008f1fa85d70eb6a2a7c04ae7df17fbffd82 Mon Sep 17 00:00:00 2001 From: Wentao Zhang <zhangwt1...@gmail.com> Date: Wed, 24 Apr 2024 16:33:55 -0500 Subject: [PATCH 5/8] 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 be2fa16cee66c..ca71b4b039bad 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -316,13 +316,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 0eeb0d3e392b1ea1ae47bc8c977eb709cd1c264c Mon Sep 17 00:00:00 2001 From: Wentao Zhang <zhangwt1...@gmail.com> Date: Wed, 24 Apr 2024 22:47:06 -0500 Subject: [PATCH 6/8] 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 ca71b4b039bad..2b230391d2d8f 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -293,14 +293,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))) { @@ -308,7 +304,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 @@ -321,7 +319,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. @@ -369,12 +369,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()); + } FileID File = SM.getFileID(Loc); if (!Visited.insert(File).second) >From a3656b1381be4dfcc47fa4a23e5adbb212619f03 Mon Sep 17 00:00:00 2001 From: Wentao Zhang <zhangwt1...@gmail.com> Date: Tue, 21 May 2024 16:47:49 -0500 Subject: [PATCH 7/8] 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 2b230391d2d8f..ca71b4b039bad 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -293,10 +293,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))) { @@ -304,9 +308,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 @@ -319,9 +321,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. @@ -369,12 +369,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()); FileID File = SM.getFileID(Loc); if (!Visited.insert(File).second) >From 68a3dfbb7702689f94fa5861e97324c7762c077c Mon Sep 17 00:00:00 2001 From: Wentao Zhang <zhangwt1...@gmail.com> Date: Tue, 21 May 2024 17:23:15 -0500 Subject: [PATCH 8/8] 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 ca71b4b039bad..a4474d911801f 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -372,9 +372,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()); + } FileID File = SM.getFileID(Loc); if (!Visited.insert(File).second) _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits