public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug c++/106784] New: Add __is_convertible built-in
@ 2022-08-31 10:41 redi at gcc dot gnu.org
  2022-08-31 13:38 ` [Bug c++/106784] " mpolacek at gcc dot gnu.org
                   ` (9 more replies)
  0 siblings, 10 replies; 11+ messages in thread
From: redi at gcc dot gnu.org @ 2022-08-31 10:41 UTC (permalink / raw)
  To: gcc-bugs

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

            Bug ID: 106784
           Summary: Add __is_convertible built-in
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: enhancement
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: redi at gcc dot gnu.org
  Target Milestone: ---

This is supported by Clang already, and would be very beneficial for libstdc++.
The std::is_convertible (and is_nothrow_convertible) type trait is used widely
in the std::lib, but is currently implemented in pure C++ as a class template.
A built-in would improve compile times.

We already have __is_constructible and __is_assignable, and the nothrow forms
of those.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Bug c++/106784] Add __is_convertible built-in
  2022-08-31 10:41 [Bug c++/106784] New: Add __is_convertible built-in redi at gcc dot gnu.org
@ 2022-08-31 13:38 ` mpolacek at gcc dot gnu.org
  2022-08-31 14:05 ` mpolacek at gcc dot gnu.org
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: mpolacek at gcc dot gnu.org @ 2022-08-31 13:38 UTC (permalink / raw)
  To: gcc-bugs

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

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
                 CC|                            |mpolacek at gcc dot gnu.org
   Last reconfirmed|                            |2022-08-31

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Bug c++/106784] Add __is_convertible built-in
  2022-08-31 10:41 [Bug c++/106784] New: Add __is_convertible built-in redi at gcc dot gnu.org
  2022-08-31 13:38 ` [Bug c++/106784] " mpolacek at gcc dot gnu.org
@ 2022-08-31 14:05 ` mpolacek at gcc dot gnu.org
  2022-09-01 16:26 ` redi at gcc dot gnu.org
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: mpolacek at gcc dot gnu.org @ 2022-08-31 14:05 UTC (permalink / raw)
  To: gcc-bugs

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

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |mpolacek at gcc dot gnu.org
             Status|NEW                         |ASSIGNED

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Bug c++/106784] Add __is_convertible built-in
  2022-08-31 10:41 [Bug c++/106784] New: Add __is_convertible built-in redi at gcc dot gnu.org
  2022-08-31 13:38 ` [Bug c++/106784] " mpolacek at gcc dot gnu.org
  2022-08-31 14:05 ` mpolacek at gcc dot gnu.org
@ 2022-09-01 16:26 ` redi at gcc dot gnu.org
  2022-09-20 21:24 ` mpolacek at gcc dot gnu.org
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: redi at gcc dot gnu.org @ 2022-09-01 16:26 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
To be clear, I'd like __is_nothrow_convertible too. But if I only get
__is_convertible first, that would still be great.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Bug c++/106784] Add __is_convertible built-in
  2022-08-31 10:41 [Bug c++/106784] New: Add __is_convertible built-in redi at gcc dot gnu.org
                   ` (2 preceding siblings ...)
  2022-09-01 16:26 ` redi at gcc dot gnu.org
@ 2022-09-20 21:24 ` mpolacek at gcc dot gnu.org
  2022-09-21  0:02 ` redi at gcc dot gnu.org
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: mpolacek at gcc dot gnu.org @ 2022-09-20 21:24 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #2 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
FWIW, I notice that include/std/type_traits implements struct
__is_nothrow_convertible so I think its name has to change, otherwise it would
clash with this new built-in.

Though, it seems that clang doesn't have the __is_nothrow_convertible built-in,
so maybe we want just __is_convertible.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Bug c++/106784] Add __is_convertible built-in
  2022-08-31 10:41 [Bug c++/106784] New: Add __is_convertible built-in redi at gcc dot gnu.org
                   ` (3 preceding siblings ...)
  2022-09-20 21:24 ` mpolacek at gcc dot gnu.org
@ 2022-09-21  0:02 ` redi at gcc dot gnu.org
  2022-09-23 16:17 ` cvs-commit at gcc dot gnu.org
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: redi at gcc dot gnu.org @ 2022-09-21  0:02 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
We can certainly change the struct name.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Bug c++/106784] Add __is_convertible built-in
  2022-08-31 10:41 [Bug c++/106784] New: Add __is_convertible built-in redi at gcc dot gnu.org
                   ` (4 preceding siblings ...)
  2022-09-21  0:02 ` redi at gcc dot gnu.org
@ 2022-09-23 16:17 ` cvs-commit at gcc dot gnu.org
  2022-09-23 16:18 ` mpolacek at gcc dot gnu.org
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-09-23 16:17 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The trunk branch has been updated by Marek Polacek <mpolacek@gcc.gnu.org>:

https://gcc.gnu.org/g:8a7bcf95a82c3dd68bd4bcfbd8432eb970575bc2

commit r13-2822-g8a7bcf95a82c3dd68bd4bcfbd8432eb970575bc2
Author: Marek Polacek <polacek@redhat.com>
Date:   Tue Sep 20 15:48:52 2022 -0400

    c++: Implement __is_{nothrow_,}convertible [PR106784]

    To improve compile times, the C++ library could use compiler built-ins
    rather than implementing std::is_convertible (and _nothrow) as class
    templates.  This patch adds the built-ins.  We already have
    __is_constructible and __is_assignable, and the nothrow forms of those.

    Microsoft (and clang, for compatibility) also provide an alias called
    __is_convertible_to.  I did not add it, but it would be trivial to do
    so.

    I noticed that our __is_assignable doesn't implement the "Access checks
    are performed as if from a context unrelated to either type" requirement,
    therefore std::is_assignable / __is_assignable give two different results
    here:

      class S {
        operator int();
        friend void g(); // #1
      };

      void
      g ()
      {
        // #1 doesn't matter
        static_assert(std::is_assignable<int&, S>::value, "");
        static_assert(__is_assignable(int&, S), "");
      }

    This is not a problem if __is_assignable is not meant to be used by
    the users.

    This patch doesn't make libstdc++ use the new built-ins, but I had to
    rename a class otherwise its name would clash with the new built-in.

            PR c++/106784

    gcc/c-family/ChangeLog:

            * c-common.cc (c_common_reswords): Add __is_convertible and
            __is_nothrow_convertible.
            * c-common.h (enum rid): Add RID_IS_CONVERTIBLE and
            RID_IS_NOTHROW_CONVERTIBLE.

    gcc/cp/ChangeLog:

            * constraint.cc (diagnose_trait_expr): Handle CPTK_IS_CONVERTIBLE
            and CPTK_IS_NOTHROW_CONVERTIBLE.
            * cp-objcp-common.cc (names_builtin_p): Handle RID_IS_CONVERTIBLE
            RID_IS_NOTHROW_CONVERTIBLE.
            * cp-tree.h (enum cp_trait_kind): Add CPTK_IS_CONVERTIBLE and
            CPTK_IS_NOTHROW_CONVERTIBLE.
            (is_convertible): Declare.
            (is_nothrow_convertible): Likewise.
            * cxx-pretty-print.cc (pp_cxx_trait_expression): Handle
            CPTK_IS_CONVERTIBLE and CPTK_IS_NOTHROW_CONVERTIBLE.
            * method.cc (is_convertible): New.
            (is_nothrow_convertible): Likewise.
            * parser.cc (cp_parser_primary_expression): Handle
RID_IS_CONVERTIBLE
            and RID_IS_NOTHROW_CONVERTIBLE.
            (cp_parser_trait_expr): Likewise.
            * semantics.cc (trait_expr_value): Handle CPTK_IS_CONVERTIBLE and
            CPTK_IS_NOTHROW_CONVERTIBLE.
            (finish_trait_expr): Likewise.

    libstdc++-v3/ChangeLog:

            * include/std/type_traits: Rename __is_nothrow_convertible to
            __is_nothrow_convertible_lib.
            * testsuite/20_util/is_nothrow_convertible/value_ext.cc: Likewise.

    gcc/testsuite/ChangeLog:

            * g++.dg/ext/has-builtin-1.C: Enhance to test __is_convertible and
            __is_nothrow_convertible.
            * g++.dg/ext/is_convertible1.C: New test.
            * g++.dg/ext/is_convertible2.C: New test.
            * g++.dg/ext/is_nothrow_convertible1.C: New test.
            * g++.dg/ext/is_nothrow_convertible2.C: New test.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Bug c++/106784] Add __is_convertible built-in
  2022-08-31 10:41 [Bug c++/106784] New: Add __is_convertible built-in redi at gcc dot gnu.org
                   ` (5 preceding siblings ...)
  2022-09-23 16:17 ` cvs-commit at gcc dot gnu.org
@ 2022-09-23 16:18 ` mpolacek at gcc dot gnu.org
  2022-09-24 14:10 ` redi at gcc dot gnu.org
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: mpolacek at gcc dot gnu.org @ 2022-09-23 16:18 UTC (permalink / raw)
  To: gcc-bugs

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

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

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

--- Comment #5 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Added in GCC 13.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Bug c++/106784] Add __is_convertible built-in
  2022-08-31 10:41 [Bug c++/106784] New: Add __is_convertible built-in redi at gcc dot gnu.org
                   ` (6 preceding siblings ...)
  2022-09-23 16:18 ` mpolacek at gcc dot gnu.org
@ 2022-09-24 14:10 ` redi at gcc dot gnu.org
  2022-09-26 16:42 ` cvs-commit at gcc dot gnu.org
  2023-04-29 17:44 ` pinskia at gcc dot gnu.org
  9 siblings, 0 replies; 11+ messages in thread
