diff mbox

Remove relational operators from common/offset-type.h

Message ID 20181029211401.21602-1-sergiodj@redhat.com
State New, archived
Headers

Commit Message

Sergio Durigan Junior Oct. 29, 2018, 9:14 p.m. UTC 
  This patch is a follow-up of:

  https://sourceware.org/ml/gdb-patches/2018-10/msg00601.html

It removes the declaration of the relational operators for
common/offset-type.h.  As it turns out, these overloads are not being
used when a new offset type is declared, because, according to Pedro
Alves:

  I think the functions aren't called because they are templates, and
  thus the built-in (non-template) versions take precedence.  If you
  make them non-templates, then they should be called.  But, the
  built-ins are fine, so yeah, we can just remove the custom
  definitions.

The patch also adjusts the comments on the code.

No regressions introduced.

gdb/ChangeLog:
2018-10-29  Sergio Durigan Junior  <sergiodj@redhat.com>

	* common/offset-type.h (DEFINE_OFFSET_REL_OP): Delete.
	Adjust comments.
---
 gdb/ChangeLog            |  5 +++++
 gdb/common/offset-type.h | 18 +-----------------
 2 files changed, 6 insertions(+), 17 deletions(-)

Comments

Simon Marchi Oct. 30, 2018, 3:17 a.m. UTC | #1 
On 2018-10-29 17:14, Sergio Durigan Junior wrote:
> This patch is a follow-up of:
> 
>   https://sourceware.org/ml/gdb-patches/2018-10/msg00601.html
> 
> It removes the declaration of the relational operators for
> common/offset-type.h.  As it turns out, these overloads are not being
> used when a new offset type is declared, because, according to Pedro
> Alves:
> 
>   I think the functions aren't called because they are templates, and
>   thus the built-in (non-template) versions take precedence.  If you
>   make them non-templates, then they should be called.  But, the
>   built-ins are fine, so yeah, we can just remove the custom
>   definitions.
> 
> The patch also adjusts the comments on the code.
> 
> No regressions introduced.
> 
> gdb/ChangeLog:
> 2018-10-29  Sergio Durigan Junior  <sergiodj@redhat.com>
> 
> 	* common/offset-type.h (DEFINE_OFFSET_REL_OP): Delete.
> 	Adjust comments.

Thanks, LGTM.

Simon
Sergio Durigan Junior Oct. 30, 2018, 3:49 a.m. UTC | #2 
On Monday, October 29 2018, Simon Marchi wrote:

> On 2018-10-29 17:14, Sergio Durigan Junior wrote:
>> This patch is a follow-up of:
>>
>>   https://sourceware.org/ml/gdb-patches/2018-10/msg00601.html
>>
>> It removes the declaration of the relational operators for
>> common/offset-type.h.  As it turns out, these overloads are not being
>> used when a new offset type is declared, because, according to Pedro
>> Alves:
>>
>>   I think the functions aren't called because they are templates, and
>>   thus the built-in (non-template) versions take precedence.  If you
>>   make them non-templates, then they should be called.  But, the
>>   built-ins are fine, so yeah, we can just remove the custom
>>   definitions.
>>
>> The patch also adjusts the comments on the code.
>>
>> No regressions introduced.
>>
>> gdb/ChangeLog:
>> 2018-10-29  Sergio Durigan Junior  <sergiodj@redhat.com>
>>
>> 	* common/offset-type.h (DEFINE_OFFSET_REL_OP): Delete.
>> 	Adjust comments.
>
> Thanks, LGTM.

Thanks, pushed.

fd332753fa7050bb9d7c89147e32d285099fe402
diff mbox

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 1cba619fd9..6fb02d26ea 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@ 
+2018-10-29  Sergio Durigan Junior  <sergiodj@redhat.com>
+
+	* common/offset-type.h (DEFINE_OFFSET_REL_OP): Delete.
+	Adjust comments.
+
 2018-10-29  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>
 
 	* procfs.c: Include common/pathstuff.h.
diff --git a/gdb/common/offset-type.h b/gdb/common/offset-type.h
index b480b14406..174ad1e456 100644
--- a/gdb/common/offset-type.h
+++ b/gdb/common/offset-type.h
@@ -57,7 +57,7 @@ 
 /* The macro macro is all you need to know use offset types.  The rest
    below is all implementation detail.  */
 
-/* For each enum class type that you want to support relational
+/* For each enum class type that you want to support arithmetic
    operators, declare an "is_offset_type" overload that has exactly
    one parameter, of type that enum class.  E.g.,:
 
@@ -73,22 +73,6 @@ 
    function via ADL.
 */
 
-#define DEFINE_OFFSET_REL_OP(OP)					\
-  template<typename E,							\
-	   typename = decltype (is_offset_type (std::declval<E> ()))>	\
-  constexpr bool							\
-  operator OP (E lhs, E rhs)						\
-  {									\
-    using underlying = typename std::underlying_type<E>::type;		\
-    return (static_cast<underlying> (lhs)				\
-	    OP static_cast<underlying> (lhs));				\
-  }
-
-DEFINE_OFFSET_REL_OP(>)
-DEFINE_OFFSET_REL_OP(>=)
-DEFINE_OFFSET_REL_OP(<)
-DEFINE_OFFSET_REL_OP(<=)
-
 /* Adding or subtracting an integer to an offset type shifts the
    offset.  This is like "PTR = PTR + INT" and "PTR += INT".  */