Skip to content

Conversation

@hnrklssn
Copy link
Member

@hnrklssn hnrklssn commented Jul 7, 2025

EvaluateAsInitializer does not support evaluating values with dependent types. This was previously guarded with a check for the initializer expression, but it is possible for the VarDecl to have a dependent type without the initializer having a dependent type, when the initializer is a specialized template type and the VarDecl has the unspecialized type. This adds a guard checking for dependence in the VarDecl type as well. This fixes the issue raised by Google in #145447

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules labels Jul 7, 2025
@llvmbot
Copy link
Member

llvmbot commented Jul 7, 2025

@llvm/pr-subscribers-clang-modules

@llvm/pr-subscribers-clang

Author: Henrik G. Olsson (hnrklssn)

Changes

EvaluateAsInitializer does not support evaluating values with dependent types. This was previously guarded with a check for the initializer expression, but it is possible for the VarDecl to have a dependent type without the initializer having a dependent type, when the initializer is a specialized template type and the VarDecl has the unspecialized type. This adds a guard checking for dependence in the VarDecl type as well. This fixes the issue raised by Google in #145447


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

2 Files Affected:

  • (modified) clang/lib/AST/Decl.cpp (+1-1)
  • (added) clang/test/Modules/var-init-side-effects-templated.cpp (+20)
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index e0362245d6ecd..f25e9fe7e8f2f 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -2450,7 +2450,7 @@ bool VarDecl::hasInitWithSideEffects() const {
     ES->HasSideEffects =
         E->HasSideEffects(getASTContext()) &&
         // We can get a value-dependent initializer during error recovery.
-        (E->isValueDependent() || !evaluateValue());
+        (E->isValueDependent() || getType()->isDependentType() || !evaluateValue());
     ES->CheckedForSideEffects = true;
   }
   return ES->HasSideEffects;
diff --git a/clang/test/Modules/var-init-side-effects-templated.cpp b/clang/test/Modules/var-init-side-effects-templated.cpp
new file mode 100644
index 0000000000000..542ca270429b6
--- /dev/null
+++ b/clang/test/Modules/var-init-side-effects-templated.cpp
@@ -0,0 +1,20 @@
+// Tests referencing variable with initializer containing side effect across module boundary
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -o %t
+
+export module Foo;
+
+export template <class Float>
+struct Wrapper {
+  double value;
+};
+
+export constexpr Wrapper<double> Compute() {
+  return Wrapper<double>{1.0};
+}
+
+export template <typename Float>
+Wrapper<Float> ComputeInFloat() {
+  const Wrapper<Float> a = Compute();
+  return a;
+}

@github-actions
Copy link

github-actions bot commented Jul 7, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

EvaluateAsInitializer does not support evaluating values with dependent
types. This was previously guarded with a check for the initializer
expression, but it is possible for the VarDecl to have a dependent type
without the initializer having a dependent type, when the initializer is
a specialized template type and the VarDecl has the unspecialized type.
This adds a guard checking for dependence in the VarDecl type as well.
@hnrklssn hnrklssn force-pushed the fix-hasInitWithSideEffects2 branch from 793b129 to 6c494ad Compare July 7, 2025 19:27
@hnrklssn hnrklssn merged commit c885005 into llvm:main Jul 7, 2025
9 checks passed
hnrklssn added a commit to hnrklssn/llvm-project that referenced this pull request Jul 7, 2025
EvaluateAsInitializer does not support evaluating values with dependent
types. This was previously guarded with a check for the initializer
expression, but it is possible for the VarDecl to have a dependent type
without the initializer having a dependent type, when the initializer is
a specialized template type and the VarDecl has the unspecialized type.
This adds a guard checking for dependence in the VarDecl type as well.
This fixes the issue raised by Google in
llvm#145447

(cherry picked from commit c885005)
@bgra8
Copy link
Contributor

bgra8 commented Jul 8, 2025

Thanks for the quick fix @hnrklssn !!

hokein pushed a commit to hokein/llvm-project that referenced this pull request Jul 10, 2025
EvaluateAsInitializer does not support evaluating values with dependent
types. This was previously guarded with a check for the initializer
expression, but it is possible for the VarDecl to have a dependent type
without the initializer having a dependent type, when the initializer is
a specialized template type and the VarDecl has the unspecialized type.
This adds a guard checking for dependence in the VarDecl type as well.
This fixes the issue raised by Google in
llvm#145447
hokein pushed a commit to hokein/llvm-project that referenced this pull request Jul 16, 2025
EvaluateAsInitializer does not support evaluating values with dependent
types. This was previously guarded with a check for the initializer
expression, but it is possible for the VarDecl to have a dependent type
without the initializer having a dependent type, when the initializer is
a specialized template type and the VarDecl has the unspecialized type.
This adds a guard checking for dependence in the VarDecl type as well.
This fixes the issue raised by Google in
llvm#145447
hnrklssn added a commit to hnrklssn/llvm-project that referenced this pull request Sep 11, 2025
EvaluateAsInitializer does not support evaluating values with dependent
types. This was previously guarded with a check for the initializer
expression, but it is possible for the VarDecl to have a dependent type
without the initializer having a dependent type, when the initializer is
a specialized template type and the VarDecl has the unspecialized type.
This adds a guard checking for dependence in the VarDecl type as well.
This fixes the issue raised by Google in
llvm#145447

(cherry picked from commit c885005)
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:modules C++20 modules and Clang Header Modules clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants