This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Granularize the rest of memory
ClosedPublic

Authored by philnik on Aug 27 2022, 8:43 AM.

Diff Detail

Event Timeline

philnik created this revision.Aug 27 2022, 8:43 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 27 2022, 8:43 AM
Herald added a subscriber: mgorny. · View Herald Transcript
philnik requested review of this revision.Aug 27 2022, 8:43 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 27 2022, 8:43 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
philnik updated this revision to Diff 457213.Sep 1 2022, 2:59 AM
  • generate files
philnik updated this revision to Diff 457220.Sep 1 2022, 3:43 AM
  • Try to fix CI
philnik updated this revision to Diff 457594.Sep 2 2022, 7:24 AM
  • Try to fix CI
ldionne accepted this revision.Sep 2 2022, 8:24 AM

LGTM, thanks for the cleanup! I would also remove the "and remove transitive includes" from the commit message, since it does not really remove any user visible transitive includes.

libcxx/include/__memory/destruct_n.h
24

Side comment -- in a separate patch, let's investigate this. There really shouldn't be any logic in here, we should just call std::destroy_n. That should handle trivially destructible types and all.

This seems to be used as a RAII cleanup in some algorithms.

libcxx/include/algorithm
1900–1901

I think this needs to be rebased, since I would expect a check for && C++ <= 20 here (or something like that).

libcxx/include/memory
880–884

&& C++ <= 20?

This revision is now accepted and ready to land.Sep 2 2022, 8:24 AM
philnik retitled this revision from [libc++] Granularize the rest of memory and remove transitive includes to [libc++] Granularize the rest of memory.Sep 2 2022, 12:38 PM
This revision was automatically updated to reflect the committed changes.
philnik marked 2 inline comments as done.

Hi, I'm pretty sure we see a breakage in the llvm-libc++-static.cfg.in :: libcxx/transitive_includes.sh.cpp test due to this patch in Fuchsia's Clang CI.

Is it possible to take a look? I don't see an obvious reason we see this on x64 linux, but not other platforms, so maybe there's just something missed in the preprocessor directives?

The failing bot can be found here: https://ci.chromium.org/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8804103688804713457/overview

This revision is now accepted and ready to land.Sep 2 2022, 7:39 PM
philnik updated this revision to Diff 457785.Sep 3 2022, 5:50 AM
  • Fix transitive_includes test
philnik updated this revision to Diff 457849.Sep 4 2022, 4:26 AM
  • Try to fix CI
philnik updated this revision to Diff 457853.Sep 4 2022, 5:01 AM
  • Try to fix CI
philnik updated this revision to Diff 457859.Sep 4 2022, 7:38 AM
  • Add <memory> include to <functional>
philnik updated this revision to Diff 457865.Sep 4 2022, 8:44 AM
  • Next try
philnik updated this revision to Diff 457866.Sep 4 2022, 9:04 AM
  • Next try
Herald added a project: Restricted Project. · View Herald TranscriptSep 4 2022, 9:04 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
This revision now requires review to proceed.Sep 4 2022, 9:04 AM
philnik updated this revision to Diff 457869.Sep 4 2022, 11:04 AM
  • next try
philnik updated this revision to Diff 457873.Sep 4 2022, 3:01 PM
  • Another try
This revision was not accepted when it landed; it landed in state Needs Review.Sep 5 2022, 3:36 AM
This revision was automatically updated to reflect the committed changes.