Jonathan Wakely <jwakely@redhat.com> writes:

> [..snip..]
> Thanks for adding the comments like "// C++ < 20".
>
> I think in <any> the comment on the #endif can be just __cpp_lib_any
> rather than defined(__cpp_lib_any). Similarly for
> __cpp_lib_atomic_float in <atomic>. Oh, and __cpp_lib_atomic_ref. And
> in <barrier>, and several others. I think I'd like those to be
> consistent, and usually we just name the macro in the #endif comment,
> sometimes abbreviated for clarity, without the explicit defined(...).

ACK.  Fixed all of those.

> For this error in <atomic> please add <> around "version" and remove
> the question mark:
> +# error "libstdc++ bug: no lock-free atomics but they were emitted in version?"
>
> Similarly, please remove the question marks from the two #errors in
> <type_traits>:
> +#  error "libstdc++ bug: is_corresponding_member and
> is_layout_compatible are provided but their FTM is not set?"
> +#  error "libstdc++ bug: is_pointer_interconvertible available but FTM unset?"
>
> In <string_view> you have:
> +# error "libstdc++ bug: string_contents not defined when it should be"
> That should be contains, not contents.
>
> OK for trunk with the #error changes. The #endif cleanup can be
> fixed in a follow-up.
> 
> It seems like there's some inconsistency (probably some preexisting)
> about whether you use:
> #if __cpp_lib_xxx
> or
> #ifdef __cpp_lib_xxx
> That can be tidied up later.
>
> Currently we define many of the macros in the "bits" headers, e.g. in
> bits/stl_iterator.h
>
> +#define __glibcxx_want_constexpr_iterator
> +#define __glibcxx_want_array_constexpr
> +#define __glibcxx_want_make_reverse_iterator
> +#define __glibcxx_want_move_iterator_concept
> +#include <bits/version.h>
>
> We should consider only defining those in <iterator> itself. So that
> when other parts of the lib include bits/stl_iterator.h they don't
> define the macros. That would mean that
> __cpp_lib_make_reverse_iterator is not defined by <vector> and
> <string>, for example. Even though they do actually provide the
> features, the macro would only be defined by <version> and <iterator>.
> This might encourage users to include the right headers, instead of
> relying on transitive includes.

> If we do that, our own internal checks for features would all need to use:
> #if __glibcxx_make_reverse_iterator
> because they wouldn't have the __cpp_lib_xxx macro, because they only
> include the internal bits header not <iterator>.
>
> That's for another day though.

Yes, that sounds quite reasonable.  I like the idea that headers should
export narrower FTMs.

Pushed.  Thanks :-)
-- 
Arsen Arsenović