Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang][ExprConst] Don't try to evaluate value-dependent DeclRefExprs #67778

Merged
merged 1 commit into from
Oct 5, 2023

Conversation

tbaederr
Copy link
Contributor

The Expression here migth be value dependent, which makes us run into an assertion later on. Just bail out early.

Fixes #67690

@tbaederr tbaederr requested review from cor3ntin and shafik September 29, 2023 09:25
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Sep 29, 2023
@llvmbot
Copy link
Member

llvmbot commented Sep 29, 2023

@llvm/pr-subscribers-clang

Changes

The Expression here migth be value dependent, which makes us run into an assertion later on. Just bail out early.

Fixes #67690


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

2 Files Affected:

  • (modified) clang/lib/AST/ExprConstant.cpp (+3)
  • (modified) clang/test/SemaCXX/constant-expression-cxx1z.cpp (+13)
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index fea06b97259fe31..8372d81234669fd 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -3357,6 +3357,9 @@ static bool evaluateVarDeclInit(EvalInfo &Info, const Expr *E,
     return false;
   }
 
+  if (E->isValueDependent())
+    return false;
+
   // Dig out the initializer, and use the declaration which it's attached to.
   // FIXME: We should eventually check whether the variable has a reachable
   // initializing declaration.
diff --git a/clang/test/SemaCXX/constant-expression-cxx1z.cpp b/clang/test/SemaCXX/constant-expression-cxx1z.cpp
index 9335626a5c90a4f..a36b8f15f826f41 100644
--- a/clang/test/SemaCXX/constant-expression-cxx1z.cpp
+++ b/clang/test/SemaCXX/constant-expression-cxx1z.cpp
@@ -177,3 +177,16 @@ namespace LambdaCallOp {
     p();
   }
 }
+
+/// This used to crash due to an assertion failure,
+/// see gh#67690
+namespace {
+  struct C {
+    int x;
+  };
+
+  template <const C *p> void f() {
+    const auto &[c] = *p;
+    &c;
+  }
+}

Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs a release note, Otherwise LGTM

@tbaederr
Copy link
Contributor Author

Should we get crash fixes like this into clang 17 as well?

@tbaederr tbaederr force-pushed the 67690 branch 2 times, most recently from 8d771fa to 22339d9 Compare October 3, 2023 18:11
The Expression here migth be value dependent, which makes us run into an
assertion later on. Just bail out early.

Fixes llvm#67690
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[clang] Crash from constexpr-evaluation of structured-binding variable
4 participants