This is an archive of the discontinued LLVM Phabricator instance.

[libc++] <type_traits>: Avoid instantiating a pointer type.
ClosedPublic

Authored by bsdjhb on Oct 7 2022, 11:02 AM.

Details

Summary

GCC expands the pointer type in this conditional expression even for
template types _Up that are not arrays. This raises an error when
std::decay<> is used with reference types (as is done in LLVM's
sources). Using add_pointer<> causes GCC to only instantiate a
pointer type for array types.

Diff Detail

Event Timeline

bsdjhb created this revision.Oct 7 2022, 11:02 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 7 2022, 11:02 AM
bsdjhb requested review of this revision.Oct 7 2022, 11:02 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 7 2022, 11:02 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript

I ran into this error in a perhaps unusual environment which is using GCC 12.0 to compile LLVM 14 (included in FreeBSD) using libc++ as the C++ runtime (rather than using libstdc++). The exact error from GCC is included in the review I opened for FreeBSD at https://reviews.freebsd.org/D36898. I forward ported the patch from LLVM 14 but do not have an easy way to test the setup I used to reproduce the GCC error on LLVM 14. I did not use __add_pointer_t because line 49 uses add_pointer<>, but if __add_pointer_t is preferred I'm happy to switch it to that instead.

philnik requested changes to this revision.Oct 7 2022, 11:11 AM
philnik added a subscriber: philnik.

Please add a regression test in libcxx/test/std/meta/type.traits/meta.trans/meta.trans.other.

libcxx/include/__type_traits/decay.h
45
This revision now requires changes to proceed.Oct 7 2022, 11:11 AM
emaste added a subscriber: emaste.Oct 7 2022, 11:19 AM
bsdjhb updated this revision to Diff 476566.Nov 18 2022, 12:25 PM
bsdjhb marked an inline comment as done.
  • Add tests.
ldionne accepted this revision as: ldionne.Nov 18 2022, 2:19 PM
ldionne added a subscriber: ldionne.

Thanks for fixing! This LGTM but I'll let @philnik give the libc++ approval since he had requested changes.

Also, please rebase onto main and reupload the diff. It should fix the failing CI.

philnik accepted this revision.Nov 19 2022, 5:58 AM

LGTM with green CI.

libcxx/include/__type_traits/decay.h
45
This revision is now accepted and ready to land.Nov 19 2022, 5:58 AM
bsdjhb updated this revision to Diff 476693.Nov 19 2022, 11:07 AM

Adjust formatting.

bsdjhb marked an inline comment as done.Nov 19 2022, 11:07 AM