On 02/02/2024 09:34, Marek Polacek wrote:
> On Fri, Feb 02, 2024 at 10:27:23AM +0000, Alex Coplan wrote:
> > Bootstrapped/regtested on x86_64-apple-darwin, OK for trunk?
> >
> > Thanks,
> > Alex
> >
> > -- >8 --
> >
> > When __has_feature was introduced for GCC 14, I included the feature
> > cxx_constexpr_string_builtins, since of the relevant string builtins
> > that GCC implements, it seems to support constexpr evaluation of those
> > builtins.
> >
> > However, as the PR shows, GCC doesn't implement the full list of
> > builtins in the clang documentation. After enumerating the builtins,
> > the clang docs [1] say:
> >
> > > Support for constant expression evaluation for the above builtins can
> > > be detected with __has_feature(cxx_constexpr_string_builtins).
> >
> > and a strict reading of this would suggest we can't really support
> > constexpr evaluation of a builtin if we don't implement the builtin in
> > the first place.
> >
> > So the conservatively correct thing to do seems to be to stop
> > advertising the feature altogether to avoid failing to build code which
> > assumes the presence of this feature implies the presence of all the
> > builtins listed in the clang documentation.
> >
> > [1] : https://clang.llvm.org/docs/LanguageExtensions.html#string-builtins
> >
> > gcc/cp/ChangeLog:
> >
> > PR c++/113658
> > * cp-objcp-common.cc (cp_feature_table): Remove entry for
> > cxx_constexpr_string_builtins.
> >
> > gcc/testsuite/ChangeLog:
> >
> > PR c++/113658
> > * g++.dg/ext/pr113658.C: New test.
>
> > diff --git a/gcc/cp/cp-objcp-common.cc b/gcc/cp/cp-objcp-common.cc
> > index f06edf04ef0..85dde0459fa 100644
> > --- a/gcc/cp/cp-objcp-common.cc
> > +++ b/gcc/cp/cp-objcp-common.cc
> > @@ -110,7 +110,6 @@ static constexpr cp_feature_info cp_feature_table[] =
> > { "cxx_alignof", cxx11 },
> > { "cxx_attributes", cxx11 },
> > { "cxx_constexpr", cxx11 },
> > - { "cxx_constexpr_string_builtins", cxx11 },
> > { "cxx_decltype", cxx11 },
> > { "cxx_decltype_incomplete_return_types", cxx11 },
> > { "cxx_default_function_template_args", cxx11 },
> > diff --git a/gcc/testsuite/g++.dg/ext/pr113658.C
> > b/gcc/testsuite/g++.dg/ext/pr113658.C
> > new file mode 100644
> > index 00000000000..f4a34888f28
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/ext/pr113658.C
>
> Might be better to name this has-feature2.C
>
> > @@ -0,0 +1,13 @@
>
> Please include
> // PR c++/113658
Can do.
>
> > +// { dg-do compile }
> > +// { dg-options "" }
>
> Why dg-options ""? It doesn't seem to have any purpose here.
That is to disable -pedantic-errors which IIRC is added by default in
the testsuite options.
With -pedantic-errors we would have __has_extension behaving like
__has_feature, and I wanted to check in the test that this doesn't get
reported as a feature or extension.
Incidentally it also means we don't have to provide a dummy declaration,
with -pedantic-errors we would get a warning about an empty TU which
would make the test fail.
Thanks,
Alex
>
> > +// PR113658: we shouldn't declare support for
> > cxx_constexpr_string_builtins as
> > +// GCC is missing some of the builtins that clang implements.
> > +
> > +#if __has_feature (cxx_constexpr_string_builtins)
> > +#error
> > +#endif
> > +
> > +#if __has_extension (cxx_constexpr_string_builtins)
> > +#error
> > +#endif
>
>
> Marek
>
