This is an archive of the discontinued LLVM Phabricator instance.

[libc++][NFC] Rename _EnableIf to __enable_if_t for consistency
ClosedPublic

Authored by ldionne on Sep 8 2021, 6:16 AM.

Details

Summary

In other places in the code, we use lowercase spelling for things that
are not available in prior standards.

Diff Detail

Event Timeline

ldionne requested review of this revision.Sep 8 2021, 6:16 AM
ldionne created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptSep 8 2021, 6:16 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript

Seems reasonable if this is the direction we end up going. I think you should hold off until we figure out whether this needs to be _LIBCPP_ENABLEIF. (I think that depends only on whether we care about good diagnostics on Clang 13.)

Per my latest comment/update on D109411: I buy this now. We're switching away from the SCARY metabase technique (which irreparably interferes with the good diagnostics) at the same time that we're switching away from the _EnableIf name. Simultaneously, on the Clang side, we're special-casing the new name, __enable_if_t, so that it will start giving the good diagnostics.

(I think it would still be nice to decide whether we are going to use __enable_if_t throughout for consistency, or permit _LIBCPP_STD_VER > 11 codepaths to use enable_if_t if they want. I predict the latter will win out anyway, though.)

LGTM, ship it! Assuming you agree with D109411 at this point too.

ldionne accepted this revision as: Restricted Project.Sep 8 2021, 12:20 PM

Per my latest comment/update on D109411: I buy this now. We're switching away from the SCARY metabase technique (which irreparably interferes with the good diagnostics) at the same time that we're switching away from the _EnableIf name. Simultaneously, on the Clang side, we're special-casing the new name, __enable_if_t, so that it will start giving the good diagnostics.

(I think it would still be nice to decide whether we are going to use __enable_if_t throughout for consistency, or permit _LIBCPP_STD_VER > 11 codepaths to use enable_if_t if they want. I predict the latter will win out anyway, though.)

My opinion is that we should use enable_if_t where we can, and __enable_if_t when we can't because the std version doesn't permit us to do so. It's what we do for all other utilities, e.g. std::__to_address & friends, so that approach seems consistent to me.

LGTM, ship it! Assuming you agree with D109411 at this point too.

Yes, I agree with D109411 (just approved it).

This revision is now accepted and ready to land.Sep 8 2021, 12:20 PM