Skip to content

[clang][bytecode] Add Descriptor::hasTrivialDtor() #146286

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 30, 2025
Merged

Conversation

tbaederr
Copy link
Contributor

We sometimes used to have a long list of

  GetLocalPtr
  PopPtr
  [...]

ops at the end of scopes, because we first got a pointer to a local variable and only then did we figure out that we didn't actually want to call the destructor for it. Add a new function that allows us to just ask the Descriptor whether we need to call its destructor.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:bytecode Issues for the clang bytecode constexpr interpreter labels Jun 29, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 29, 2025

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

We sometimes used to have a long list of

  GetLocalPtr
  PopPtr
  [...]

ops at the end of scopes, because we first got a pointer to a local variable and only then did we figure out that we didn't actually want to call the destructor for it. Add a new function that allows us to just ask the Descriptor whether we need to call its destructor.


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

3 Files Affected:

  • (modified) clang/lib/AST/ByteCode/Compiler.h (+9-9)
  • (modified) clang/lib/AST/ByteCode/Descriptor.cpp (+15)
  • (modified) clang/lib/AST/ByteCode/Descriptor.h (+3)
diff --git a/clang/lib/AST/ByteCode/Compiler.h b/clang/lib/AST/ByteCode/Compiler.h
index a1d068cc7e0ae..1a5fd86785872 100644
--- a/clang/lib/AST/ByteCode/Compiler.h
+++ b/clang/lib/AST/ByteCode/Compiler.h
@@ -574,17 +574,17 @@ template <class Emitter> class LocalScope : public VariableScope<Emitter> {
     // Emit destructor calls for local variables of record
     // type with a destructor.
     for (Scope::Local &Local : llvm::reverse(this->Ctx->Descriptors[*Idx])) {
-      if (!Local.Desc->isPrimitive() && !Local.Desc->isPrimitiveArray()) {
-        if (!this->Ctx->emitGetPtrLocal(Local.Offset, E))
-          return false;
+      if (Local.Desc->hasTrivialDtor())
+        continue;
+      if (!this->Ctx->emitGetPtrLocal(Local.Offset, E))
+        return false;
 
-        if (!this->Ctx->emitDestruction(Local.Desc, Local.Desc->getLoc()))
-          return false;
+      if (!this->Ctx->emitDestruction(Local.Desc, Local.Desc->getLoc()))
+        return false;
 
-        if (!this->Ctx->emitPopPtr(E))
-          return false;
-        removeIfStoredOpaqueValue(Local);
-      }
+      if (!this->Ctx->emitPopPtr(E))
+        return false;
+      removeIfStoredOpaqueValue(Local);
     }
     return true;
   }
diff --git a/clang/lib/AST/ByteCode/Descriptor.cpp b/clang/lib/AST/ByteCode/Descriptor.cpp
index 46e4d0d940b3e..ec0c838fd1ae5 100644
--- a/clang/lib/AST/ByteCode/Descriptor.cpp
+++ b/clang/lib/AST/ByteCode/Descriptor.cpp
@@ -502,6 +502,21 @@ SourceInfo Descriptor::getLoc() const {
   llvm_unreachable("Invalid descriptor type");
 }
 
+bool Descriptor::hasTrivialDtor() const {
+  if (isPrimitive() || isPrimitiveArray())
+    return true;
+
+  if (isRecord()) {
+    assert(ElemRecord);
+    const CXXDestructorDecl *Dtor = ElemRecord->getDestructor();
+    return !Dtor || Dtor->isTrivial();
+  }
+
+  // Composite arrays.
+  assert(ElemDesc);
+  return ElemDesc->hasTrivialDtor();
+}
+
 bool Descriptor::isUnion() const { return isRecord() && ElemRecord->isUnion(); }
 
 InitMap::InitMap(unsigned N)
diff --git a/clang/lib/AST/ByteCode/Descriptor.h b/clang/lib/AST/ByteCode/Descriptor.h
index f25ad8f4c758c..4591eabb69bb4 100644
--- a/clang/lib/AST/ByteCode/Descriptor.h
+++ b/clang/lib/AST/ByteCode/Descriptor.h
@@ -281,6 +281,9 @@ struct Descriptor final {
   /// Checks if this is a dummy descriptor.
   bool isDummy() const { return IsDummy; }
 
+  /// Whether variables of this descriptor need their destructor called or not.
+  bool hasTrivialDtor() const;
+
   void dump() const;
   void dump(llvm::raw_ostream &OS) const;
   void dumpFull(unsigned Offset = 0, unsigned Indent = 0) const;

@tbaederr tbaederr merged commit 00cdaa5 into llvm:main Jun 30, 2025
7 checks passed
rlavaee pushed a commit to rlavaee/llvm-project that referenced this pull request Jul 1, 2025
We sometimes used to have a long list of 

```
  GetLocalPtr
  PopPtr
  [...]
```

ops at the end of scopes, because we first got a pointer to a local
variable and only then did we figure out that we didn't actually want to
call the destructor for it. Add a new function that allows us to just
ask the `Descriptor` whether we need to call its destructor.
rlavaee pushed a commit to rlavaee/llvm-project that referenced this pull request Jul 1, 2025
We sometimes used to have a long list of 

```
  GetLocalPtr
  PopPtr
  [...]
```

ops at the end of scopes, because we first got a pointer to a local
variable and only then did we figure out that we didn't actually want to
call the destructor for it. Add a new function that allows us to just
ask the `Descriptor` whether we need to call its destructor.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:bytecode Issues for the clang bytecode constexpr interpreter clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants