[clang] [clang][Interp] Fix diagnosing non-const variables pre-C++11 (PR #76718)

Timm Baeder via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 8 01:12:42 PST 2024


Timm =?utf-8?q?Bäder?= <tbaeder at redhat.com>
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/76718 at github.com>


https://github.com/tbaederr updated https://github.com/llvm/llvm-project/pull/76718

>From 64039561ffa3bad6e5a0c98c729478e59777e782 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Thu, 9 Nov 2023 15:45:05 +0100
Subject: [PATCH 1/2] [clang][Interp] Diagnose reads from non-const global
 variables

---
 clang/lib/AST/Interp/ByteCodeExprGen.cpp |  2 +-
 clang/lib/AST/Interp/Interp.cpp          | 47 +++++++++++++++++++-----
 clang/lib/AST/Interp/Interp.h            | 14 +++++++
 clang/lib/AST/Interp/Opcodes.td          |  1 +
 clang/test/AST/Interp/arrays.cpp         | 32 ++++++++++++++++
 clang/test/AST/Interp/cxx23.cpp          | 22 ++++++++---
 clang/test/AST/Interp/literals.cpp       | 27 ++++++++++++--
 7 files changed, 125 insertions(+), 20 deletions(-)

diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
index e6b3097a80d8f7..edbcf4005aedbf 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -2330,7 +2330,7 @@ bool ByteCodeExprGen<Emitter>::visitDecl(const VarDecl *VD) {
     auto GlobalIndex = P.getGlobal(VD);
     assert(GlobalIndex); // visitVarDecl() didn't return false.
     if (VarT) {
-      if (!this->emitGetGlobal(*VarT, *GlobalIndex, VD))
+      if (!this->emitGetGlobalUnchecked(*VarT, *GlobalIndex, VD))
         return false;
     } else {
       if (!this->emitGetPtrGlobal(*GlobalIndex, VD))
diff --git a/clang/lib/AST/Interp/Interp.cpp b/clang/lib/AST/Interp/Interp.cpp
index 21ea2503b94bff..6d4e45dceeacfa 100644
--- a/clang/lib/AST/Interp/Interp.cpp
+++ b/clang/lib/AST/Interp/Interp.cpp
@@ -53,6 +53,21 @@ static bool Jf(InterpState &S, CodePtr &PC, int32_t Offset) {
   return true;
 }
 
+static void diagnoseNonConstVariable(InterpState &S, CodePtr OpPC,
+                                     const ValueDecl *VD) {
+  if (!S.getLangOpts().CPlusPlus)
+    return;
+
+  const SourceInfo &Loc = S.Current->getSource(OpPC);
+  S.FFDiag(Loc,
+           VD->getType()->isIntegralOrEnumerationType()
+               ? diag::note_constexpr_ltor_non_const_int
+               : diag::note_constexpr_ltor_non_constexpr,
+           1)
+      << VD;
+  S.Note(VD->getLocation(), diag::note_declared_at);
+}
+
 static bool CheckActive(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
                         AccessKinds AK) {
   if (Ptr.isActive())
@@ -159,9 +174,7 @@ bool CheckExtern(InterpState &S, CodePtr OpPC, const Pointer &Ptr) {
 
   if (!S.checkingPotentialConstantExpression() && S.getLangOpts().CPlusPlus) {
     const auto *VD = Ptr.getDeclDesc()->asValueDecl();
-    const SourceInfo &Loc = S.Current->getSource(OpPC);
-    S.FFDiag(Loc, diag::note_constexpr_ltor_non_constexpr, 1) << VD;
-    S.Note(VD->getLocation(), diag::note_declared_at);
+    diagnoseNonConstVariable(S, OpPC, VD);
   }
   return false;
 }
@@ -204,6 +217,24 @@ bool CheckLive(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
   return true;
 }
 
+bool CheckConstant(InterpState &S, CodePtr OpPC, const Descriptor *Desc) {
+  assert(Desc);
+  if (const auto *D = Desc->asValueDecl()) {
+    if (const auto *VD = dyn_cast<VarDecl>(D);
+        VD && VD->hasGlobalStorage() &&
+        !(VD->isConstexpr() || VD->getType().isConstQualified())) {
+      diagnoseNonConstVariable(S, OpPC, VD);
+      return false;
+    }
+  }
+
+  return true;
+}
+
+static bool CheckConstant(InterpState &S, CodePtr OpPC, const Pointer &Ptr) {
+  return CheckConstant(S, OpPC, Ptr.getDeclDesc());
+}
+
 bool CheckDummy(InterpState &S, CodePtr OpPC, const Pointer &Ptr) {
   return !Ptr.isZero() && !Ptr.isDummy();
 }
@@ -292,6 +323,8 @@ bool CheckInitialized(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
 bool CheckLoad(InterpState &S, CodePtr OpPC, const Pointer &Ptr) {
   if (!CheckLive(S, OpPC, Ptr, AK_Read))
     return false;
+  if (!CheckConstant(S, OpPC, Ptr))
+    return false;
   if (!CheckDummy(S, OpPC, Ptr))
     return false;
   if (!CheckExtern(S, OpPC, Ptr))
@@ -593,13 +626,7 @@ bool CheckDeclRef(InterpState &S, CodePtr OpPC, const DeclRefExpr *DR) {
     }
   } else if (const auto *VD = dyn_cast<VarDecl>(D)) {
     if (!VD->getType().isConstQualified()) {
-      S.FFDiag(E,
-               VD->getType()->isIntegralOrEnumerationType()
-                   ? diag::note_constexpr_ltor_non_const_int
-                   : diag::note_constexpr_ltor_non_constexpr,
-               1)
-          << VD;
-      S.Note(VD->getLocation(), diag::note_declared_at) << VD->getSourceRange();
+      diagnoseNonConstVariable(S, OpPC, VD);
       return false;
     }
 
diff --git a/clang/lib/AST/Interp/Interp.h b/clang/lib/AST/Interp/Interp.h
index c05dea0cc55d3c..dbbc4c09ce42a1 100644
--- a/clang/lib/AST/Interp/Interp.h
+++ b/clang/lib/AST/Interp/Interp.h
@@ -77,6 +77,9 @@ bool CheckSubobject(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
 /// Checks if a pointer points to const storage.
 bool CheckConst(InterpState &S, CodePtr OpPC, const Pointer &Ptr);
 
+/// Checks if the Descriptor is of a constexpr or const global variable.
+bool CheckConstant(InterpState &S, CodePtr OpPC, const Descriptor *Desc);
+
 /// Checks if a pointer points to a mutable field.
 bool CheckMutable(InterpState &S, CodePtr OpPC, const Pointer &Ptr);
 
@@ -1004,12 +1007,23 @@ bool SetThisField(InterpState &S, CodePtr OpPC, uint32_t I) {
 template <PrimType Name, class T = typename PrimConv<Name>::T>
 bool GetGlobal(InterpState &S, CodePtr OpPC, uint32_t I) {
   const Block *B = S.P.getGlobal(I);
+
+  if (!CheckConstant(S, OpPC, B->getDescriptor()))
+    return false;
   if (B->isExtern())
     return false;
   S.Stk.push<T>(B->deref<T>());
   return true;
 }
 
+/// Same as GetGlobal, but without the checks.
+template <PrimType Name, class T = typename PrimConv<Name>::T>
+bool GetGlobalUnchecked(InterpState &S, CodePtr OpPC, uint32_t I) {
+  auto *B = S.P.getGlobal(I);
+  S.Stk.push<T>(B->deref<T>());
+  return true;
+}
+
 template <PrimType Name, class T = typename PrimConv<Name>::T>
 bool SetGlobal(InterpState &S, CodePtr OpPC, uint32_t I) {
   // TODO: emit warning.
diff --git a/clang/lib/AST/Interp/Opcodes.td b/clang/lib/AST/Interp/Opcodes.td
index 69068e87d5720a..e01b6b9eea7dbb 100644
--- a/clang/lib/AST/Interp/Opcodes.td
+++ b/clang/lib/AST/Interp/Opcodes.td
@@ -379,6 +379,7 @@ def CheckGlobalCtor : Opcode {}
 
 // [] -> [Value]
 def GetGlobal : AccessOpcode;
+def GetGlobalUnchecked : AccessOpcode;
 // [Value] -> []
 def InitGlobal : AccessOpcode;
 // [Value] -> []
diff --git a/clang/test/AST/Interp/arrays.cpp b/clang/test/AST/Interp/arrays.cpp
index 4aa10da55dd3ae..e14ff34dd73716 100644
--- a/clang/test/AST/Interp/arrays.cpp
+++ b/clang/test/AST/Interp/arrays.cpp
@@ -566,3 +566,35 @@ namespace GH69115 {
   static_assert(foo2() == 0, "");
 #endif
 }
+
+namespace NonConstReads {
+#if __cplusplus >= 202002L
+  void *p = nullptr; // ref-note {{declared here}} \
+                     // expected-note {{declared here}}
+
+  int arr[!p]; // ref-error {{not allowed at file scope}} \
+               // expected-error {{not allowed at file scope}} \
+               // ref-warning {{variable length arrays}} \
+               // ref-note {{read of non-constexpr variable 'p'}} \
+               // expected-warning {{variable length arrays}} \
+               // expected-note {{read of non-constexpr variable 'p'}}
+  int z; // ref-note {{declared here}} \
+         // expected-note {{declared here}}
+  int a[z]; // ref-error {{not allowed at file scope}} \
+            // expected-error {{not allowed at file scope}} \
+            // ref-warning {{variable length arrays}} \
+            // ref-note {{read of non-const variable 'z'}} \
+            // expected-warning {{variable length arrays}} \
+            // expected-note {{read of non-const variable 'z'}}
+#else
+  void *p = nullptr;
+  int arr[!p]; // ref-error {{not allowed at file scope}} \
+               // expected-error {{not allowed at file scope}}
+  int z;
+  int a[z]; // ref-error {{not allowed at file scope}} \
+            // expected-error {{not allowed at file scope}}
+#endif
+
+  const int y = 0;
+  int yy[y];
+}
diff --git a/clang/test/AST/Interp/cxx23.cpp b/clang/test/AST/Interp/cxx23.cpp
index bd1cf186d519c5..133ab10023df0e 100644
--- a/clang/test/AST/Interp/cxx23.cpp
+++ b/clang/test/AST/Interp/cxx23.cpp
@@ -25,22 +25,32 @@ constexpr int g(int n) {        // ref20-error {{constexpr function never produc
 }
 
 constexpr int c_thread_local(int n) { // ref20-error {{constexpr function never produces a constant expression}} \
-                                      // ref23-error {{constexpr function never produces a constant expression}}
+                                      // ref23-error {{constexpr function never produces a constant expression}} \
+                                      // expected20-error {{constexpr function never produces a constant expression}} \
+                                      // expected23-error {{constexpr function never produces a constant expression}}
   static _Thread_local int m = 0;     // ref20-note {{control flows through the definition of a thread_local variable}} \
                                       // ref20-warning {{is a C++23 extension}} \
                                       // ref23-note {{control flows through the definition of a thread_local variable}} \
-                                      // expected20-warning {{is a C++23 extension}}
-  return m;
+                                      // expected20-warning {{is a C++23 extension}} \
+                                      // expected20-note {{declared here}} \
+                                      // expected23-note {{declared here}}
+  return m; // expected20-note {{read of non-const variable}} \
+            // expected23-note {{read of non-const variable}}
 }
 
 
 constexpr int gnu_thread_local(int n) { // ref20-error {{constexpr function never produces a constant expression}} \
-                                        // ref23-error {{constexpr function never produces a constant expression}}
+                                        // ref23-error {{constexpr function never produces a constant expression}} \
+                                        // expected20-error {{constexpr function never produces a constant expression}} \
+                                        // expected23-error {{constexpr function never produces a constant expression}}
   static __thread int m = 0;            // ref20-note {{control flows through the definition of a thread_local variable}} \
                                         // ref20-warning {{is a C++23 extension}} \
                                         // ref23-note {{control flows through the definition of a thread_local variable}} \
-                                        // expected20-warning {{is a C++23 extension}}
-  return m;
+                                        // expected20-warning {{is a C++23 extension}} \
+                                        // expected20-note {{declared here}} \
+                                        // expected23-note {{declared here}}
+  return m; // expected20-note {{read of non-const variable}} \
+            // expected23-note {{read of non-const variable}}
 }
 
 constexpr int h(int n) {  // ref20-error {{constexpr function never produces a constant expression}} \
diff --git a/clang/test/AST/Interp/literals.cpp b/clang/test/AST/Interp/literals.cpp
index 85adfe551384d2..5d87ddcebb9115 100644
--- a/clang/test/AST/Interp/literals.cpp
+++ b/clang/test/AST/Interp/literals.cpp
@@ -1177,8 +1177,29 @@ namespace InvalidDeclRefs {
                           // expected-error {{not an integral constant expression}} \
                           // expected-note {{initializer of 'b02' is unknown}}
 
-  /// FIXME: This should also be diagnosed in the new interpreter.
-  int b03 = 3; // ref-note {{declared here}}
+  int b03 = 3; // ref-note {{declared here}} \
+               // expected-note {{declared here}}
   static_assert(b03, ""); // ref-error {{not an integral constant expression}} \
-                          // ref-note {{read of non-const variable}}
+                          // ref-note {{read of non-const variable}} \
+                          // expected-error {{not an integral constant expression}} \
+                          // expected-note {{read of non-const variable}}
+}
+
+namespace NonConstReads {
+  void *p = nullptr; // ref-note {{declared here}} \
+                     // expected-note {{declared here}}
+  static_assert(!p, ""); // ref-error {{not an integral constant expression}} \
+                         // ref-note {{read of non-constexpr variable 'p'}} \
+                         // expected-error {{not an integral constant expression}} \
+                         // expected-note {{read of non-constexpr variable 'p'}}
+
+  int arr[!p]; // ref-error {{variable length array}} \
+               // expected-error {{variable length array}}
+
+  int z; // ref-note {{declared here}} \
+         // expected-note {{declared here}}
+  static_assert(z == 0, ""); // ref-error {{not an integral constant expression}} \
+                             // ref-note {{read of non-const variable 'z'}} \
+                             // expected-error {{not an integral constant expression}} \
+                             // expected-note {{read of non-const variable 'z'}}
 }

>From d20718347ecf9d560ff8059831da3fb12bdaf13d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Sat, 30 Dec 2023 20:46:13 +0100
Subject: [PATCH 2/2] [clang][Interp] Fix diagnosing non-const variables
 pre-C++11

In CheckConstant(), consider that in C++98 const variables may not be
read at all, and diagnose that accordingly.
---
 clang/lib/AST/Interp/Interp.cpp | 42 ++++++++++++++++++++++++++-------
 clang/test/AST/Interp/cxx11.cpp | 24 +++++++++++++++++++
 clang/test/AST/Interp/cxx98.cpp | 36 ++++++++++++++++++++++++++++
 3 files changed, 93 insertions(+), 9 deletions(-)
 create mode 100644 clang/test/AST/Interp/cxx11.cpp
 create mode 100644 clang/test/AST/Interp/cxx98.cpp

diff --git a/clang/lib/AST/Interp/Interp.cpp b/clang/lib/AST/Interp/Interp.cpp
index 6d4e45dceeacfa..327435e5767581 100644
--- a/clang/lib/AST/Interp/Interp.cpp
+++ b/clang/lib/AST/Interp/Interp.cpp
@@ -59,12 +59,16 @@ static void diagnoseNonConstVariable(InterpState &S, CodePtr OpPC,
     return;
 
   const SourceInfo &Loc = S.Current->getSource(OpPC);
-  S.FFDiag(Loc,
-           VD->getType()->isIntegralOrEnumerationType()
-               ? diag::note_constexpr_ltor_non_const_int
-               : diag::note_constexpr_ltor_non_constexpr,
-           1)
-      << VD;
+
+  if (VD->getType()->isIntegralOrEnumerationType())
+    S.FFDiag(Loc, diag::note_constexpr_ltor_non_const_int, 1) << VD;
+  else
+    S.FFDiag(Loc,
+             S.getLangOpts().CPlusPlus11
+                 ? diag::note_constexpr_ltor_non_constexpr
+                 : diag::note_constexpr_ltor_non_integral,
+             1)
+        << VD << VD->getType();
   S.Note(VD->getLocation(), diag::note_declared_at);
 }
 
@@ -219,12 +223,32 @@ bool CheckLive(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
 
 bool CheckConstant(InterpState &S, CodePtr OpPC, const Descriptor *Desc) {
   assert(Desc);
+
+  auto IsConstType = [&S](const VarDecl *VD) -> bool {
+    if (VD->isConstexpr())
+      return true;
+
+    if (S.getLangOpts().CPlusPlus && !S.getLangOpts().CPlusPlus11)
+      return false;
+
+    QualType T = VD->getType();
+    if (T.isConstQualified())
+      return true;
+
+    if (const auto *RT = T->getAs<ReferenceType>())
+      return RT->getPointeeType().isConstQualified();
+
+    if (const auto *PT = T->getAs<PointerType>())
+      return PT->getPointeeType().isConstQualified();
+
+    return false;
+  };
+
   if (const auto *D = Desc->asValueDecl()) {
     if (const auto *VD = dyn_cast<VarDecl>(D);
-        VD && VD->hasGlobalStorage() &&
-        !(VD->isConstexpr() || VD->getType().isConstQualified())) {
+        VD && VD->hasGlobalStorage() && !IsConstType(VD)) {
       diagnoseNonConstVariable(S, OpPC, VD);
-      return false;
+      return S.inConstantContext();
     }
   }
 
diff --git a/clang/test/AST/Interp/cxx11.cpp b/clang/test/AST/Interp/cxx11.cpp
new file mode 100644
index 00000000000000..81e293fec17502
--- /dev/null
+++ b/clang/test/AST/Interp/cxx11.cpp
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify=both,expected -std=c++11 %s
+// RUN: %clang_cc1 -verify=both,ref -std=c++11 %s
+
+// expected-no-diagnostics
+
+namespace IntOrEnum {
+  const int k = 0;
+  const int &p = k;
+  template<int n> struct S {};
+  S<p> s;
+}
+
+const int cval = 2;
+template <int> struct C{};
+template struct C<cval>;
+
+
+/// FIXME: This example does not get properly diagnosed in the new interpreter.
+extern const int recurse1;
+const int recurse2 = recurse1; // ref-note {{here}}
+const int recurse1 = 1;
+int array1[recurse1];
+int array2[recurse2]; // ref-warning 2{{variable length array}} \
+                      // ref-note {{initializer of 'recurse2' is not a constant expression}}
diff --git a/clang/test/AST/Interp/cxx98.cpp b/clang/test/AST/Interp/cxx98.cpp
new file mode 100644
index 00000000000000..bc96723c2287da
--- /dev/null
+++ b/clang/test/AST/Interp/cxx98.cpp
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify=both,expected -std=c++98 %s
+// RUN: %clang_cc1 -verify=both,ref -std=c++98 %s
+
+
+
+namespace IntOrEnum {
+  const int k = 0;
+  const int &p = k; // both-note {{declared here}}
+  template<int n> struct S {};
+  S<p> s; // both-error {{not an integral constant expression}} \
+          // both-note {{read of variable 'p' of non-integral, non-enumeration type 'const int &'}}
+}
+
+const int cval = 2;
+template <int> struct C{};
+template struct C<cval>;
+
+
+/// FIXME: This example does not get properly diagnosed in the new interpreter.
+extern const int recurse1;
+const int recurse2 = recurse1; // ref-note {{here}}
+const int recurse1 = 1;
+int array1[recurse1];
+int array2[recurse2]; // ref-warning 2{{variable length array}} \
+                      // ref-note {{initializer of 'recurse2' is not a constant expression}} \
+                      // expected-warning {{variable length array}} \
+                      // expected-error {{variable length array}}
+
+int NCI; // expected-note {{declared here}} \
+         // ref-note {{declared here}}
+int NCIA[NCI]; // expected-warning {{variable length array}} \
+               // expected-error {{variable length array}} \\
+               // expected-note {{read of non-const variable 'NCI'}} \
+               // ref-warning {{variable length array}} \
+               // ref-error {{variable length array}} \\
+               // ref-note {{read of non-const variable 'NCI'}}



More information about the cfe-commits mailing list