[pushed,v2,3/4] Rewrite valid-expr.h's internals in terms of the detection idiom (C++17/N4502)

Message ID 20200914213112.19593-4-pedro@palves.net
State New
Headers show
Series
  • Rewrite enum_flags, add unit tests, fix problems
Related show

Commit Message

Pedro Alves Sept. 14, 2020, 9:31 p.m.
An earlier attempt at doing this had failed (wouldn't work in GCCs
around 4.8, IIRC), but now that I try again, it works.  I suspect that
my previous attempt did not use the pre C++14-safe void_t (in
traits.h).

I want to switch to this model because:

 - It's the standard detection idiom that folks will learn starting
   with C++17.

 - In the enum_flags unit tests, I have a static_assert that triggers
   a warning (resulting in build error), which GCC does not suppress
   because the warning is not being triggered in the SFINAE context.
   Switching to the detection idiom fixes that.  Alternatively,
   switching to the C++03-style expression-validity checking with a
   varargs overload would allow addressing that, but I think that
   would be going backwards idiomatically speaking.

 - While this patch shows a net increase of lines of code, the magic
   being added to traits.h can be removed in a few years when we start
   requiring C++17.

gdbsupport/ChangeLog:

	* traits.h (struct nonesuch, struct detector, detected_or)
	(detected_or_t, is_detected, detected_t, detected_or)
	(detected_or_t, is_detected_exact, is_detected_convertible): New.
	* valid-expr.h (CHECK_VALID_EXPR_INT): Use gdb::is_detected_exact.
---
 gdbsupport/ChangeLog    |  7 ++++++
 gdbsupport/traits.h     | 67 +++++++++++++++++++++++++++++++++++++++++++++++++
 gdbsupport/valid-expr.h | 20 +++------------
 3 files changed, 77 insertions(+), 17 deletions(-)

-- 
2.14.5

Comments

Vaseeharan Vinayagamoorthy Sept. 17, 2020, 10:57 a.m. | #1
Relevant to this patch, I am seeing error: type/value mismatch from valid-expr.h:65:20 when building GDB with:
Build: aarch64-none-linux-gnu
Host: aarch64-none-linux-gnu
Target: aarch64-none-linux-gnu


In file included from binutils-gdb--gdb/gdb/unittests/offset-type-selftests.c:24:0:
/binutils-gdb--gdb/gdb/unittests/offset-type-selftests.c: In substitution of 'template<class Expected, template<class ...> class Op, class ... Args> using is_detected_exact = std::is_same<Expected, typename gdb::detection_detail::detector<gdb::nonesuch, void, Op, Args ...>::type> [with Expected = selftests::offset_type::off_A&; Op = selftests::offset_type::check_valid_expr75::archetype; Args = {selftests::offset_type::off_A, selftests::offset_type::off_B}]':

/binutils-gdb--gdb/gdb/unittests/offset-type-selftests.c:75:1:   required from here
/binutils-gdb--gdb/gdb/../gdbsupport/valid-expr.h:65:20: error: type/value mismatch at argument 2 in template parameter list for 'template<class Expected, template<class ...> class Op, class ... Args> using is_detected_exact = std::is_same<Expected, typename gdb::detection_detail::detector<gdb::nonesuch, void, Op, Args ...>::type>'
    archetype, TYPES>::value == VALID,   \
                    ^
