llvmbot wrote:

<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

<details>
<summary>Changes</summary>

This breaks a ton of libc++ tests otherwise, since calling std::destroy_at will 
currently end the lifetime of the entire array not just the given element.

See https://github.com/llvm/llvm-project/issues/147528

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


3 Files Affected:

- (modified) clang/lib/AST/ByteCode/Interp.cpp (+10) 
- (modified) clang/test/AST/ByteCode/builtin-functions.cpp (+6-4) 
- (modified) clang/test/AST/ByteCode/lifetimes26.cpp (+17) 


``````````diff
diff --git a/clang/lib/AST/ByteCode/Interp.cpp 
b/clang/lib/AST/ByteCode/Interp.cpp
index 931d3879f0ff8..aeab9ff381711 100644
--- a/clang/lib/AST/ByteCode/Interp.cpp
+++ b/clang/lib/AST/ByteCode/Interp.cpp
@@ -1852,6 +1852,11 @@ bool EndLifetime(InterpState &S, CodePtr OpPC) {
   const auto &Ptr = S.Stk.peek<Pointer>();
   if (Ptr.isBlockPointer() && !CheckDummy(S, OpPC, Ptr.block(), AK_Destroy))
     return false;
+
+  // FIXME: We need per-element lifetime information for primitive arrays.
+  if (Ptr.isArrayElement())
+    return true;
+
   endLifetimeRecurse(Ptr.narrow());
   return true;
 }
@@ -1861,6 +1866,11 @@ bool EndLifetimePop(InterpState &S, CodePtr OpPC) {
   const auto &Ptr = S.Stk.pop<Pointer>();
   if (Ptr.isBlockPointer() && !CheckDummy(S, OpPC, Ptr.block(), AK_Destroy))
     return false;
+
+  // FIXME: We need per-element lifetime information for primitive arrays.
+  if (Ptr.isArrayElement())
+    return true;
+
   endLifetimeRecurse(Ptr.narrow());
   return true;
 }
diff --git a/clang/test/AST/ByteCode/builtin-functions.cpp 
b/clang/test/AST/ByteCode/builtin-functions.cpp
index 878c0d1a40f26..3277ef65a880b 100644
--- a/clang/test/AST/ByteCode/builtin-functions.cpp
+++ b/clang/test/AST/ByteCode/builtin-functions.cpp
@@ -1789,9 +1789,11 @@ namespace WithinLifetime {
   } xstd; // both-error {{is not a constant expression}} \
           // both-note {{in call to}}
 
+  /// FIXME: We do not have per-element lifetime information for primitive 
arrays.
+  /// See https://github.com/llvm/llvm-project/issues/147528
   consteval bool test_dynamic(bool read_after_deallocate) {
     std::allocator<int> a;
-    int* p = a.allocate(1);
+    int* p = a.allocate(1); // expected-note 2{{allocation performed here was 
not deallocated}}
     // a.allocate starts the lifetime of an array,
     // the complete object of *p has started its lifetime
     if (__builtin_is_within_lifetime(p))
@@ -1804,12 +1806,12 @@ namespace WithinLifetime {
       return false;
     a.deallocate(p, 1);
     if (read_after_deallocate)
-      __builtin_is_within_lifetime(p); // both-note {{read of heap allocated 
object that has been deleted}}
+      __builtin_is_within_lifetime(p); // ref-note {{read of heap allocated 
object that has been deleted}}
     return true;
   }
-  static_assert(test_dynamic(false));
+  static_assert(test_dynamic(false)); // expected-error {{not an integral 
constant expression}}
   static_assert(test_dynamic(true)); // both-error {{not an integral constant 
expression}} \
-                                     // both-note {{in call to}}
+                                     // ref-note {{in call to}}
 }
 
 #ifdef __SIZEOF_INT128__
diff --git a/clang/test/AST/ByteCode/lifetimes26.cpp 
b/clang/test/AST/ByteCode/lifetimes26.cpp
index a5203ae77bc13..9a1f269b4f55c 100644
--- a/clang/test/AST/ByteCode/lifetimes26.cpp
+++ b/clang/test/AST/ByteCode/lifetimes26.cpp
@@ -47,3 +47,20 @@ constexpr void destroy_pointer() {
 }
 static_assert((destroy_pointer(), true));
 
+
+namespace DestroyArrayElem {
+  /// This is proof that std::destroy_at'ing an array element
+  /// ends the lifetime of the entire array.
+  /// See https://github.com/llvm/llvm-project/issues/147528
+  /// Using destroy_at on array elements is currently a no-op due to this.
+  constexpr int test() {
+    int a[4] = {};
+
+    std::destroy_at(&a[3]);
+    int r = a[1];
+    std::construct_at(&a[3]);
+
+    return r;
+  }
+  static_assert(test() == 0);
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/154119
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to