[clang][Interp] Implement DecompositionDecls

[tbaeder]

Summary

Factor out the actual variable allocation logic into a separate function, so we can call it multiple times from visitVarDecl() if the variable declaration is a DecompositionDecl().

[shafik]

clang/lib/AST/Interp/ByteCodeExprGen.h

269–270

clang/test/AST/Interp/cxx17.cpp

8

It would also be good to test references, bit-fields, volatile and tuple-like types if possible, if not then leave a FIXME.

[tbaeder]

clang/test/AST/Interp/cxx17.cpp

8

Clang doesn't seem to support volatile in constant expressions at all right now(?). Can you come up with a reproducer that works? All I get is errors about a non-literal type: https://godbolt.org/z/PbevGf3E8

[tbaeder]

Add more tests and fix decomposition decls of reference type.

[shafik]

clang/test/AST/Interp/cxx17.cpp

8

Apologies, my mistake. In my enthusiasm I forgot that while constexpr volatile is a thing the variable is not usable in a constant expression.

[tbaeder]

Ping

[aaron.ballman]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

34–35

The destructor for LocalScope already calls emitDestruction() which is a virtual function, so is this necessary?

1537–1539

1548–1551

1571–1572

<uncertain>Is this correct? IIRC, the decomposition declaration is its own object, but the bindings themselves are references back to the decomposition declaration object directly and so they're not distinct objects themselves (they're more like aliases).</uncertain>

[tbaeder]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

1571–1572

Is this not reflected in the individual bindings? What does it mean that the bindings aren't "distinct objects themselves"?

[aaron.ballman]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

1571–1572

Bindings are basically a label back to an object. Taking the easy case of a structure being bound:

struct S { int i, j; } s;

int main() {
  auto [val1, val2] = s;
  return val1 + val2;
}

The way this works under the hood is akin to:

struct S { int i, j; } s;

int main() {
  S __s = s; // This is the decomposition declaration
  return __s.i + __s.j; // And the structured bindings give alternative names to the fields in the decomposition declaration
}

so there's no allocation made for val1 or val2 because they're not really objects, just names. You can see that in: https://godbolt.org/z/sdj3Mvqhb (note the LLVM IR, which is identical between foo and bar aside from debug info).

[tbaeder]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

1571–1572

Ah, I see. I guess my way works as well, but yours is better, so I'll change the patch. I can basically replace the added stuff in VisitDeclRefExpr() by just return this->visit(BD->getBinding()) and that will give me the right thing. I hope that way I can remove some of the BindingDecl special cases I've added as well.

Thanks!

[aaron.ballman]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

34–35

Still wondering about this

1536

Spurious whitespace change

clang/test/AST/Interp/cxx17.cpp

31

It's be helpful to have a test where a and b are modified to demonstrate we change the values in f as expected. e.g.,
`
F f = getF();
auto &[a, b] = f;
a += 1;
b += 2;

return f.a + f.b;
`

54–56

Spurious whitespace

[tbaeder]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

34–35

You're right, removed it again. Thanks.

[tbaeder]

Ping

[aaron.ballman]

LGTM!

clang/lib/AST/Interp/ByteCodeExprGen.h

private:
   const CXXRecordDecl *getRecordDecl(const Expr *E) const {
     QualType T = E->getType();
     if (const auto *RD = T->getPointeeCXXRecordDecl())
       return RD;
     return T->getAsCXXRecordDecl();
   }
 
   /// Returns whether we should create a global variable for the
