Page MenuHomePhabricator

Fix SFINAE in llvm::bit_cast.
ClosedPublic

Authored by jlebar on Feb 10 2020, 8:24 PM.

Details

Summary

As far as I can tell, the SFINAE was broken; there is no such thing as
std::is_trivially_constructible<T>::type.

Diff Detail

Event Timeline

jlebar created this revision.Feb 10 2020, 8:24 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 10 2020, 8:24 PM
jlebar updated this revision to Diff 243729.Feb 10 2020, 8:31 PM

Attempt to remove files that should have been ignored.

jlebar updated this revision to Diff 243730.Feb 10 2020, 8:35 PM

Undo overzealous re-clang-formatting.

"How did this ever work?" Since is_trivially_constructible<T>::type doesn't exist, wouldn't this always be a substitution failure?

Harbormaster completed remote builds in B46171: Diff 243730.
jlebar added a reviewer: jfb.Feb 10 2020, 8:40 PM

Since is_trivially_constructible<T>::type doesn't exist, wouldn't this always be a substitution failure?

Ah, ::type exists, it is either true_type or false_type. I still don't see how this ever worked, though, because then there's still no SFINAE...

Lint: Pre-merge checks

I chose not to reformat this patch because it's a tricky diff that I wanted to keep small and I'm going to reformat it in a later patch that changes all enable_if to enable_if_t anyway. But I can reformat it if reviewers would like.

Hah, nice catch, this sounds like something that might make a nice clang-tidy check?

MaskRay accepted this revision.Feb 11 2020, 11:12 PM
This revision is now accepted and ready to land.Feb 11 2020, 11:12 PM

Thank you for the reviews!

this sounds like something that might make a nice clang-tidy check?

Good idea! Is there a place where people keep track of open clang-tidy check requests?

(Another way this could have been caught is if LLVM supported "negative compilation tests", i.e. unit tests that check that certain code doesn't compile.)

This revision was automatically updated to reflect the committed changes.