On Fri, Feb 02, 2024 at 03:45:48PM +0000, Alex Coplan wrote: > 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.
Right. > 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. Oh I see. A comment to that effect might be helpful. Otherwise LGTM, thanks. Marek
