diff mbox series

implement generic debug() functions for wide_int's

Message ID CAGm3qMUoL5g4EszRwiPpfGuTJOPY5bOqigiYpzWUpkoUJTzvxg@mail.gmail.com
State New
Headers show
Series implement generic debug() functions for wide_int's | expand

Commit Message

Aldy Hernandez Oct. 18, 2017, 5:08 p.m. UTC
It looks like the generic debug() function for wide_int's is missing.
Instead, we have a wi->dump() method.  I have implemented debug() for
generic wide_int and for widest_int, which should cover the common
cases.  Anything else, can continue using the ->dump() method
templated methods.

Also, do we have a blessed way of specifying overloaded functions in
ChangeLog's?  I couldn't find anything in our GCC coding guidelines or
in the GNU coding guidelines.  For lack of direction, I'm doing the
following:

* wide-int.cc (debug) [const wide_int &]: New.
(debug) [const wide_int *]: New.
(debug) [const widest_int &]: New.
(debug) [const widest_int *]: New.

It seems appropriate, as that is the GNU way of changelogs for a
conditional change to a function ???.

OK for trunk?

Comments

Mike Stump Oct. 18, 2017, 7:29 p.m. UTC | #1
On Oct 18, 2017, at 10:08 AM, Aldy Hernandez <aldyh@redhat.com> wrote:
> 
> It looks like the generic debug() function for wide_int's is missing.

> OK for trunk?

Ok.
Pedro Alves Oct. 18, 2017, 10:22 p.m. UTC | #2
On 10/18/2017 06:08 PM, Aldy Hernandez wrote:

> Also, do we have a blessed way of specifying overloaded functions in
> ChangeLog's?  I couldn't find anything in our GCC coding guidelines or
> in the GNU coding guidelines.  For lack of direction, I'm doing the
> following:
> 
> * wide-int.cc (debug) [const wide_int &]: New.
> (debug) [const wide_int *]: New.
> (debug) [const widest_int &]: New.
> (debug) [const widest_int *]: New.
> 
> It seems appropriate, as that is the GNU way of changelogs for a
> conditional change to a function ???.

Doesn't seem that appropriate to me.  Square brackets are used for
conditional compilation (#ifdef etc.), but overloads are not that.

I'd suggest looking in libstdc++'s ChangeLog for precedents.  It's where
I looked at when I had the same question for GDB, FWIW.  E.g., a very
recent libstdc++ commit from Jon had:

        * include/bits/stl_map.h (map::insert(value_type&&))
        (map::insert(const_iterator, value_type&&)): Add overload for rvalues.

I.e., simply use the full prototype as function name, since it's
really what it is in C++.

Thanks,
Pedro Alves
Martin Sebor Oct. 19, 2017, 3:23 a.m. UTC | #3
On 10/18/2017 11:08 AM, Aldy Hernandez wrote:
> It looks like the generic debug() function for wide_int's is missing.
> Instead, we have a wi->dump() method.  I have implemented debug() for
> generic wide_int and for widest_int, which should cover the common
> cases.  Anything else, can continue using the ->dump() method
> templated methods.

Do you by any chance also plan to add support to the pretty printer
for wide_int (and offset_int)?  It would obviate having to convert
them to {s,u}hwi.

Martin
Aldy Hernandez Oct. 19, 2017, 7:51 a.m. UTC | #4
On 10/18/2017 06:22 PM, Pedro Alves wrote:
> On 10/18/2017 06:08 PM, Aldy Hernandez wrote:
> 
>> Also, do we have a blessed way of specifying overloaded functions in
>> ChangeLog's?  I couldn't find anything in our GCC coding guidelines or
>> in the GNU coding guidelines.  For lack of direction, I'm doing the
>> following:
>>
>> * wide-int.cc (debug) [const wide_int &]: New.
>> (debug) [const wide_int *]: New.
>> (debug) [const widest_int &]: New.
>> (debug) [const widest_int *]: New.
>>
>> It seems appropriate, as that is the GNU way of changelogs for a
>> conditional change to a function ???.
> 
> Doesn't seem that appropriate to me.  Square brackets are used for
> conditional compilation (#ifdef etc.), but overloads are not that.
> 
> I'd suggest looking in libstdc++'s ChangeLog for precedents.  It's where
> I looked at when I had the same question for GDB, FWIW.  E.g., a very
> recent libstdc++ commit from Jon had:
> 
>          * include/bits/stl_map.h (map::insert(value_type&&))
>          (map::insert(const_iterator, value_type&&)): Add overload for rvalues.

...but that's sooo ugly :).

Very well.  Committed the patch below.

Thanks for setting me straight.

commit 1a91de5beb16d130bed17f6eeb6b5ca6af454003 (HEAD -> trunk)
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Thu Oct 19 03:49:34 2017 -0400

     Update my last ChangeLog entry to properly specify overloaded 
functions.

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index a139a824d35..e43c53661e9 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -17,10 +17,10 @@

  2017-10-18  Aldy Hernandez  <aldyh@redhat.com>

-       * wide-int.cc (debug) [const wide_int &]: New.
-       (debug) [const wide_int *]: New.
-       (debug) [const widest_int &]: New.
-       (debug) [const widest_int *]: New.
+       * wide-int.cc (debug (const wide_int &)): New.
+       (debug (const wide_int *)): New.
+       (debug (const widest_int &)): New.
+       (debug (const widest_int *)): New.
Aldy Hernandez Oct. 19, 2017, 7:54 a.m. UTC | #5
On 10/18/2017 11:23 PM, Martin Sebor wrote:
> On 10/18/2017 11:08 AM, Aldy Hernandez wrote:
>> It looks like the generic debug() function for wide_int's is missing.
>> Instead, we have a wi->dump() method.  I have implemented debug() for
>> generic wide_int and for widest_int, which should cover the common
>> cases.  Anything else, can continue using the ->dump() method
>> templated methods.
> 
> Do you by any chance also plan to add support to the pretty printer
> for wide_int (and offset_int)?  It would obviate having to convert
> them to {s,u}hwi.

The pretty printer wasn't on my radar.  I am mostly concerned in 
enhancing the debugging experience right now.  Perhaps I should put that 
on my todo list...

Aldy
diff mbox series

Patch

gcc/

	* wide-int.cc (debug) [const wide_int &]: New.
	(debug) [const wide_int *]: New.
	(debug) [const widest_int &]: New.
	(debug) [const widest_int *]: New.

diff --git a/gcc/wide-int.cc b/gcc/wide-int.cc
index 71e24ec22af..1a1a68c1943 100644
--- a/gcc/wide-int.cc
+++ b/gcc/wide-int.cc
@@ -2146,6 +2146,39 @@  template void generic_wide_int <wide_int_ref_storage <true> >::dump () const;
 template void offset_int::dump () const;
 template void widest_int::dump () const;
 
+/* We could add all the above ::dump variants here, but wide_int and
+   widest_int should handle the common cases.  Besides, you can always
+   call the dump method directly.  */
+
+DEBUG_FUNCTION void
+debug (const wide_int &ref)
+{
+  ref.dump ();
+}
+
+DEBUG_FUNCTION void
+debug (const wide_int *ptr)
+{
+  if (ptr)
+    debug (*ptr);
+  else
+    fprintf (stderr, "<nil>\n");
+}
+
+DEBUG_FUNCTION void
+debug (const widest_int &ref)
+{
+  ref.dump ();
+}
+
+DEBUG_FUNCTION void
+debug (const widest_int *ptr)
+{
+  if (ptr)
+    debug (*ptr);
+  else
+    fprintf (stderr, "<nil>\n");
+}
 
 #if CHECKING_P