[Clang] Correct the DeclRefExpr's Type after the initializer gets instantiated

[zyn0217]

The instantiation of a VarDecl's initializer might be deferred until the variable is actually used. However, we were still building the DeclRefExpr with a type that could later be changed by the initializer's instantiation, which is incorrect when incomplete arrays are involved.

Fixes #79750
Fixes #113936
Fixes #133047

[llvmbot]

@llvm/pr-subscribers-clang

Author: Younan Zhang (zyn0217)

Changes

The instantiation of a VarDecl's initializer might be deferred until the variable is actually used. However, we were still building the DeclRefExpr with a type that could later be changed by the initializer's instantiation, which is incorrect when incomplete arrays are involved.

Fixes #79750
Fixes #113936
Fixes #133047

---

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

3 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+1)
• (modified) clang/lib/Sema/SemaExpr.cpp (+7-4)
• (modified) clang/test/SemaCXX/cxx1y-variable-templates_top_level.cpp (+26)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 8182bccdd2da8..6749369c91559 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -355,6 +355,7 @@ Bug Fixes to C++ Support
 - Correctly diagnoses if unresolved using declarations shadows template paramters (#GH129411)
 - Clang was previously coalescing volatile writes to members of volatile base class subobjects.
   The issue has been addressed by propagating qualifiers during derived-to-base conversions in the AST. (#GH127824)
+- Correctly propagates the instantiated array type to the ``DeclRefExpr`` that refers to it. (#GH79750), (#GH113936), (#GH133047)
 - Fixed a Clang regression in C++20 mode where unresolved dependent call expressions were created inside non-dependent contexts (#GH122892)
 - Clang now emits the ``-Wunused-variable`` warning when some structured bindings are unused
   and the ``[[maybe_unused]]`` attribute is not applied. (#GH125810)
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 3af6d6c23438f..f837b047ddfb4 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -19849,11 +19849,14 @@ static void DoMarkVarDeclReferenced(
           SemaRef.InstantiateVariableDefinition(PointOfInstantiation, Var);
         });
 
-        // Re-set the member to trigger a recomputation of the dependence bits
-        // for the expression.
-        if (auto *DRE = dyn_cast_or_null<DeclRefExpr>(E))
+        if (auto *DRE = dyn_cast_or_null<DeclRefExpr>(E)) {
+          // Re-set the member to trigger a recomputation of the dependence bits
+          // for the expression.
           DRE->setDecl(DRE->getDecl());
-        else if (auto *ME = dyn_cast_or_null<MemberExpr>(E))
+          if (SemaRef.Context.getAsIncompleteArrayType(DRE->getType()) &&
+              !SemaRef.Context.getAsIncompleteArrayType(Var->getType()))
+            DRE->setType(Var->getType());
+        } else if (auto *ME = dyn_cast_or_null<MemberExpr>(E))
           ME->setMemberDecl(ME->getMemberDecl());
       } else if (FirstInstantiation) {
         SemaRef.PendingInstantiations
diff --git a/clang/test/SemaCXX/cxx1y-variable-templates_top_level.cpp b/clang/test/SemaCXX/cxx1y-variable-templates_top_level.cpp
index da1678ec68627..6fc2032ee7fb4 100644
--- a/clang/test/SemaCXX/cxx1y-variable-templates_top_level.cpp
+++ b/clang/test/SemaCXX/cxx1y-variable-templates_top_level.cpp
@@ -467,3 +467,29 @@ namespace VexingParse {
   template <typename> int var; // expected-note {{declared here}}
   int x(var); // expected-error {{use of variable template 'var' requires template arguments}}
 }
+
+#ifndef PRECXX11
+
+namespace GH79750 {
+
+enum class Values { A };
+
+template<typename E>
+constexpr Values values[] = {E::A};
+
+constexpr auto r = values<Values>[0] == Values::A;
+
+}
+
+namespace GH113956 {
+
+template <class T, T... VALUES>
+struct C {
+  static constexpr T VALUEARRAY[] = {VALUES...};
+};
+
+static_assert(C<int, 0,1,2,3,4>::VALUEARRAY[3] == 3, "");
+static_assert(C<int, 0,1,2,3,4>::VALUEARRAY[0] == 0, "");
+
+}
+#endif

[cor3ntin]

Should we do it unconditionally? are there other cases?

[zyn0217]

There are examples where the type of Var is a reference type that couldn't get bound to an expression

template<decltype(auto) v> void f() {
}

struct X {};

template<auto x> auto &TempParamObject = x;
template void f<TempParamObject<X{}>>();

The original type of DeclRefExpr is calculated by BuildDeclarationNameExpr, which is handled by a large switch-cases that I don't think it's reasonable to recompute the type through that logic at this point - It's rare (in fact, the IncompleteArray case is the only one I could think of) that substituting the initializer could change the type of the VarDecl - otherwise, we would have the type of Decl completed before the call to BuildDeclarationNameExpr(), which ensures the correctness of the DeclRefExpr.

[cor3ntin]

Let's just add a comment. I suspect this is not the right place for this code (it feels too late), but i do not have a better suggestion.

[zyn0217]

it feels too late

Hmm. This is the last place where we know the DeclRefExpr that needs an update, (The instantiation doesn't care the DeclRefExpr) and the instantiation happens right before these lines

[cor3ntin]

So, I was looking at where InstantiateVariableDefinition is used.
And I found out we have a completeExprArrayBound function - I suspect we might want to use that. Can you look into it?

[zyn0217]

And I found out we have a completeExprArrayBound function - I suspect we might want to use that. Can you look into it?

I'm surprised we have such a function, thanks! I made it call getCompletedType() which seems to target to the IncompleteArrayTypes and calls completeExprArrayBound() if possible

[cor3ntin]

Thanks

[shafik]

I am assuming you checked by hand that #133047 passes now? It might have been helpful to ask for a reduction to ensure we had fuller test coverage.

[zyn0217]

I did test it locally :) The reduced version of it (I didn't post it) is similar to #113936 so I don't add it to the test coverage.