[06/49] timevar.h: add auto_client_timevar class

Message ID 1573867416-55618-7-git-send-email-dmalcolm@redhat.com
State New
Headers show
Series
  • RFC: Add a static analysis framework to GCC
Related show

Commit Message

David Malcolm Nov. 16, 2019, 1:22 a.m.
This patch adds a class "auto_client_timevar", similar to auto_timevar,
but for plugins to use on their own timing items that aren't in
timevar.def

gcc/ChangeLog:
	* timevar.h (class auto_client_timevar): New class.
---
 gcc/timevar.h | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

-- 
1.8.5.3

Comments

Martin Sebor Dec. 4, 2019, 4:39 p.m. | #1
On 11/15/19 6:22 PM, David Malcolm wrote:
> This patch adds a class "auto_client_timevar", similar to auto_timevar,

> but for plugins to use on their own timing items that aren't in

> timevar.def

> 

> gcc/ChangeLog:

> 	* timevar.h (class auto_client_timevar): New class.

> ---

>   gcc/timevar.h | 33 +++++++++++++++++++++++++++++++++

>   1 file changed, 33 insertions(+)

> 

> diff --git a/gcc/timevar.h b/gcc/timevar.h

> index ef404d0..d053ab7 100644

> --- a/gcc/timevar.h

> +++ b/gcc/timevar.h

> @@ -256,6 +256,39 @@ class auto_timevar

>     timevar_id_t m_tv;

>   };

>   

> +/* An RAII class analogous to auto_timevar, but for a client item,

> +   for use by plugins.  */

> +

> +class auto_client_timevar

> +{

> + public:

> +  auto_client_timevar (timer *t, const char *item_name)

> +    : m_timer (t)

> +  {

> +    if (m_timer)

> +      m_timer->push_client_item (item_name);

> +  }

> +

> +  explicit auto_client_timevar (const char *item_name)

> +    : m_timer (g_timer)

> +  {

> +    if (m_timer)

> +      m_timer->push_client_item (item_name);

> +  }

> +

> +  ~auto_client_timevar ()

> +  {

> +    if (m_timer)

> +      m_timer->pop_client_item ();

> +  }

> +

> + private:

> +  // Private to disallow copies.

> +  auto_client_timevar (const auto_client_timevar &);


I don't know why it's important to disallow making copies of
these classes (they look safe to copy) but usually it goes
along with assignment so I would suggest adding a comment
with a rationale and (perhaps also) disabling assignment.

Martin

PS I see auto_timevar is also assignable but not copyable.
It's a common mistake to make to forget about one or the other
(or both) so if it's intentional it helps to document it.

> +

> +  timer *m_timer;

> +};

> +

>   extern void print_time (const char *, long);

>   

>   #endif /* ! GCC_TIMEVAR_H */

>
Tom Tromey Dec. 4, 2019, 5:27 p.m. | #2
>>>>> "Martin" == Martin Sebor <msebor@gmail.com> writes:


>> + private:

>> +  // Private to disallow copies.

>> +  auto_client_timevar (const auto_client_timevar &);


Martin> I don't know why it's important to disallow making copies of
Martin> these classes (they look safe to copy) but usually it goes
Martin> along with assignment so I would suggest adding a comment
Martin> with a rationale and (perhaps also) disabling assignment.

ansidecl.h has a DISABLE_COPY_AND_ASSIGN macro you can use here.
gdb uses this quite a bit.

Tom
David Malcolm Dec. 4, 2019, 9:15 p.m. | #3
On Wed, 2019-12-04 at 10:27 -0700, Tom Tromey wrote:
> > > > > > "Martin" == Martin Sebor <msebor@gmail.com> writes:

> > > + private:

> > > +  // Private to disallow copies.

> > > +  auto_client_timevar (const auto_client_timevar &);

> 

> Martin> I don't know why it's important to disallow making copies of

> Martin> these classes (they look safe to copy) but usually it goes

> Martin> along with assignment so I would suggest adding a comment

> Martin> with a rationale and (perhaps also) disabling assignment.

> 

> ansidecl.h has a DISABLE_COPY_AND_ASSIGN macro you can use here.

> gdb uses this quite a bit.

> 

> Tom


Thanks Martin and Tom; I'm currently adding DISABLE_COPY_AND_ASSIGN in
various places.

Dave
Jeff Law Dec. 7, 2019, 2:29 p.m. | #4
On Fri, 2019-11-15 at 20:22 -0500, David Malcolm wrote:
> This patch adds a class "auto_client_timevar", similar to

> auto_timevar,

> but for plugins to use on their own timing items that aren't in

> timevar.def

> 

> gcc/ChangeLog:

> 	* timevar.h (class auto_client_timevar): New class.

So if we move away from a plug-in architecture to an integrated first
class analyzer, do we still want this change?

jeff
>
David Malcolm Dec. 8, 2019, 2:30 p.m. | #5
On Sat, 2019-12-07 at 07:29 -0700, Jeff Law wrote:
> On Fri, 2019-11-15 at 20:22 -0500, David Malcolm wrote:

> > This patch adds a class "auto_client_timevar", similar to

> > auto_timevar,

> > but for plugins to use on their own timing items that aren't in

> > timevar.def

> > 

> > gcc/ChangeLog:

> > 	* timevar.h (class auto_client_timevar): New class.

> So if we move away from a plug-in architecture to an integrated first

> class analyzer, do we still want this change?


I've dropped this change in the v3 of the kit, in favor of new TV_*
values; see:
  https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00529.html

Dave

Patch

diff --git a/gcc/timevar.h b/gcc/timevar.h
index ef404d0..d053ab7 100644
--- a/gcc/timevar.h
+++ b/gcc/timevar.h
@@ -256,6 +256,39 @@  class auto_timevar
   timevar_id_t m_tv;
 };
 
+/* An RAII class analogous to auto_timevar, but for a client item,
+   for use by plugins.  */
+
+class auto_client_timevar
+{
+ public:
+  auto_client_timevar (timer *t, const char *item_name)
+    : m_timer (t)
+  {
+    if (m_timer)
+      m_timer->push_client_item (item_name);
+  }
+
+  explicit auto_client_timevar (const char *item_name)
+    : m_timer (g_timer)
+  {
+    if (m_timer)
+      m_timer->push_client_item (item_name);
+  }
+
+  ~auto_client_timevar ()
+  {
+    if (m_timer)
+      m_timer->pop_client_item ();
+  }
+
+ private:
+  // Private to disallow copies.
+  auto_client_timevar (const auto_client_timevar &);
+
+  timer *m_timer;
+};
+
 extern void print_time (const char *, long);
 
 #endif /* ! GCC_TIMEVAR_H */