From: redi at gcc dot gnu.org @ 2022-09-24 14:10 UTC (permalink / raw)
  To: gcc-bugs

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jason at gcc dot gnu.org

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I'm seeing a regression in 21_strings/basic_string/types/1.cc when I start
using __is_convertible in the library.

Reduced to:

template<bool B>
struct bool_constant { static constexpr bool value = B; };
using true_type = bool_constant<true>;
using false_type = bool_constant<false>;

template<typename T> struct is_void : false_type { };
template<> struct is_void<void> : true_type { };

template<typename T> T&& declval();

template<bool> struct enable_if { };
template<> struct enable_if<true> { using type = void; };
template<bool B> using enable_if_t = typename enable_if<B>::type;

#ifdef USE_BUILTIN
  template<typename _From, typename _To>
    struct is_convertible
    : public bool_constant<__is_convertible(_From, _To)>
    { };

#else

  template<typename _From, typename _To,
           bool = is_void<_From>::value>
    struct __is_convertible_helper
    {
      typedef typename is_void<_To>::type type;
    };

#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wctor-dtor-privacy"
  template<typename _From, typename _To>
    class __is_convertible_helper<_From, _To, false>
    {
      template<typename _To1>
        static void __test_aux(_To1) noexcept;

      template<typename _From1, typename _To1,
               typename = decltype(__test_aux<_To1>(declval<_From1>()))>
        static true_type
        __test(int);

      template<typename, typename>
        static false_type
        __test(...);

    public:
      typedef decltype(__test<_From, _To>(0)) type;
    };
