Re: [PATCH 1/2] libstdc++: Implement more maintainable <version> header

[Jonathan Wakely <jwakely@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 4591 bytes --]

On Sat, 29 Apr 2023 at 11:25, Arsen Arsenović via Libstdc++ <
libstdc++@gcc.gnu.org> wrote:

> This commit replaces the ad-hoc logic in <version> with an AutoGen
> database that (mostly) declaratively generates a version.h bit which
> combines all of the FTM logic across all headers together.
>
> This generated header defines macros of the form __glibcxx_foo,
> equivalent to their __cpp_lib_foo variants, according to rules specified
> in version.def and, optionally, if __glibcxx_want_foo or
> __glibcxx_want_all are defined, also defines __cpp_lib_foo forms with
> the same definition.
>
> libstdc++-v3/ChangeLog:
>
>         * include/Makefile.am (bits_freestanding): Add version.h.
>         (allcreated): Add version.h.
>         (${bits_srcdir}/version.h): New rule.  Regenerates
>         version.h out of version.{def,tpl}.
>         * include/Makefile.in: Regenerate.
>         * include/bits/version.def: New file.  Declares a list of
>         all feature test macros, their values and their preconditions.
>         * include/bits/version.tpl: New file.  Turns version.def
>         into a sequence of #if blocks.
>         * include/bits/version.h: New file.  Generated from
>         version.def.
>         * include/std/version: Replace with a __glibcxx_want_all define
>         and bits/version.h include.
> ---
>  libstdc++-v3/include/Makefile.am      |   10 +-
>  libstdc++-v3/include/Makefile.in      |   10 +-
>  libstdc++-v3/include/bits/version.def | 1591 ++++++++++++++++++++
>  libstdc++-v3/include/bits/version.h   | 1937 +++++++++++++++++++++++++
>  libstdc++-v3/include/bits/version.tpl |  209 +++
>  libstdc++-v3/include/std/version      |  350 +----
>  6 files changed, 3758 insertions(+), 349 deletions(-)
>  create mode 100644 libstdc++-v3/include/bits/version.def
>  create mode 100644 libstdc++-v3/include/bits/version.h
>  create mode 100644 libstdc++-v3/include/bits/version.tpl
>
> diff --git a/libstdc++-v3/include/Makefile.am
> b/libstdc++-v3/include/Makefile.am
> index a880e8ee227..a07b4c18585 100644
> --- a/libstdc++-v3/include/Makefile.am
> +++ b/libstdc++-v3/include/Makefile.am
> @@ -154,6 +154,7 @@ bits_freestanding = \
>         ${bits_srcdir}/stl_raw_storage_iter.h \
>         ${bits_srcdir}/stl_relops.h \
>         ${bits_srcdir}/stl_uninitialized.h \
> +       ${bits_srcdir}/version.h \
>         ${bits_srcdir}/string_view.tcc \
>         ${bits_srcdir}/uniform_int_dist.h \
>         ${bits_srcdir}/unique_ptr.h \
> @@ -1113,7 +1114,8 @@ allcreated = \
>         ${host_builddir}/c++config.h \
>         ${host_builddir}/largefile-config.h \
>         ${thread_host_headers} \
> -       ${pch_build}
> +       ${pch_build} \
> +       ${bits_srcdir}/version.h
>
>  # Here are the rules for building the headers
>  all-local: ${allstamped} ${allcreated}
> @@ -1463,6 +1465,12 @@ ${pch3_output}: ${pch3_source} ${pch2_output}
>         -mkdir -p ${pch3_output_builddir}
>         $(CXX) $(PCHFLAGS) $(AM_CPPFLAGS) -O2 -g ${pch3_source} -o $@
>
> +# AutoGen <bits/version.h>.
> +${bits_srcdir}/version.h: ${bits_srcdir}/version.def \
> +                               ${bits_srcdir}/version.tpl
> +       cd $(@D) && \
> +       autogen version.def
>

It looks like this will regenerate the bits/version.h file if it's older
than the definitions or the autogen template, right?

Generally we don't want to touch anything in the source tree as part of a
normal build. It's OK to do that when configured with
--enable-maintainer-mode (which nobody working on libstdc++ actually uses,
because it causes problems IME) or via a dedicated target which is not
built by default (e.g. doc/Makefile.am has the doc-html-docbook-regenerate
target, which is isn't a prereq of any other targets so it's only run if
you explicitly request it).

The problem with modifying the source tree as part of a normal build is
that it might be on read-only media, and so the build will fail if this
target can't be updated. We would also want to add the version.h header to
the contrib/gcc_update script that updates the timestamps of generated
files, so that they are always newer than their prereqs.

Maybe the best option here is to assume that version.h is always up to
date, and add a custom target to regen it manually, which we can run after
editing the .def or .tpl files. What do you think?

My only other concern with this patch is that I don't speak lisp so the
Guile code in version.tpl is opaque and unmaintainable for me. That is
fixable though.