/binutils-gdb--gdb/gdb/../gdbsupport/valid-expr.h:79:3: note: in expansion of macro 'CHECK_VALID_EXPR_INT'
   CHECK_VALID_EXPR_INT (ESC_PARENS(typename T1, typename T2),  \
   ^~~~~~~~~~~~~~~~~~~~
/binutils-gdb--gdb/gdb/unittests/offset-type-selftests.c:42:3: note: in expansion of macro 'CHECK_VALID_EXPR_2'
   CHECK_VALID_EXPR_2 (off_A, off_B, VALID, EXPR_TYPE, EXPR)
   ^~~~~~~~~~~~~~~~~~



Regards,
Vasee




On 14/09/2020, 22:31, "Gdb-patches on behalf of Pedro Alves" <gdb-patches-bounces@sourceware.org on behalf of pedro@palves.net> wrote:

    An earlier attempt at doing this had failed (wouldn't work in GCCs
    around 4.8, IIRC), but now that I try again, it works.  I suspect that
    my previous attempt did not use the pre C++14-safe void_t (in
    traits.h).

    I want to switch to this model because:

     - It's the standard detection idiom that folks will learn starting
       with C++17.

     - In the enum_flags unit tests, I have a static_assert that triggers
       a warning (resulting in build error), which GCC does not suppress
       because the warning is not being triggered in the SFINAE context.
       Switching to the detection idiom fixes that.  Alternatively,
       switching to the C++03-style expression-validity checking with a
       varargs overload would allow addressing that, but I think that
       would be going backwards idiomatically speaking.

     - While this patch shows a net increase of lines of code, the magic
       being added to traits.h can be removed in a few years when we start
       requiring C++17.

    gdbsupport/ChangeLog:

    	* traits.h (struct nonesuch, struct detector, detected_or)
    	(detected_or_t, is_detected, detected_t, detected_or)
    	(detected_or_t, is_detected_exact, is_detected_convertible): New.
    	* valid-expr.h (CHECK_VALID_EXPR_INT): Use gdb::is_detected_exact.
    ---
     gdbsupport/ChangeLog    |  7 ++++++
     gdbsupport/traits.h     | 67 +++++++++++++++++++++++++++++++++++++++++++++++++
     gdbsupport/valid-expr.h | 20 +++------------
     3 files changed, 77 insertions(+), 17 deletions(-)

    diff --git a/gdbsupport/ChangeLog b/gdbsupport/ChangeLog
    index 6cda6050f9..4858cc6b56 100644
    --- a/gdbsupport/ChangeLog
    +++ b/gdbsupport/ChangeLog
    @@ -1,3 +1,10 @@
    +2020-09-14  Pedro Alves  <pedro@palves.net>
    +
    +	* traits.h (struct nonesuch, struct detector, detected_or)
    +	(detected_or_t, is_detected, detected_t, detected_or)
    +	(detected_or_t, is_detected_exact, is_detected_convertible): New.
    +	* valid-expr.h (CHECK_VALID_EXPR_INT): Use gdb::is_detected_exact.
    +
     2020-09-10  Kamil Rytarowski  <n54@gmx.com>

     	* eintr.h: New file.
    diff --git a/gdbsupport/traits.h b/gdbsupport/traits.h
    index 2a6f00654c..93b609ac10 100644
    --- a/gdbsupport/traits.h
    +++ b/gdbsupport/traits.h
    @@ -52,6 +52,73 @@ struct make_void { typedef void type; };
     template<typename... Ts>
     using void_t = typename make_void<Ts...>::type;

    +/* Implementation of the detection idiom:
    +
    +   - http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/n4502.pdf
    +   - http://en.cppreference.com/w/cpp/experimental/is_detected
    +
    +*/
    +
    +struct nonesuch
    +{
    +  nonesuch () = delete;
    +  ~nonesuch () = delete;
    +  nonesuch (const nonesuch &) = delete;
    +  void operator= (const nonesuch &) = delete;
    +};
    +
    +namespace detection_detail {
    +/* Implementation of the detection idiom (negative case).  */
    +template<typename Default, typename AlwaysVoid,
    +	 template<typename...> class Op, typename... Args>
    +struct detector
    +{
    +  using value_t = std::false_type;
    +  using type = Default;
    +};
    +
    +/* Implementation of the detection idiom (positive case).  */
    +template<typename Default, template<typename...> class Op, typename... Args>
    +struct detector<Default, void_t<Op<Args...>>, Op, Args...>
    +{
    +  using value_t = std::true_type;
    +  using type = Op<Args...>;
    +};
    +
    +/* Detect whether Op<Args...> is a valid type, use Default if not.  */
    +template<typename Default, template<typename...> class Op,
    +	 typename... Args>
    +using detected_or = detector<Default, void, Op, Args...>;
    +
    +/* Op<Args...> if that is a valid type, otherwise Default.  */
    +template<typename Default, template<typename...> class Op,
    +	 typename... Args>
    +using detected_or_t
    +  = typename detected_or<Default, Op, Args...>::type;
    +
    +} /* detection_detail */
    +
    +template<template<typename...> class Op, typename... Args>
    +using is_detected
    +  = typename detection_detail::detector<nonesuch, void, Op, Args...>::value_t;
    +
    +template<template<typename...> class Op, typename... Args>
    +using detected_t
    +  = typename detection_detail::detector<nonesuch, void, Op, Args...>::type;
    +
    +template<typename Default, template<typename...> class Op, typename... Args>
    +using detected_or = detection_detail::detected_or<Default, Op, Args...>;
    +
    +template<typename Default, template<typename...> class Op, typename... Args>
    +using detected_or_t = typename detected_or<Default, Op, Args...>::type;
    +
    +template<typename Expected, template<typename...> class Op, typename... Args>
    +using is_detected_exact = std::is_same<Expected, detected_t<Op, Args...>>;
    +
    +template<typename To, template<typename...> class Op, typename... Args>
    +using is_detected_convertible
    +  = std::is_convertible<detected_t<Op, Args...>, To>;
    +
     /* A few trait helpers, mainly stolen from libstdc++.  Uppercase
        because "and/or", etc. are reserved keywords.  */

    diff --git a/gdbsupport/valid-expr.h b/gdbsupport/valid-expr.h
    index b1c8446814..a22fa61134 100644
    --- a/gdbsupport/valid-expr.h
    +++ b/gdbsupport/valid-expr.h
    @@ -58,26 +58,12 @@
     #define CHECK_VALID_EXPR_INT(TYPENAMES, TYPES, VALID, EXPR_TYPE, EXPR)	\
       namespace CONCAT (check_valid_expr, __LINE__) {			\
     									\
    -  template<typename, typename, typename = void>				\
    -  struct is_valid_expression						\
    -    : std::false_type {};						\
    -									\
       template <TYPENAMES>							\
    -    struct is_valid_expression<TYPES, gdb::void_t<decltype (EXPR)>>	\
    -    : std::true_type {};						\
    +    using archetype = decltype (EXPR);					\
     									\
    -  static_assert (is_valid_expression<TYPES>::value == VALID,		\
    +  static_assert (gdb::is_detected_exact<EXPR_TYPE,			\
    +		 archetype, TYPES>::value == VALID,			\
     		 "");							\
    -									\
    -  template<TYPENAMES, typename = void>					\
    -  struct is_same_type							\
    -    : std::is_same<EXPR_TYPE, void> {};					\
    -									\
    -  template <TYPENAMES>							\
    -    struct is_same_type<TYPES, gdb::void_t<decltype (EXPR)>>		\
    -    : std::is_same<EXPR_TYPE, decltype (EXPR)> {};			\
    -									\
    -  static_assert (is_same_type<TYPES>::value, "");			\
       } /* namespace */

     /* A few convenience macros that support expressions involving a
    -- 
    2.14.5
Simon Marchi via Gdb-patches Sept. 17, 2020, 11:41 a.m. | #2
On 9/17/20 7:57 AM, Vaseeharan Vinayagamoorthy wrote:
> Relevant to this patch, I am seeing error: type/value mismatch from valid-expr.h:65:20 when building GDB with:

> Build: aarch64-none-linux-gnu

> Host: aarch64-none-linux-gnu

> Target: aarch64-none-linux-gnu

> 

> 

> In file included from binutils-gdb--gdb/gdb/unittests/offset-type-selftests.c:24:0:

> /binutils-gdb--gdb/gdb/unittests/offset-type-selftests.c: In substitution of 'template<class Expected, template<class ...> class Op, class ... Args> using is_detected_exact = std::is_same<Expected, typename gdb::detection_detail::detector<gdb::nonesuch, void, Op, Args ...>::type> [with Expected = selftests::offset_type::off_A&; Op = selftests::offset_type::check_valid_expr75::archetype; Args = {selftests::offset_type::off_A, selftests::offset_type::off_B}]':

> 

> /binutils-gdb--gdb/gdb/unittests/offset-type-selftests.c:75:1:   required from here

> /binutils-gdb--gdb/gdb/../gdbsupport/valid-expr.h:65:20: error: type/value mismatch at argument 2 in template parameter list for 'template<class Expected, template<class ...> class Op, class ... Args> using is_detected_exact = std::is_same<Expected, typename gdb::detection_detail::detector<gdb::nonesuch, void, Op, Args ...>::type>'

>      archetype, TYPES>::value == VALID,   \

>                      ^

> /binutils-gdb--gdb/gdb/../gdbsupport/valid-expr.h:79:3: note: in expansion of macro 'CHECK_VALID_EXPR_INT'

>     CHECK_VALID_EXPR_INT (ESC_PARENS(typename T1, typename T2),  \

>     ^~~~~~~~~~~~~~~~~~~~

> /binutils-gdb--gdb/gdb/unittests/offset-type-selftests.c:42:3: note: in expansion of macro 'CHECK_VALID_EXPR_2'

>     CHECK_VALID_EXPR_2 (off_A, off_B, VALID, EXPR_TYPE, EXPR)

>     ^~~~~~~~~~~~~~~~~~

> 

> 

> 

> Regards,

> Vasee



I haven't seen this. It's been building fine so far. What type of system 
are you using? I'm on Ubuntu 18.04 with GCC 7.5.
Vaseeharan Vinayagamoorthy Sept. 17, 2020, 4:10 p.m. | #3
I observe this error when building on an aarch64-none-linux-gnu machine, running ubuntu 14.04.5.
The compiler it is using is aarch64-none-linux-gnu-gcc version 6.4.1.


On 17/09/2020, 11:58, "Gdb-patches on behalf of Vaseeharan Vinayagamoorthy" <gdb-patches-bounces@sourceware.org on behalf of Vaseeharan.Vinayagamoorthy@arm.com> wrote:

    Relevant to this patch, I am seeing error: type/value mismatch from valid-expr.h:65:20 when building GDB with:
    Build: aarch64-none-linux-gnu
    Host: aarch64-none-linux-gnu
    Target: aarch64-none-linux-gnu


    In file included from binutils-gdb--gdb/gdb/unittests/offset-type-selftests.c:24:0:
    /binutils-gdb--gdb/gdb/unittests/offset-type-selftests.c: In substitution of 'template<class Expected, template<class ...> class Op, class ... Args> using is_detected_exact = std::is_same<Expected, typename gdb::detection_detail::detector<gdb::nonesuch, void, Op, Args ...>::type> [with Expected = selftests::offset_type::off_A&; Op = selftests::offset_type::check_valid_expr75::archetype; Args = {selftests::offset_type::off_A, selftests::offset_type::off_B}]':

    /binutils-gdb--gdb/gdb/unittests/offset-type-selftests.c:75:1:   required from here
    /binutils-gdb--gdb/gdb/../gdbsupport/valid-expr.h:65:20: error: type/value mismatch at argument 2 in template parameter list for 'template<class Expected, template<class ...> class Op, class ... Args> using is_detected_exact = std::is_same<Expected, typename gdb::detection_detail::detector<gdb::nonesuch, void, Op, Args ...>::type>'
        archetype, TYPES>::value == VALID,   \
                        ^
    /binutils-gdb--gdb/gdb/../gdbsupport/valid-expr.h:79:3: note: in expansion of macro 'CHECK_VALID_EXPR_INT'
       CHECK_VALID_EXPR_INT (ESC_PARENS(typename T1, typename T2),  \
       ^~~~~~~~~~~~~~~~~~~~
    /binutils-gdb--gdb/gdb/unittests/offset-type-selftests.c:42:3: note: in expansion of macro 'CHECK_VALID_EXPR_2'
       CHECK_VALID_EXPR_2 (off_A, off_B, VALID, EXPR_TYPE, EXPR)
       ^~~~~~~~~~~~~~~~~~



    Regards,
    Vasee




    On 14/09/2020, 22:31, "Gdb-patches on behalf of Pedro Alves" <gdb-patches-bounces@sourceware.org on behalf of pedro@palves.net> wrote:

        An earlier attempt at doing this had failed (wouldn't work in GCCs
        around 4.8, IIRC), but now that I try again, it works.  I suspect that
        my previous attempt did not use the pre C++14-safe void_t (in
        traits.h).

        I want to switch to this model because:

         - It's the standard detection idiom that folks will learn starting
           with C++17.

         - In the enum_flags unit tests, I have a static_assert that triggers
           a warning (resulting in build error), which GCC does not suppress
           because the warning is not being triggered in the SFINAE context.
           Switching to the detection idiom fixes that.  Alternatively,
           switching to the C++03-style expression-validity checking with a
           varargs overload would allow addressing that, but I think that
           would be going backwards idiomatically speaking.

         - While this patch shows a net increase of lines of code, the magic
           being added to traits.h can be removed in a few years when we start
           requiring C++17.

        gdbsupport/ChangeLog:

        	* traits.h (struct nonesuch, struct detector, detected_or)
        	(detected_or_t, is_detected, detected_t, detected_or)
        	(detected_or_t, is_detected_exact, is_detected_convertible): New.
        	* valid-expr.h (CHECK_VALID_EXPR_INT): Use gdb::is_detected_exact.
        ---
         gdbsupport/ChangeLog    |  7 ++++++
         gdbsupport/traits.h     | 67 +++++++++++++++++++++++++++++++++++++++++++++++++
         gdbsupport/valid-expr.h | 20 +++------------
         3 files changed, 77 insertions(+), 17 deletions(-)

        diff --git a/gdbsupport/ChangeLog b/gdbsupport/ChangeLog
        index 6cda6050f9..4858cc6b56 100644
        --- a/gdbsupport/ChangeLog
        +++ b/gdbsupport/ChangeLog
        @@ -1,3 +1,10 @@
        +2020-09-14  Pedro Alves  <pedro@palves.net>
        +
        +	* traits.h (struct nonesuch, struct detector, detected_or)
        +	(detected_or_t, is_detected, detected_t, detected_or)
        +	(detected_or_t, is_detected_exact, is_detected_convertible): New.
        +	* valid-expr.h (CHECK_VALID_EXPR_INT): Use gdb::is_detected_exact.
        +
         2020-09-10  Kamil Rytarowski  <n54@gmx.com>

         	* eintr.h: New file.
        diff --git a/gdbsupport/traits.h b/gdbsupport/traits.h
        index 2a6f00654c..93b609ac10 100644
        --- a/gdbsupport/traits.h
        +++ b/gdbsupport/traits.h
        @@ -52,6 +52,73 @@ struct make_void { typedef void type; };
         template<typename... Ts>
         using void_t = typename make_void<Ts...>::type;

        +/* Implementation of the detection idiom:
        +
        +   - http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/n4502.pdf
        +   - http://en.cppreference.com/w/cpp/experimental/is_detected
        +
        +*/
        +
        +struct nonesuch
        +{
        +  nonesuch () = delete;
        +  ~nonesuch () = delete;
        +  nonesuch (const nonesuch &) = delete;
        +  void operator= (const nonesuch &) = delete;
        +};
        +
        +namespace detection_detail {
        +/* Implementation of the detection idiom (negative case).  */
        +template<typename Default, typename AlwaysVoid,
        +	 template<typename...> class Op, typename... Args>
        +struct detector
        +{
        +  using value_t = std::false_type;
        +  using type = Default;
        +};
        +
        +/* Implementation of the detection idiom (positive case).  */
        +template<typename Default, template<typename...> class Op, typename... Args>
        +struct detector<Default, void_t<Op<Args...>>, Op, Args...>
        +{
        +  using value_t = std::true_type;
        +  using type = Op<Args...>;
        +};
        +
        +/* Detect whether Op<Args...> is a valid type, use Default if not.  */
        +template<typename Default, template<typename...> class Op,
        +	 typename... Args>
        +using detected_or = detector<Default, void, Op, Args...>;
        +
        +/* Op<Args...> if that is a valid type, otherwise Default.  */
        +template<typename Default, template<typename...> class Op,
        +	 typename... Args>
        +using detected_or_t
        +  = typename detected_or<Default, Op, Args...>::type;
        +
        +} /* detection_detail */
        +
        +template<template<typename...> class Op, typename... Args>
        +using is_detected
        +  = typename detection_detail::detector<nonesuch, void, Op, Args...>::value_t;
        +
        +template<template<typename...> class Op, typename... Args>
        +using detected_t
        +  = typename detection_detail::detector<nonesuch, void, Op, Args...>::type;
        +
        +template<typename Default, template<typename...> class Op, typename... Args>
        +using detected_or = detection_detail::detected_or<Default, Op, Args...>;
        +
        +template<typename Default, template<typename...> class Op, typename... Args>
        +using detected_or_t = typename detected_or<Default, Op, Args...>::type;
        +
        +template<typename Expected, template<typename...> class Op, typename... Args>
        +using is_detected_exact = std::is_same<Expected, detected_t<Op, Args...>>;
        +
        +template<typename To, template<typename...> class Op, typename... Args>
        +using is_detected_convertible
        +  = std::is_convertible<detected_t<Op, Args...>, To>;
        +
         /* A few trait helpers, mainly stolen from libstdc++.  Uppercase
            because "and/or", etc. are reserved keywords.  */

        diff --git a/gdbsupport/valid-expr.h b/gdbsupport/valid-expr.h
        index b1c8446814..a22fa61134 100644
        --- a/gdbsupport/valid-expr.h
        +++ b/gdbsupport/valid-expr.h
        @@ -58,26 +58,12 @@
         #define CHECK_VALID_EXPR_INT(TYPENAMES, TYPES, VALID, EXPR_TYPE, EXPR)	\
           namespace CONCAT (check_valid_expr, __LINE__) {			\
         									\
        -  template<typename, typename, typename = void>				\
        -  struct is_valid_expression						\
        -    : std::false_type {};						\
        -									\
           template <TYPENAMES>							\
        -    struct is_valid_expression<TYPES, gdb::void_t<decltype (EXPR)>>	\
        -    : std::true_type {};						\
        +    using archetype = decltype (EXPR);					\
         									\
        -  static_assert (is_valid_expression<TYPES>::value == VALID,		\
        +  static_assert (gdb::is_detected_exact<EXPR_TYPE,			\
        +		 archetype, TYPES>::value == VALID,			\
         		 "");							\
        -									\
        -  template<TYPENAMES, typename = void>					\
        -  struct is_same_type							\
        -    : std::is_same<EXPR_TYPE, void> {};					\
        -									\
        -  template <TYPENAMES>							\
        -    struct is_same_type<TYPES, gdb::void_t<decltype (EXPR)>>		\
        -    : std::is_same<EXPR_TYPE, decltype (EXPR)> {};			\
        -									\
        -  static_assert (is_same_type<TYPES>::value, "");			\
           } /* namespace */

         /* A few convenience macros that support expressions involving a
        -- 
        2.14.5
Simon Marchi via Gdb-patches Sept. 17, 2020, 4:23 p.m. | #4
Ok. Even though 14.04 is no longer supported, I suspect GCC 6.4.1 is 
still fairly recent that we shouldn't be seeing this.

On 9/17/20 1:10 PM, Vaseeharan Vinayagamoorthy wrote:
> I observe this error when building on an aarch64-none-linux-gnu machine, running ubuntu 14.04.5.

> The compiler it is using is aarch64-none-linux-gnu-gcc version 6.4.1.

> 

> 

> On 17/09/2020, 11:58, "Gdb-patches on behalf of Vaseeharan Vinayagamoorthy" <gdb-patches-bounces@sourceware.org on behalf of Vaseeharan.Vinayagamoorthy@arm.com> wrote:

> 

>      Relevant to this patch, I am seeing error: type/value mismatch from valid-expr.h:65:20 when building GDB with:

>      Build: aarch64-none-linux-gnu

>      Host: aarch64-none-linux-gnu

>      Target: aarch64-none-linux-gnu

> 

> 

>      In file included from binutils-gdb--gdb/gdb/unittests/offset-type-selftests.c:24:0:

>      /binutils-gdb--gdb/gdb/unittests/offset-type-selftests.c: In substitution of 'template<class Expected, template<class ...> class Op, class ... Args> using is_detected_exact = std::is_same<Expected, typename gdb::detection_detail::detector<gdb::nonesuch, void, Op, Args ...>::type> [with Expected = selftests::offset_type::off_A&; Op = selftests::offset_type::check_valid_expr75::archetype; Args = {selftests::offset_type::off_A, selftests::offset_type::off_B}]':

> 

>      /binutils-gdb--gdb/gdb/unittests/offset-type-selftests.c:75:1:   required from here

>      /binutils-gdb--gdb/gdb/../gdbsupport/valid-expr.h:65:20: error: type/value mismatch at argument 2 in template parameter list for 'template<class Expected, template<class ...> class Op, class ... Args> using is_detected_exact = std::is_same<Expected, typename gdb::detection_detail::detector<gdb::nonesuch, void, Op, Args ...>::type>'

>          archetype, TYPES>::value == VALID,   \

>                          ^

>      /binutils-gdb--gdb/gdb/../gdbsupport/valid-expr.h:79:3: note: in expansion of macro 'CHECK_VALID_EXPR_INT'

>         CHECK_VALID_EXPR_INT (ESC_PARENS(typename T1, typename T2),  \

>         ^~~~~~~~~~~~~~~~~~~~

>      /binutils-gdb--gdb/gdb/unittests/offset-type-selftests.c:42:3: note: in expansion of macro 'CHECK_VALID_EXPR_2'

>         CHECK_VALID_EXPR_2 (off_A, off_B, VALID, EXPR_TYPE, EXPR)

>         ^~~~~~~~~~~~~~~~~~

> 

> 

> 

>      Regards,

>      Vasee

> 

> 

> 

> 

>      On 14/09/2020, 22:31, "Gdb-patches on behalf of Pedro Alves" <gdb-patches-bounces@sourceware.org on behalf of pedro@palves.net> wrote:

> 

>          An earlier attempt at doing this had failed (wouldn't work in GCCs

>          around 4.8, IIRC), but now that I try again, it works.  I suspect that

>          my previous attempt did not use the pre C++14-safe void_t (in

>          traits.h).

> 

>          I want to switch to this model because:

> 

>           - It's the standard detection idiom that folks will learn starting

>             with C++17.

> 

>           - In the enum_flags unit tests, I have a static_assert that triggers

>             a warning (resulting in build error), which GCC does not suppress

>             because the warning is not being triggered in the SFINAE context.

>             Switching to the detection idiom fixes that.  Alternatively,

>             switching to the C++03-style expression-validity checking with a

>             varargs overload would allow addressing that, but I think that

>             would be going backwards idiomatically speaking.

> 

>           - While this patch shows a net increase of lines of code, the magic

>             being added to traits.h can be removed in a few years when we start

>             requiring C++17.

> 

>          gdbsupport/ChangeLog:

> 

>          	* traits.h (struct nonesuch, struct detector, detected_or)

>          	(detected_or_t, is_detected, detected_t, detected_or)

>          	(detected_or_t, is_detected_exact, is_detected_convertible): New.

>          	* valid-expr.h (CHECK_VALID_EXPR_INT): Use gdb::is_detected_exact.

>          ---

>           gdbsupport/ChangeLog    |  7 ++++++

>           gdbsupport/traits.h     | 67 +++++++++++++++++++++++++++++++++++++++++++++++++

>           gdbsupport/valid-expr.h | 20 +++------------

>           3 files changed, 77 insertions(+), 17 deletions(-)

> 

>          diff --git a/gdbsupport/ChangeLog b/gdbsupport/ChangeLog

>          index 6cda6050f9..4858cc6b56 100644

>          --- a/gdbsupport/ChangeLog

>          +++ b/gdbsupport/ChangeLog

>          @@ -1,3 +1,10 @@

>          +2020-09-14  Pedro Alves  <pedro@palves.net>

>          +

>          +	* traits.h (struct nonesuch, struct detector, detected_or)

>          +	(detected_or_t, is_detected, detected_t, detected_or)

>          +	(detected_or_t, is_detected_exact, is_detected_convertible): New.

>          +	* valid-expr.h (CHECK_VALID_EXPR_INT): Use gdb::is_detected_exact.

>          +

>           2020-09-10  Kamil Rytarowski  <n54@gmx.com>

> 

>           	* eintr.h: New file.

>          diff --git a/gdbsupport/traits.h b/gdbsupport/traits.h

>          index 2a6f00654c..93b609ac10 100644

>          --- a/gdbsupport/traits.h

>          +++ b/gdbsupport/traits.h

>          @@ -52,6 +52,73 @@ struct make_void { typedef void type; };

>           template<typename... Ts>

>           using void_t = typename make_void<Ts...>::type;

> 

>          +/* Implementation of the detection idiom:

>          +

>          +   - http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/n4502.pdf

>          +   - http://en.cppreference.com/w/cpp/experimental/is_detected

>          +

>          +*/

>          +

>          +struct nonesuch

>          +{

>          +  nonesuch () = delete;

>          +  ~nonesuch () = delete;

>          +  nonesuch (const nonesuch &) = delete;

>          +  void operator= (const nonesuch &) = delete;

>          +};

>          +

>          +namespace detection_detail {

>          +/* Implementation of the detection idiom (negative case).  */

>          +template<typename Default, typename AlwaysVoid,

>          +	 template<typename...> class Op, typename... Args>

>          +struct detector

>          +{

>          +  using value_t = std::false_type;

>          +  using type = Default;

>          +};

>          +

>          +/* Implementation of the detection idiom (positive case).  */

>          +template<typename Default, template<typename...> class Op, typename... Args>

>          +struct detector<Default, void_t<Op<Args...>>, Op, Args...>

>          +{

>          +  using value_t = std::true_type;

>          +  using type = Op<Args...>;

>          +};

>          +

>          +/* Detect whether Op<Args...> is a valid type, use Default if not.  */

>          +template<typename Default, template<typename...> class Op,

>          +	 typename... Args>

>          +using detected_or = detector<Default, void, Op, Args...>;

>          +

>          +/* Op<Args...> if that is a valid type, otherwise Default.  */

>          +template<typename Default, template<typename...> class Op,

>          +	 typename... Args>

>          +using detected_or_t

>          +  = typename detected_or<Default, Op, Args...>::type;

>          +

>          +} /* detection_detail */

>          +

>          +template<template<typename...> class Op, typename... Args>

>          +using is_detected

>          +  = typename detection_detail::detector<nonesuch, void, Op, Args...>::value_t;

>          +

>          +template<template<typename...> class Op, typename... Args>

>          +using detected_t

>          +  = typename detection_detail::detector<nonesuch, void, Op, Args...>::type;

>          +

>          +template<typename Default, template<typename...> class Op, typename... Args>

>          +using detected_or = detection_detail::detected_or<Default, Op, Args...>;

>          +

>          +template<typename Default, template<typename...> class Op, typename... Args>

>          +using detected_or_t = typename detected_or<Default, Op, Args...>::type;

>          +

>          +template<typename Expected, template<typename...> class Op, typename... Args>

>          +using is_detected_exact = std::is_same<Expected, detected_t<Op, Args...>>;

>          +

>          +template<typename To, template<typename...> class Op, typename... Args>

>          +using is_detected_convertible

>          +  = std::is_convertible<detected_t<Op, Args...>, To>;

>          +

>           /* A few trait helpers, mainly stolen from libstdc++.  Uppercase

>              because "and/or", etc. are reserved keywords.  */

> 

>          diff --git a/gdbsupport/valid-expr.h b/gdbsupport/valid-expr.h

>          index b1c8446814..a22fa61134 100644

>          --- a/gdbsupport/valid-expr.h

>          +++ b/gdbsupport/valid-expr.h

>          @@ -58,26 +58,12 @@

>           #define CHECK_VALID_EXPR_INT(TYPENAMES, TYPES, VALID, EXPR_TYPE, EXPR)	\

>             namespace CONCAT (check_valid_expr, __LINE__) {			\

>           									\

>          -  template<typename, typename, typename = void>				\

>          -  struct is_valid_expression						\

>          -    : std::false_type {};						\

>          -									\

>             template <TYPENAMES>							\

>          -    struct is_valid_expression<TYPES, gdb::void_t<decltype (EXPR)>>	\

>          -    : std::true_type {};						\

>          +    using archetype = decltype (EXPR);					\

>           									\

>          -  static_assert (is_valid_expression<TYPES>::value == VALID,		\

>          +  static_assert (gdb::is_detected_exact<EXPR_TYPE,			\

>          +		 archetype, TYPES>::value == VALID,			\

>           		 "");							\

>          -									\

>          -  template<TYPENAMES, typename = void>					\

>          -  struct is_same_type							\

>          -    : std::is_same<EXPR_TYPE, void> {};					\

>          -									\

>          -  template <TYPENAMES>							\

>          -    struct is_same_type<TYPES, gdb::void_t<decltype (EXPR)>>		\

>          -    : std::is_same<EXPR_TYPE, decltype (EXPR)> {};			\

>          -									\

>          -  static_assert (is_same_type<TYPES>::value, "");			\

>             } /* namespace */

> 

>           /* A few convenience macros that support expressions involving a

>          --

>          2.14.5

> 

> 

>

Patch

diff --git a/gdbsupport/ChangeLog b/gdbsupport/ChangeLog
index 6cda6050f9..4858cc6b56 100644
--- a/gdbsupport/ChangeLog
+++ b/gdbsupport/ChangeLog
@@ -1,3 +1,10 @@ 
+2020-09-14  Pedro Alves  <pedro@palves.net>
+
+	* traits.h (struct nonesuch, struct detector, detected_or)
+	(detected_or_t, is_detected, detected_t, detected_or)
+	(detected_or_t, is_detected_exact, is_detected_convertible): New.
+	* valid-expr.h (CHECK_VALID_EXPR_INT): Use gdb::is_detected_exact.
+
 2020-09-10  Kamil Rytarowski  <n54@gmx.com>
 
 	* eintr.h: New file.
diff --git a/gdbsupport/traits.h b/gdbsupport/traits.h
index 2a6f00654c..93b609ac10 100644
--- a/gdbsupport/traits.h
+++ b/gdbsupport/traits.h
@@ -52,6 +52,73 @@  struct make_void { typedef void type; };
 template<typename... Ts>
 using void_t = typename make_void<Ts...>::type;
 
+/* Implementation of the detection idiom:
+
+   - http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/n4502.pdf
+   - http://en.cppreference.com/w/cpp/experimental/is_detected
+
+*/
+
+struct nonesuch
+{
+  nonesuch () = delete;
+  ~nonesuch () = delete;
+  nonesuch (const nonesuch &) = delete;
+  void operator= (const nonesuch &) = delete;
+};
+
+namespace detection_detail {
+/* Implementation of the detection idiom (negative case).  */
+template<typename Default, typename AlwaysVoid,
+	 template<typename...> class Op, typename... Args>
+struct detector
+{
+  using value_t = std::false_type;
+  using type = Default;
+};
+
+/* Implementation of the detection idiom (positive case).  */
+template<typename Default, template<typename...> class Op, typename... Args>
+struct detector<Default, void_t<Op<Args...>>, Op, Args...>
+{
+  using value_t = std::true_type;
+  using type = Op<Args...>;
+};
+
+/* Detect whether Op<Args...> is a valid type, use Default if not.  */
+template<typename Default, template<typename...> class Op,
+	 typename... Args>
+using detected_or = detector<Default, void, Op, Args...>;
+
+/* Op<Args...> if that is a valid type, otherwise Default.  */
+template<typename Default, template<typename...> class Op,
+	 typename... Args>
+using detected_or_t
+  = typename detected_or<Default, Op, Args...>::type;
+
+} /* detection_detail */
+
+template<template<typename...> class Op, typename... Args>
+using is_detected
+  = typename detection_detail::detector<nonesuch, void, Op, Args...>::value_t;
+
+template<template<typename...> class Op, typename... Args>
+using detected_t
+  = typename detection_detail::detector<nonesuch, void, Op, Args...>::type;
+
+template<typename Default, template<typename...> class Op, typename... Args>
+using detected_or = detection_detail::detected_or<Default, Op, Args...>;
+
+template<typename Default, template<typename...> class Op, typename... Args>
+using detected_or_t = typename detected_or<Default, Op, Args...>::type;
+
+template<typename Expected, template<typename...> class Op, typename... Args>
+using is_detected_exact = std::is_same<Expected, detected_t<Op, Args...>>;
+
+template<typename To, template<typename...> class Op, typename... Args>
+using is_detected_convertible
+  = std::is_convertible<detected_t<Op, Args...>, To>;
+
 /* A few trait helpers, mainly stolen from libstdc++.  Uppercase
    because "and/or", etc. are reserved keywords.  */
 
diff --git a/gdbsupport/valid-expr.h b/gdbsupport/valid-expr.h
index b1c8446814..a22fa61134 100644
--- a/gdbsupport/valid-expr.h
+++ b/gdbsupport/valid-expr.h
@@ -58,26 +58,12 @@ 
 #define CHECK_VALID_EXPR_INT(TYPENAMES, TYPES, VALID, EXPR_TYPE, EXPR)	\
   namespace CONCAT (check_valid_expr, __LINE__) {			\
 									\
-  template<typename, typename, typename = void>				\
-  struct is_valid_expression						\
-    : std::false_type {};						\
-									\
   template <TYPENAMES>							\
-    struct is_valid_expression<TYPES, gdb::void_t<decltype (EXPR)>>	\
-    : std::true_type {};						\
+    using archetype = decltype (EXPR);					\
 									\
-  static_assert (is_valid_expression<TYPES>::value == VALID,		\
+  static_assert (gdb::is_detected_exact<EXPR_TYPE,			\
+		 archetype, TYPES>::value == VALID,			\
 		 "");							\
-									\
-  template<TYPENAMES, typename = void>					\
-  struct is_same_type							\
-    : std::is_same<EXPR_TYPE, void> {};					\
-									\
-  template <TYPENAMES>							\
-    struct is_same_type<TYPES, gdb::void_t<decltype (EXPR)>>		\
-    : std::is_same<EXPR_TYPE, decltype (EXPR)> {};			\
-									\
-  static_assert (is_same_type<TYPES>::value, "");			\
   } /* namespace */
 
 /* A few convenience macros that support expressions involving a