#pragma GCC diagnostic pop

  /// is_convertible
  template<typename _From, typename _To>
    struct is_convertible
    : public __is_convertible_helper<_From, _To>::type
    { };
#endif

template<class CharT>
struct char_traits
{
  static unsigned long length(const char* s) { eq(*s, *s); return 0; }

  static void eq(CharT l, CharT r) noexcept { l.f(r); }
};

template<class CharT>
struct basic_string_view
{
  using traits_type = char_traits<CharT>;

  constexpr basic_string_view() = default;
  constexpr basic_string_view(const basic_string_view&) = default;

  constexpr
  basic_string_view(const CharT* __str) noexcept
  : _M_len{traits_type::length(__str)}
  { }

  unsigned long _M_len = 0;
};

template<class CharT>
struct basic_string
{
  template<class T>
    enable_if_t<is_convertible<const T&, basic_string_view<CharT>>::value
                && !is_convertible<const T&, const char*>::value>
    replace(int, T) { }

  void replace(unsigned long, const char*) { }

  void replace(const char* s) { replace(1, s); }
};

int main()
{
  basic_string<char> s;
  s.replace("");
}

This is fine using the library trait, but compiled with -DUSE_BUILTIN it fails:

conv.cc: In instantiation of 'static void char_traits<CharT>::eq(CharT, CharT)
[with CharT = char]':
conv.cc:62:50:   required from 'static long unsigned int
char_traits<CharT>::length(const char*) [with CharT = char]'
conv.cc:77:31:   required from 'constexpr
basic_string_view<CharT>::basic_string_view(const CharT*) [with CharT = char]'
conv.cc:18:28:   required from 'struct is_convertible<const char* const&,
basic_string_view<char> >'
conv.cc:87:69:   required by substitution of 'template<class T>
enable_if_t<(is_convertible<const T&, basic_string_view<char> >::value && (!
is_convertible<const T&, const char*>::value))>
basic_string<char>::replace(int, T) [with T = const char*]'
conv.cc:93:40:   required from 'void basic_string<CharT>::replace(const char*)
[with CharT = char]'
conv.cc:99:12:   required from here
conv.cc:64:49: error: request for member 'f' in 'l', which is of non-class type
'char'
   64 |   static void eq(CharT l, CharT r) noexcept { l.f(r); }
      |                                               ~~^


