Page MenuHomePhabricator

[libc++][P0784] Marked the default allocator constexpr.
Needs RevisionPublic

Authored by mpark on Oct 17 2019, 12:45 PM.

Details

Event Timeline

mpark created this revision.Oct 17 2019, 12:45 PM
mpark updated this revision to Diff 225502.Oct 17 2019, 12:50 PM

Fixed allocate.size.fail.cpp test.

ldionne requested changes to this revision.Nov 4 2019, 6:28 AM
ldionne added inline comments.
libcxx/include/memory
1873–1874

Isn't it fine to throw inside constexpr if we can't allocate?

1999–2000

Ditto

This revision now requires changes to proceed.Nov 4 2019, 6:28 AM

Woops, it also looks like you are missing the feature test macros:

Add to Table 17: Feature-test macros with the appropriate value:
__cpp_constexpr_dynamic_alloc
Add to Table 36: Standard library feature-test macros with the appropriate value:
__cpp_lib_constexpr_dynamic_alloc <memory>

mpark updated this revision to Diff 228020.Nov 6 2019, 1:21 AM

Removed unnecessary calls to is_constant_evaluated.

mpark marked 2 inline comments as done.Nov 6 2019, 1:22 AM
mpark added a comment.Nov 6 2019, 1:29 AM

I'm planning to add __cpp_lib_constexpr_dynamic_alloc in a later patch, after the rest of P0784.

mpark updated this revision to Diff 228271.Nov 7 2019, 9:56 AM

Updated to check for __cpp_constexpr_dynamic_alloc.

mpark updated this revision to Diff 228452.Nov 8 2019, 7:20 AM

Rebased

mpark updated this revision to Diff 228857.Tue, Nov 12, 4:16 AM

Rebased on top of D70117.

mpark updated this revision to Diff 228976.EditedTue, Nov 12, 3:35 PM

Marked max_size, construct and destroy constexpr as well.

mpark retitled this revision from [libc++][P0784] Marked allocator constexpr. to [libc++][P0784] Marked default allocator constexpr..Tue, Nov 12, 3:35 PM
mpark retitled this revision from [libc++][P0784] Marked default allocator constexpr. to [libc++][P0784] Marked the default allocator constexpr..
mpark updated this revision to Diff 229008.Tue, Nov 12, 10:30 PM

Rebased.

ldionne requested changes to this revision.Wed, Nov 13, 8:30 AM
ldionne added inline comments.
libcxx/include/memory
1957

This is because __cpp_constexpr_dynamic_alloc also controls whether you can call a destructor explicitly inside constexpr, right? It looks kind of funny, because we don't allocate here.

EDIT: I guess the same hold for construct, where we use placement-new.

libcxx/include/new
261

I assume you had to move the __do_call functions here because making them constexpr somehow required their definition to appear above the definition of the various __do_allocate functions below?

libcxx/test/std/utilities/memory/default.allocator/allocator.members/allocate.fail.cpp
29

I don't understand that test. Shouldn't allocator be made constexpr by this patch? Why do you check that there's an error saying it's not an ICE?

libcxx/test/std/utilities/memory/default.allocator/allocator.members/allocate.pass.cpp
83

You have a mismatch in the indentation of the return and body.

libcxx/test/std/utilities/memory/default.allocator/allocator.members/allocate.size.fail.cpp
38

I would expect that you get one error for each statement up in test() above. Isn't it what's happening? I would expect something like expected-error 4 {{...}}.

39

Why not just static_assert here?

This revision now requires changes to proceed.Wed, Nov 13, 8:30 AM