-  /// given VarDecl.
-  bool shouldBeGloballyIndexed(const VarDecl *VD) const {
-    return VD->hasGlobalStorage() || VD->isConstexpr();
+  /// given ValueDecl.
+  bool shouldBeGloballyIndexed(const ValueDecl *VD) const {
+    if (const auto *V = dyn_cast<VarDecl>(VD))
+      return V->hasGlobalStorage() || V->isConstexpr();
+
+    return false;
   }
 
   llvm::RoundingMode getRoundingMode(const Expr *E) const {
     FPOptions FPO = E->getFPFeaturesInEffect(Ctx.getLangOpts());
 
     if (FPO.getRoundingMode() == llvm::RoundingMode::Dynamic)
       return llvm::RoundingMode::NearestTiesToEven;
 
protected:
 
   /// Current scope.
   VariableScope<Emitter> *VarScope = nullptr;
 
   /// Current argument index. Needed to emit ArrayInitIndexExpr.
   std::optional<uint64_t> ArrayIndex;
 
   /// Flag indicating if return value is to be discarded.
   bool DiscardResult = false;
 };
 
 extern template class ByteCodeExprGen<ByteCodeEmitter>;
 extern template class ByteCodeExprGen<EvalEmitter>;
 
 /// Scope chain managing the variable lifetimes.
 template <class Emitter> class VariableScope {
 public:
   VariableScope(ByteCodeExprGen<Emitter> *Ctx)

clang/lib/AST/Interp/ByteCodeExprGen.cpp

 using APSInt = llvm::APSInt;
 
 namespace clang {
 namespace interp {
 
 /// Scope used to handle temporaries in toplevel variable declarations.
 template <class Emitter> class DeclScope final : public LocalScope<Emitter> {
 public:
-  DeclScope(ByteCodeExprGen<Emitter> *Ctx, const VarDecl *VD)
+  DeclScope(ByteCodeExprGen<Emitter> *Ctx, const ValueDecl *VD)
       : LocalScope<Emitter>(Ctx), Scope(Ctx->P, VD) {}
 
   void addExtended(const Scope::Local &Local) override {
     return this->addLocal(Local);
   }
 
 private:
   Program::DeclScope Scope;
 };
 
 /// Scope used to handle initialization methods.
 template <class Emitter> class OptionScope {

 
 template <class Emitter>
 bool ByteCodeExprGen<Emitter>::dereference(
     const Expr *LV, DerefKind AK, llvm::function_ref<bool(PrimType)> Direct,
     llvm::function_ref<bool(PrimType)> Indirect) {
   if (std::optional<PrimType> T = classify(LV->getType())) {
     if (!LV->refersToBitField()) {
       // Only primitive, non bit-field types can be dereferenced directly.
-      if (auto *DE = dyn_cast<DeclRefExpr>(LV)) {
+      if (const auto *DE = dyn_cast<DeclRefExpr>(LV)) {
         if (!DE->getDecl()->getType()->isReferenceType()) {
-          if (auto *PD = dyn_cast<ParmVarDecl>(DE->getDecl()))
+          if (const auto *PD = dyn_cast<ParmVarDecl>(DE->getDecl()))
             return dereferenceParam(LV, *T, PD, AK, Direct, Indirect);
-          if (auto *VD = dyn_cast<VarDecl>(DE->getDecl()))
+          if (const auto *VD = dyn_cast<VarDecl>(DE->getDecl()))
             return dereferenceVar(LV, *T, VD, AK, Direct, Indirect);
         }
       }
     }
 
     if (!visit(LV))
       return false;
     return Indirect(*T);
assert(Init);
         if (!this->visit(Init))
           return false;
         return this->emitInitGlobal(*VarT, *GlobalIndex, VD);
       }
       return this->visitGlobalInitializer(Init, *GlobalIndex);
     }
   } else {
     VariableScope<Emitter> LocalScope(this);
     if (VarT) {
       unsigned Offset = this->allocateLocalPrimitive(
           VD, *VarT, VD->getType().isConstQualified());
       if (Init) {
         // Compile the initializer in its own scope.
         ExprScope<Emitter> Scope(this);
         if (!this->visit(Init))
           return false;
 
         return this->emitSetLocal(*VarT, Offset, VD);
       }
     } else {
       if (std::optional<unsigned> Offset = this->allocateLocal(VD)) {
         if (Init)
           return this->visitLocalInitializer(Init, *Offset);
       }
     }
     return true;
   }
 
   return false;
 }
 
 template <class Emitter>
 bool ByteCodeExprGen<Emitter>::VisitBuiltinCallExpr(const CallExpr *E) {
   const Function *Func = getFunction(E->getDirectCallee());
   if (!Func)
     return false;
 
   // Put arguments on the stack.
   for (const auto *Arg : E->arguments()) {
     if (!this->visit(Arg))
       return false;
   }
 
   if (!this->emitCallBI(Func, E))
     return false;
 
   QualType ReturnType = E->getCallReturnType(Ctx.getASTContext());
   if (DiscardResult && !ReturnType->isVoidType()) {
     PrimType T = classifyPrim(ReturnType);
     return this->emitPop(T, E);
   }
 
   return true;
bool ByteCodeExprGen<Emitter>::VisitDeclRefExpr(const DeclRefExpr *E) {
   } else if (const auto *PVD = dyn_cast<ParmVarDecl>(Decl)) {
     if (auto It = this->Params.find(PVD); It != this->Params.end()) {
       if (IsReference)
         return this->emitGetParam(PT_Ptr, It->second, E);
       return this->emitGetPtrParam(It->second, E);
     }
   } else if (const auto *ECD = dyn_cast<EnumConstantDecl>(Decl)) {
     return this->emitConst(ECD->getInitVal(), E);
+  } else if (const auto *BD = dyn_cast<BindingDecl>(Decl)) {
+    return this->visit(BD->getBinding());
   }
 
   return false;
 }
 
 template <class Emitter>
 void ByteCodeExprGen<Emitter>::emitCleanup() {
   for (VariableScope<Emitter> *C = VarScope; C; C = C->getParent())

clang/lib/AST/Interp/ByteCodeStmtGen.cpp

for (auto *InnerStmt : CompoundStmt->body())
     if (!visitStmt(InnerStmt))
       return false;
   return true;
 }
 
 template <class Emitter>
 bool ByteCodeStmtGen<Emitter>::visitDeclStmt(const DeclStmt *DS) {
   for (auto *D : DS->decls()) {
-    // Variable declarator.
-    if (auto *VD = dyn_cast<VarDecl>(D)) {
-      if (!this->visitVarDecl(VD))
-        return false;
-      continue;
-    }
-
-    // Decomposition declarator.
-    if (auto *DD = dyn_cast<DecompositionDecl>(D)) {
-      return this->bail(DD);
-    }
+    const auto *VD = dyn_cast<VarDecl>(D);
+    if (!VD)
+      return false;
   }
 
   return true;
 }
 
 template <class Emitter>
 bool ByteCodeStmtGen<Emitter>::visitReturnStmt(const ReturnStmt *RS) {
   if (const Expr *RE = RS->getRetValue()) {

clang/lib/AST/Interp/Program.h

Descriptor *createDescriptor(const DeclTy &D, const Type *Ty,
                                Descriptor::MetadataSize MDSize = std::nullopt,
                                bool IsConst = false, bool IsTemporary = false,
                                bool IsMutable = false,
                                const Expr *Init = nullptr);
 
   /// Context to manage declaration lifetimes.
   class DeclScope {
   public:
-    DeclScope(Program &P, const VarDecl *VD) : P(P) { P.startDeclaration(VD); }
+    DeclScope(Program &P, const ValueDecl *VD) : P(P) {
+      P.startDeclaration(VD);
+    }
     ~DeclScope() { P.endDeclaration(); }
 
   private:
     Program &P;
   };
 
   /// Returns the current declaration ID.
   std::optional<unsigned> getCurrentDecl() const {
private:
   /// No declaration ID.
   static constexpr unsigned NoDeclaration = (unsigned)-1;
   /// Last declaration ID.
   unsigned LastDeclaration = 0;
   /// Current declaration ID.
   unsigned CurrentDeclaration = NoDeclaration;
 
   /// Starts evaluating a declaration.
-  void startDeclaration(const VarDecl *Decl) {
+  void startDeclaration(const ValueDecl *Decl) {
     LastDeclaration += 1;
     CurrentDeclaration = LastDeclaration;
   }
 
   /// Ends a global declaration.
   void endDeclaration() {
     CurrentDeclaration = NoDeclaration;
   }

clang/test/AST/Interp/cxx17.cpp

+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++17 -verify %s
+// RUN: %clang_cc1 -std=c++17 -verify=ref %s
+
+// ref-no-diagnostics
+
+struct F { int a; int b;};
+constexpr F getF() {
+  return {12, 3};
+}
+constexpr int f() {
+  auto [a1, b1] = getF();
+  auto [a2, b2] = getF();
+
+  return a1 + a2 + b1 + b2;
+}
+static_assert(f() == 30);
+
+
+constexpr int structRefs() {
+  const auto &[a, b] = getF();
+
+  return a + b;
+}
+// FIXME: This should work, but the MaterializeTemporaryExpr handling is not ready for it.
+static_assert(structRefs() == 15); // expected-error {{not an integral constant expression}}
+
+constexpr int structRefs2() {
+  F f = getF();
+  const auto &[a, b] = f;
+
+  return a + b;
+}
+static_assert(structRefs2() == 15);
+
+
+template<typename T>
+struct Tuple {
+  T first;
+  T second;
+  constexpr Tuple(T a, T b) : first(a), second(b) {}
+};
+template<typename T>
+constexpr T addTuple(const Tuple<T> &Tu) {
+  auto [a, b] = Tu;
+  return a + b;
+}
+
+template<typename T>
+constexpr T addTuple2(const Tuple<T> &Tu) {
+  auto [a, b] = Tu;
+  return Tu.first + Tu.second;
+}
+
+constexpr Tuple<int> T1 = Tuple(1,2);
+static_assert(addTuple(T1) == 3);
+static_assert(addTuple2(T1) == 3);
+
+constexpr Tuple<short> T2 = Tuple<short>(11,2);
+static_assert(addTuple(T2) == 13);
+static_assert(addTuple2(T2) == 13);
+
+constexpr int Modify() {
+  auto T = Tuple<int>(10, 20);
+  auto &[x, y] = T;
+  x += 1;
+  y += 1;
+  return T.first + T.second;
+}
+static_assert(Modify() == 32, "");
+
+constexpr int a() {
+  int a[2] = {5, 3};
+  auto [x, y] = a;
+  return x + y;
+}
+static_assert(a() == 8);
+
+constexpr int b() {
+  int a[2] = {5, 3};
+  auto &[x, y] = a;
+  x += 1;
+  y += 2;
+  return a[0] + a[1];
+}
+static_assert(b() == 11);