Page MenuHomePhabricator

[libc++] Remove _LIBCPP_CONSTEVAL
ClosedPublic

Authored by philnik on Feb 7 2023, 4:42 AM.

Details

Reviewers
ldionne
Mordante
Group Reviewers
Restricted Project
Commits
rGaaef3b82f4f0: [libc++] Remove _LIBCPP_CONSTEVAL
Summary

All supported compilers support consteval, so there is no more need for the macro.

Diff Detail

Event Timeline

philnik created this revision.Feb 7 2023, 4:42 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 7 2023, 4:42 AM
philnik requested review of this revision.Feb 7 2023, 4:42 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 7 2023, 4:42 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript

Can you add the motivation for this change in the commit message?

ldionne accepted this revision.Feb 7 2023, 8:15 AM

I think it must be because all supported compilers now support consteval in C++20 mode. Please add a motivation to the commit, but otherwise LGTM.

This revision is now accepted and ready to land.Feb 7 2023, 8:15 AM
philnik edited the summary of this revision. (Show Details)Feb 11 2023, 2:55 AM
Mordante requested changes to this revision.Feb 11 2023, 5:35 AM

I think it must be because all supported compilers now support consteval in C++20 mode. Please add a motivation to the commit, but otherwise LGTM.

I assume the same, but would be good to make sure.

It seems the rebasing went wrong, therefore requesting changes.

This revision now requires changes to proceed.Feb 11 2023, 5:35 AM

I think it must be because all supported compilers now support consteval in C++20 mode. Please add a motivation to the commit, but otherwise LGTM.

I assume the same, but would be good to make sure.

It seems the rebasing went wrong, therefore requesting changes.

What are you talking about? The diff looks fine to me and the CI failures look unrelated.

Mordante accepted this revision.Feb 11 2023, 6:36 AM

I think it must be because all supported compilers now support consteval in C++20 mode. Please add a motivation to the commit, but otherwise LGTM.

I assume the same, but would be good to make sure.

It seems the rebasing went wrong, therefore requesting changes.

What are you talking about? The diff looks fine to me and the CI failures look unrelated.

Weird, now the patch looks good. When I checked before there was no line with consteval in the at all.
Maybe a Phab hickup :-/

Sorry for the confusion, LGTM!

This revision is now accepted and ready to land.Feb 11 2023, 6:36 AM
This revision was landed with ongoing or failed builds.Feb 11 2023, 9:34 AM
This revision was automatically updated to reflect the committed changes.
hans added a subscriber: hans.Feb 13 2023, 7:29 AM

We're seeing mysterious memory leaks after this. I filed https://github.com/llvm/llvm-project/issues/60709 with a reproducer. Reverting until it can be fixed.