[PATCH] D136815: [clang][Interp] Ignore visitDecl() for local declarations

[Timm Bäder via Phabricator via cfe-commits]

tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, erichkeane, tahonermann, shafik.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

  We reach visitDecl() for both local and global variables, but we used to
  call Program::createGlobal() for both of them. Stop doing that.

I was working on something different and noticed that we register the same 
variable declaration twice.

Clang calls `EvaluateAsInitializer()` for local (const, probably) declarations 
as well, so we shouldn't register those as global variables.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136815

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/ByteCodeStmtGen.cpp


Index: clang/lib/AST/Interp/ByteCodeStmtGen.cpp
===================================================================
--- clang/lib/AST/Interp/ByteCodeStmtGen.cpp
+++ clang/lib/AST/Interp/ByteCodeStmtGen.cpp
@@ -388,8 +388,8 @@
 
 template <class Emitter>
 bool ByteCodeStmtGen<Emitter>::visitVarDecl(const VarDecl *VD) {
-  if (!VD->hasLocalStorage()) {
-    // No code generation required.
+  if (this->isGlobalDecl(VD)) {
+    // Has already been registred as a global variable.
     return true;
   }
 
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===================================================================
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -287,6 +287,12 @@
 
   /// Expression being initialized.
   llvm::Optional<InitFnRef> InitFn = {};
+
+  /// Returns whether we should create a global variable for the
+  /// given VarDecl.
+  bool isGlobalDecl(const VarDecl *VD) const {
+    return !VD->hasLocalStorage() || VD->isConstexpr();
+  }
 };
 
 extern template class ByteCodeExprGen<ByteCodeEmitter>;
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===================================================================
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -847,6 +847,10 @@
                                                           PrimType Ty,
                                                           bool IsConst,
                                                           bool IsExtended) {
+  // Make sure we don't accidentally register the same decl twice.
+  if (const Decl *VD = Src.dyn_cast<const Decl *>(); VD && isa<ValueDecl>(VD))
+    assert(!P.getGlobal(cast<ValueDecl>(VD)));
+
   const Descriptor::MetadataSize MDSize{sizeof(InlineDescriptor)};
   Descriptor *D =
       P.createDescriptor(Src, Ty, MDSize, IsConst, Src.is<const Expr *>());
@@ -860,6 +864,10 @@
 template <class Emitter>
 llvm::Optional<unsigned>
 ByteCodeExprGen<Emitter>::allocateLocal(DeclTy &&Src, bool IsExtended) {
+  // Make sure we don't accidentally register the same decl twice.
+  if (const Decl *VD = Src.dyn_cast<const Decl *>(); VD && isa<ValueDecl>(VD))
+    assert(!P.getGlobal(cast<ValueDecl>(VD)));
+
   const Descriptor::MetadataSize MDSize{sizeof(InlineDescriptor)};
   QualType Ty;
 
@@ -1162,35 +1170,44 @@
 template <class Emitter>
 bool ByteCodeExprGen<Emitter>::visitDecl(const VarDecl *VD) {
   const Expr *Init = VD->getInit();
+  Optional<unsigned> GlobalIndex;
 
-  if (Optional<unsigned> I = P.createGlobal(VD, Init)) {
-    if (Optional<PrimType> T = classify(VD->getType())) {
-      {
-        // Primitive declarations - compute the value and set it.
-        DeclScope<Emitter> LocalScope(this, VD);
-        if (!visit(Init))
-          return false;
-      }
+  if (isGlobalDecl(VD)) {
+    GlobalIndex = P.createGlobal(VD, Init);
+
+    if (!GlobalIndex)
+      return this->bail(VD);
+  }
+
+  if (Optional<PrimType> T = classify(VD->getType())) {
+    {
+      // Primitive declarations - compute the value and set it.
+      DeclScope<Emitter> LocalScope(this, VD);
+      if (!visit(Init))
+        return false;
+    }
 
+    if (GlobalIndex) {
       // If the declaration is global, save the value for later use.
       if (!this->emitDup(*T, VD))
         return false;
-      if (!this->emitInitGlobal(*T, *I, VD))
+      if (!this->emitInitGlobal(*T, *GlobalIndex, VD))
         return false;
-      return this->emitRet(*T, VD);
-    } else {
-      {
-        // Composite declarations - allocate storage and initialize it.
-        DeclScope<Emitter> LocalScope(this, VD);
-        if (!visitGlobalInitializer(Init, *I))
-          return false;
-      }
-
-      // Return a pointer to the global.
-      if (!this->emitGetPtrGlobal(*I, VD))
+    }
+    return this->emitRet(*T, VD);
+  } else {
+    assert(GlobalIndex);
+    {
+      // Composite declarations - allocate storage and initialize it.
+      DeclScope<Emitter> LocalScope(this, VD);
+      if (!visitGlobalInitializer(Init, *GlobalIndex))
         return false;
-      return this->emitRetValue(VD);
     }
+
+    // Return a pointer to the global.
+    if (!this->emitGetPtrGlobal(*GlobalIndex, VD))
+      return false;
+    return this->emitRetValue(VD);
   }
 
   return this->bail(VD);

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits