Skip to content

Conversation

llvmbot
Copy link
Member

@llvmbot llvmbot commented Feb 1, 2024

resolves #80285

@llvmbot llvmbot added this to the LLVM 18.X Release milestone Feb 1, 2024
@llvmbot
Copy link
Member Author

llvmbot commented Feb 1, 2024

@cor3ntin What do you think about merging this PR to the release branch?

@llvmbot llvmbot requested a review from cor3ntin February 1, 2024 22:38
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:codegen IR generation bugs: mangling, exceptions, etc. labels Feb 1, 2024
@llvmbot
Copy link
Member Author

llvmbot commented Feb 1, 2024

@llvm/pr-subscribers-clang-codegen

Author: None (llvmbot)

Changes

resolves llvm/llvm-project#80285


Full diff: https://github.com/llvm/llvm-project/pull/80356.diff

2 Files Affected:

  • (modified) clang/lib/CodeGen/CoverageMappingGen.cpp (+4-2)
  • (modified) clang/test/CoverageMapping/if.cpp (+29)
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 5eca00f22bb83..0c43317642bca 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1812,8 +1812,10 @@ struct CounterCoverageMappingBuilder
     assert(S->isConstexpr());
 
     // evaluate constant condition...
-    const auto *E = cast<ConstantExpr>(S->getCond());
-    const bool isTrue = E->getResultAsAPSInt().getExtValue();
+    const bool isTrue =
+        S->getCond()
+            ->EvaluateKnownConstInt(CVM.getCodeGenModule().getContext())
+            .getBoolValue();
 
     extendRegion(S);
 
diff --git a/clang/test/CoverageMapping/if.cpp b/clang/test/CoverageMapping/if.cpp
index 3045ffe43948c..445cdfc20e2af 100644
--- a/clang/test/CoverageMapping/if.cpp
+++ b/clang/test/CoverageMapping/if.cpp
@@ -234,6 +234,35 @@ constexpr int check_macro_consteval_if_skipped(int i) {   // CHECK-NEXT: [[@LINE
   return i;
 }
 
+struct false_value {
+  constexpr operator bool() {
+    return false;
+  }
+};
+
+template <typename> struct dependable_false_value {
+  constexpr operator bool() {
+    return false;
+  }
+};
+
+// GH-80285
+void should_not_crash() {
+  if constexpr (false_value{}) { };
+}
+
+template <typename> void should_not_crash_dependable() {
+  if constexpr (dependable_false_value<int>{}) { };
+}
+
+void should_not_crash_with_template_instance() {
+  should_not_crash_dependable<int>();
+}
+
+void should_not_crash_with_requires_expr() {
+  if constexpr (requires {42;}) { };
+}
+
 int instantiate_consteval(int i) {
   i *= check_consteval_with_else_discarded_then(i);
   i *= check_notconsteval_with_else_discarded_else(i);

@llvmbot
Copy link
Member Author

llvmbot commented Feb 1, 2024

@llvm/pr-subscribers-clang

Author: None (llvmbot)

Changes

resolves llvm/llvm-project#80285


Full diff: https://github.com/llvm/llvm-project/pull/80356.diff

2 Files Affected:

  • (modified) clang/lib/CodeGen/CoverageMappingGen.cpp (+4-2)
  • (modified) clang/test/CoverageMapping/if.cpp (+29)
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 5eca00f22bb83..0c43317642bca 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1812,8 +1812,10 @@ struct CounterCoverageMappingBuilder
     assert(S->isConstexpr());
 
     // evaluate constant condition...
-    const auto *E = cast<ConstantExpr>(S->getCond());
-    const bool isTrue = E->getResultAsAPSInt().getExtValue();
+    const bool isTrue =
+        S->getCond()
+            ->EvaluateKnownConstInt(CVM.getCodeGenModule().getContext())
+            .getBoolValue();
 
     extendRegion(S);
 
diff --git a/clang/test/CoverageMapping/if.cpp b/clang/test/CoverageMapping/if.cpp
index 3045ffe43948c..445cdfc20e2af 100644
--- a/clang/test/CoverageMapping/if.cpp
+++ b/clang/test/CoverageMapping/if.cpp
@@ -234,6 +234,35 @@ constexpr int check_macro_consteval_if_skipped(int i) {   // CHECK-NEXT: [[@LINE
   return i;
 }
 
+struct false_value {
+  constexpr operator bool() {
+    return false;
+  }
+};
+
+template <typename> struct dependable_false_value {
+  constexpr operator bool() {
+    return false;
+  }
+};
+
+// GH-80285
+void should_not_crash() {
+  if constexpr (false_value{}) { };
+}
+
+template <typename> void should_not_crash_dependable() {
+  if constexpr (dependable_false_value<int>{}) { };
+}
+
+void should_not_crash_with_template_instance() {
+  should_not_crash_dependable<int>();
+}
+
+void should_not_crash_with_requires_expr() {
+  if constexpr (requires {42;}) { };
+}
+
 int instantiate_consteval(int i) {
   i *= check_consteval_with_else_discarded_then(i);
   i *= check_notconsteval_with_else_discarded_else(i);

@cor3ntin
Copy link
Contributor

cor3ntin commented Feb 1, 2024

This fixes a regression introduced in clang 18, ship it

@tstellar tstellar merged commit 3eaa7f3 into llvm:release/18.x Feb 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen IR generation bugs: mangling, exceptions, etc. clang Clang issues not falling into any other category
Projects
Development

Successfully merging this pull request may close these issues.

4 participants