This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Add missing includes for __type_traits details
AbandonedPublic

Authored by ldionne on Dec 1 2022, 7:05 PM.

Details

Reviewers
Mordante
Group Reviewers
Restricted Project
Summary

By convention, <type_traits> should include everything under type_traits.
This was found because <cmath> was missing the declaration for
promote.

Diff Detail

Event Timeline

ldionne created this revision.Dec 1 2022, 7:05 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 1 2022, 7:05 PM
ldionne requested review of this revision.Dec 1 2022, 7:05 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 1 2022, 7:05 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
jloser added a subscriber: jloser.Dec 3 2022, 4:41 PM

Looks reasonable. Do you see any value in adding a test that verifies each top-level include actually includes all of the granularized headers? E.g. that <type_traits> does indeed include all of the header files in <__type_traits>? I idly wonder if we have similar problems for any of the other top-level includes that have been granularized such as the ones reported by the following:

find libcxx/include -type d -maxdepth 1 -name '__*'
libcxx/include/__iterator
libcxx/include/__memory
libcxx/include/__support
libcxx/include/__chrono
libcxx/include/__numeric
libcxx/include/__ranges
libcxx/include/__type_traits
libcxx/include/__variant
libcxx/include/__compare
libcxx/include/__concepts
libcxx/include/__filesystem
libcxx/include/__utility
libcxx/include/__charconv
libcxx/include/__tuple
libcxx/include/__format
libcxx/include/__algorithm
libcxx/include/__ios
libcxx/include/__functional
libcxx/include/__coroutine
libcxx/include/__thread
libcxx/include/__fwd
libcxx/include/__memory_resource
libcxx/include/__debug_utils
libcxx/include/__bit
libcxx/include/__string
libcxx/include/__random

We can probably safely not worry about __debug_utils since it's private.

Mordante accepted this revision.Dec 5 2022, 8:28 AM
Mordante added a subscriber: Mordante.

Looks reasonable. Do you see any value in adding a test that verifies each top-level include actually includes all of the granularized headers? E.g. that <type_traits> does indeed include all of the header files in <__type_traits>? I idly wonder if we have similar problems for any of the other top-level includes that have been granularized such as the ones reported by the following:

<snip>

I think that would be useful, I love to have tooling that catches errors. Looking at the list this seems like an easy error to make. I recognize one file...

Thanks for the cleanup, LGTM!

This revision is now accepted and ready to land.Dec 5 2022, 8:28 AM
ldionne abandoned this revision.Jan 26 2023, 11:45 AM

I am actually going to drop this change -- as we've discussed in a couple of other places already, while this is what we wanted to do as a policy, this doesn't work in some cases, so I won't make a change to go in that direction.