Mordante accepted this revision.
Mordante added a comment.

I'm happy with the libc++ changes, please wait a few days for @ldionne to react.

In D112921#4584704 <https://reviews.llvm.org/D112921#4584704>, @aaron.ballman 
wrote:

> In D112921#4581641 <https://reviews.llvm.org/D112921#4581641>, @rnk wrote:
>
>> What are the next steps here? Have concerns been addressed? The CI failures 
>> seem unrelated (libcxx tests is an AIX bot out of disk, clang CI is an 
>> interpreter test that seems unrelated, but please confirm).
>
> We definitely need the libc++ reviewers to approve before we move forward. 
> IMO. the clang bits look to be in reasonable shape in terms of code quality.

I leave the approval of the clang part to @aaron.ballman



================
Comment at: 
libcxx/test/libcxx/language.support/support.dynamic/libcpp_deallocate.sh.cpp:29
+// TODO(mordante) fix this test after updating clang in Docker
+// UNSUPPORTED: clang-17
+
----------------
Mordante wrote:
> The bootstrap build fails. Currently main is clang-18. Since you intend to 
> backport the patch it needs to be disabled on clang-17 too.
Based on the timing I'm no longer in favor of backporting this patch.

For the future please mark comments as done when addressed. That makes 
reviewing easier.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112921/new/

https://reviews.llvm.org/D112921

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

Reply via email to