diff mbox series

[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 | expand

Commit Message

David Malcolm Nov. 16, 2019, 1:22 a.m. UTC 
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(+)

Comments

Martin Sebor Dec. 4, 2019, 4:39 p.m. UTC | #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. UTC | #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. UTC | #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. UTC | #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. UTC | #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
diff mbox series

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 */