The error is correct, char_traits<char>::eq is ill-formed. Strictly speaking,
the library testcase is invalid for basic_string, the N::X type needs to be
equality comparable.

But I don't think __is_convertible should be detecting that. To determine if
const char* is convertible to basic_string_view(const char*) should just need
to examine the signature, and not instantiate the body of the function. I think
it probably instantiates it eagerly because it's constexpr. IIRC Jason made
some changes to template substitution to prevent such eager instantiations of
constexpr functions. Maybe something like that is currently missing from
__is_convertible? Or maybe the trait is fine, and the code above is just
invalid.

I can easily fix the library test that fails, but I think it's worth checking
first whether something in __is_convertible needs a fix.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Bug c++/106784] Add __is_convertible built-in
  2022-08-31 10:41 [Bug c++/106784] New: Add __is_convertible built-in redi at gcc dot gnu.org
                   ` (7 preceding siblings ...)
  2022-09-24 14:10 ` redi at gcc dot gnu.org
@ 2022-09-26 16:42 ` cvs-commit at gcc dot gnu.org
  2023-04-29 17:44 ` pinskia at gcc dot gnu.org
  9 siblings, 0 replies; 11+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-09-26 16:42 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The trunk branch has been updated by Marek Polacek <mpolacek@gcc.gnu.org>:

https://gcc.gnu.org/g:be4b32b9ef69b86b662cb7511b48cd1048a55403

commit r13-2879-gbe4b32b9ef69b86b662cb7511b48cd1048a55403
Author: Marek Polacek <polacek@redhat.com>
Date:   Mon Sep 26 10:21:38 2022 -0400

    c++: Instantiate less when evaluating __is_convertible

    Jon reported that evaluating __is_convertible in a test led to
    instantiating something ill-formed and so we failed to compile the test.
    __is_convertible doesn't and shouldn't need to instantiate so much, so
    let's limit it with a cp_unevaluated guard.  Use a helper function to
    implement both built-ins.

            PR c++/106784

    gcc/cp/ChangeLog:

            * method.cc (is_convertible_helper): New.
            (is_convertible): Use it.
            (is_nothrow_convertible): Likewise.

    gcc/testsuite/ChangeLog:

            * g++.dg/ext/is_convertible3.C: New test.
            * g++.dg/ext/is_nothrow_convertible3.C: New test.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Bug c++/106784] Add __is_convertible built-in
  2022-08-31 10:41 [Bug c++/106784] New: Add __is_convertible built-in redi at gcc dot gnu.org
                   ` (8 preceding siblings ...)
  2022-09-26 16:42 ` cvs-commit at gcc dot gnu.org
@ 2023-04-29 17:44 ` pinskia at gcc dot gnu.org
  9 siblings, 0 replies; 11+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-04-29 17:44 UTC (permalink / raw)
  To: gcc-bugs

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |missed-optimization
   Target Milestone|---                         |13.0

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2023-04-29 17:44 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-31 10:41 [Bug c++/106784] New: Add __is_convertible built-in redi at gcc dot gnu.org
2022-08-31 13:38 ` [Bug c++/106784] " mpolacek at gcc dot gnu.org
2022-08-31 14:05 ` mpolacek at gcc dot gnu.org
2022-09-01 16:26 ` redi at gcc dot gnu.org
2022-09-20 21:24 ` mpolacek at gcc dot gnu.org
2022-09-21  0:02 ` redi at gcc dot gnu.org
2022-09-23 16:17 ` cvs-commit at gcc dot gnu.org
2022-09-23 16:18 ` mpolacek at gcc dot gnu.org
2022-09-24 14:10 ` redi at gcc dot gnu.org
2022-09-26 16:42 ` cvs-commit at gcc dot gnu.org
2023-04-29 17:44 ` pinskia at gcc dot gnu.org

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).