[Bug c++/113658] New: GCC 14 has incomplete impl for declared feature "cxx_constexpr_string_builtins"

[Bug c++/113658] New: GCC 14 has incomplete impl for declared feature "cxx_constexpr_string_builtins"

[berrange at redhat dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113658

            Bug ID: 113658
           Summary: GCC 14 has incomplete impl for declared feature
                    "cxx_constexpr_string_builtins"
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: berrange at redhat dot com
  Target Milestone: ---

In GCC 14 there was support for the Clang __has_feature() language extension
added by

  commit 06280a906cb3dc80cf5e07cf3335b758848d488d
  Author: Alex Coplan <alex.coplan@arm.com>
  Date:   Fri Mar 17 16:30:51 2023 +0000

    c-family: Implement __has_feature and __has_extension [PR60512]

    This patch implements clang's __has_feature and __has_extension in GCC.
    Currently the patch aims to implement all documented features (and some
    undocumented ones) following the documentation at
    https://clang.llvm.org/docs/LanguageExtensions.html with the exception
    of the legacy features for C++ type traits. 

One of the features declared as implemented is "cxx_constexpr_string_builtins"
which was documented by CLang as indicating support for built-ins for memchr,
memcmp, strchr, strcmp, strlen, strncmp,  wcschr, wcscmp, wcslen, wcsncmp,
wmemchr, wmemcmp, and one extra special case for memchr.

Except GCC 14 does not provide built-ins for all those functions, so GCC is
claiming support for a feature it cannot fully support. 

As a result code that was written against this CLang feature extension now gets
enabled with GCC 14 and then fails to build.

$ cat >> demo.cpp <<EOF
#include <string.h>

#ifndef __has_feature
#warning "no __has_feature"
#define __has_feature(a) 0
#endif

char *foo(const char *s, int c, size_t n) {
#if __has_feature(cxx_constexpr_string_builtins)
    return __builtin_char_memchr(s, c, n);
#else
    #warning "no __builtin_char_memchr"
    return NULL;
#endif
}
EOF
$ g++ b.cpp
b.cpp: In function ‘char* foo(const char*, int, size_t)’:
b.cpp:10:12: error: ‘__builtin_char_memchr’ was not declared in this scope; did
you mean ‘__builtin_memchr’?
   10 |     return __builtin_char_memchr(s, c, n);
      |            ^~~~~~~~~~~~~~~~~~~~~
      |            __builtin_memchr


gcc version 14.0.1 20240125 (Red Hat 14.0.1-0) (GCC)

[Bug c++/113658] GCC 14 has incomplete impl for declared feature "cxx_constexpr_string_builtins"

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113658

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Hmm, Most other conditional uses of builtin use __has_builtins instead.
Interesting one projection just conditionalized it on
cxx_constexpr_string_builtins .

[Bug c++/113658] GCC 14 has incomplete impl for declared feature "cxx_constexpr_string_builtins"

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113658

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Newer libc++ does the "correct" thing even:
https://github.com/llvm/llvm-project/blob/430c1fd50d774dc30a9628bcf60ce243f74ff376/libcxx/include/__string/constexpr_c_functions.h#L121

[Bug c++/113658] GCC 14 has incomplete impl for declared feature "cxx_constexpr_string_builtins"

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113658

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |acoplan at gcc dot gnu.org,
                   |                            |jakub at gcc dot gnu.org

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Obviously using __has_builtin is much better than using the really badly
designed __has_feature/__has_extension.
That said, wcs{chr,cmp,len,ncmp} and wmem{chr,cmp} aren't builtins in gcc
either, so I guess we shouldn't announce this "feature".

[Bug c++/113658] GCC 14 has incomplete impl for declared feature "cxx_constexpr_string_builtins"

[berrange at redhat dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113658

--- Comment #4 from Daniel Berrange <berrange at redhat dot com> ---
(In reply to Jakub Jelinek from comment #3)
> Obviously using __has_builtin is much better than using the really badly
> designed __has_feature/__has_extension.
> That said, wcs{chr,cmp,len,ncmp} and wmem{chr,cmp} aren't builtins in gcc
> either, so I guess we shouldn't announce this "feature".

Yes, I'll change the code I found to use __has_builtin instead, so it'll work
with new & old GCC & CLang.

[Bug c++/113658] GCC 14 has incomplete impl for declared feature "cxx_constexpr_string_builtins"

[acoplan at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113658

Alex Coplan <acoplan at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2024-01-30
             Status|UNCONFIRMED                 |ASSIGNED
     Ever confirmed|0                           |1
           Assignee|unassigned at gcc dot gnu.org      |acoplan at gcc dot gnu.org

--- Comment #5 from Alex Coplan <acoplan at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #3)
> Obviously using __has_builtin is much better than using the really badly
> designed __has_feature/__has_extension.
> That said, wcs{chr,cmp,len,ncmp} and wmem{chr,cmp} aren't builtins in gcc
> either, so I guess we shouldn't announce this "feature".

Mine, then.  I can prepare a patch to stop advertising the feature.

[Bug c++/113658] GCC 14 has incomplete impl for declared feature "cxx_constexpr_string_builtins"

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113658

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
It doesn't help that __has_feature/__has_extension is very badly documented,
obviously the best meaning for a feature check would be that the selected
builtins are usable in constexpr expressions if they are implemented, but it
has been added before __has_builtin has been introduced/standardized.  And for
__builtin_wcs* etc. implementation we have the long standing problem what APIs
to use to access the wchar_ts, I think that is the reason why we don't
implement format attribute for *wprintf etc.

[Bug c++/113658] GCC 14 has incomplete impl for declared feature "cxx_constexpr_string_builtins"

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113658

--- Comment #7 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Alex Coplan <acoplan@gcc.gnu.org>:

https://gcc.gnu.org/g:0d810b7d133c72b7e62b294ffaaf131560ce2391

commit r14-8951-g0d810b7d133c72b7e62b294ffaaf131560ce2391
Author: Alex Coplan <alex.coplan@arm.com>
Date:   Wed Jan 31 14:50:55 2024 +0000

    c++: Don't advertise cxx_constexpr_string_builtins [PR113658]

    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/has-feature2.C: New test.

[Bug c++/113658] GCC 14 has incomplete impl for declared feature "cxx_constexpr_string_builtins"

[acoplan at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113658

Alex Coplan <acoplan at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #8 from Alex Coplan <acoplan at gcc dot gnu.org> ---
Fixed, thanks for the report.