Add DISABLE_COPY_AND_ASSIGN
Commit Message
We have many classes that copy cotr and assignment operator are deleted,
so this patch adds a macro to do this, and replace these existing
mechanical code with macro DISABLE_COPY_AND_ASSIGN.
gdb:
2017-07-18 Yao Qi <yao.qi@linaro.org>
* common/common-defs.h (DISABLE_COPY_AND_ASSIGN): New macro.
* annotate.h (struct annotate_arg_emitter): Use
DISABLE_COPY_AND_ASSIGN.
* common/refcounted-object.h (refcounted_object): Likewise.
* completer.h (struct completion_result): Likewise.
* dwarf2read.c (struct dwarf2_per_objfile): Likewise.
* filename-seen-cache.h (filename_seen_cache): Likewise.
* gdbcore.h (thread_section_name): Likewise.
* gdb_regex.h (compiled_regex): Likewise.
* gdbthread.h (scoped_restore_current_thread): Likewise.
* inferior.h (scoped_restore_current_inferior): Likewise.
* jit.c (jit_reader): Likewise.
* linespec.h (struct linespec_result): Likewise.
* mi/mi-parse.h (struct mi_parse): Likewise.
* nat/fork-inferior.c (execv_argv): Likewise.
* progspace.h (scoped_restore_current_program_space): Likewise.
* python/python-internal.h (class gdbpy_enter): Likewise.
* regcache.h (regcache): Likewise.
* target-descriptions.c (struct tdesc_reg): Likewise.
(struct tdesc_type): Likewise.
(struct tdesc_feature): Likewise.
* ui-out.h (public:): Likewise.
---
gdb/annotate.h | 3 +--
gdb/common/common-defs.h | 4 ++++
gdb/common/refcounted-object.h | 6 +++---
gdb/completer.h | 8 ++------
gdb/dwarf2read.c | 4 +---
gdb/filename-seen-cache.h | 4 +---
gdb/gdb_regex.h | 4 +---
gdb/gdbcore.h | 4 +---
gdb/gdbthread.h | 6 +-----
gdb/inferior.h | 6 +-----
gdb/jit.c | 3 +--
gdb/linespec.h | 3 +--
gdb/mi/mi-parse.h | 3 +--
gdb/nat/fork-inferior.c | 4 +---
gdb/progspace.h | 6 +-----
gdb/python/python-internal.h | 3 +--
gdb/regcache.h | 3 +--
gdb/target-descriptions.c | 13 ++++---------
gdb/ui-out.h | 4 +---
19 files changed, 28 insertions(+), 63 deletions(-)
Comments
On 07/18/2017 02:48 PM, Yao Qi wrote:
> We have many classes that copy cotr and assignment operator are deleted,
> so this patch adds a macro to do this, and replace these existing
> mechanical code with macro DISABLE_COPY_AND_ASSIGN.
Yes, please. I've been meaning to add something like this for a while.
IMO, this could go in include/ansidecl.h, with a fallback version for
#if __cplusplus < C++11 that declares the methods without =delete (you
get a link error instead).
> /* Pull in gdb::unique_xmalloc_ptr. */
> #include "common/gdb_unique_ptr.h"
>
> +#define DISABLE_COPY_AND_ASSIGN(TYPE) \
> + TYPE (const TYPE&) = delete; \
> + void operator= (const TYPE &) = delete;
> +
Should this have an intro comment?
> --- a/gdb/common/refcounted-object.h
> +++ b/gdb/common/refcounted-object.h
> @@ -19,6 +19,8 @@
> #ifndef REFCOUNTED_OBJECT_H
> #define REFCOUNTED_OBJECT_H
>
> +#include "common-defs.h"
> +
This should not be necessary (and is against the guidelines [1]),
since .c files must include defs.h/common-defs.h first thing. Why did
you need it?
[1] https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#Include_Files
Thanks,
Pedro Alves
Pedro Alves <palves@redhat.com> writes:
> Yes, please. I've been meaning to add something like this for a while.
>
> IMO, this could go in include/ansidecl.h, with a fallback version for
> #if __cplusplus < C++11 that declares the methods without =delete (you
> get a link error instead).
>
I thought about adding this into include/, but can't find the right
file. I'll move the macro to include/ansidecl.h.
>> /* Pull in gdb::unique_xmalloc_ptr. */
>> #include "common/gdb_unique_ptr.h"
>>
>> +#define DISABLE_COPY_AND_ASSIGN(TYPE) \
>> + TYPE (const TYPE&) = delete; \
>> + void operator= (const TYPE &) = delete;
>> +
>
> Should this have an intro comment?
>
I thought it is too simple to have a comment :) How about this?
/* A macro to disable the copy constructor and assignment operator.
When building with C++ 11, explicitly delete these methods.
Otherwise, place this macro in the private: declarations of a class. */
#if __cplusplus >= 201103
#define DISABLE_COPY_AND_ASSIGN(TYPE) \
TYPE (const TYPE&) = delete; \
void operator= (const TYPE &) = delete;
#else
#define DISABLE_COPY_AND_ASSIGN(TYPE) \
TYPE (const TYPE&); \
void operator= (const TYPE &);
#endif /* __cplusplus >= 201103 */
>> --- a/gdb/common/refcounted-object.h
>> +++ b/gdb/common/refcounted-object.h
>> @@ -19,6 +19,8 @@
>> #ifndef REFCOUNTED_OBJECT_H
>> #define REFCOUNTED_OBJECT_H
>>
>> +#include "common-defs.h"
>> +
>
> This should not be necessary (and is against the guidelines [1]),
> since .c files must include defs.h/common-defs.h first thing. Why did
> you need it?
>
> [1]
> https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#Include_Files
OK, good to know this link. Fixed.
On 07/18/2017 04:13 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
>
>> Yes, please. I've been meaning to add something like this for a while.
>>
>> IMO, this could go in include/ansidecl.h, with a fallback version for
>> #if __cplusplus < C++11 that declares the methods without =delete (you
>> get a link error instead).
>>
>
> I thought about adding this into include/, but can't find the right
> file. I'll move the macro to include/ansidecl.h.
Thanks. Note that that file is maintained by gcc.
>
>>> /* Pull in gdb::unique_xmalloc_ptr. */
>>> #include "common/gdb_unique_ptr.h"
>>>
>>> +#define DISABLE_COPY_AND_ASSIGN(TYPE) \
>>> + TYPE (const TYPE&) = delete; \
>>> + void operator= (const TYPE &) = delete;
>>> +
>>
>> Should this have an intro comment?
>>
>
> I thought it is too simple to have a comment :) How about this?
>
> /* A macro to disable the copy constructor and assignment operator.
> When building with C++ 11, explicitly delete these methods.
> Otherwise, place this macro in the private: declarations of a class. */
How about this tweak:
/* A macro to disable the copy constructor and assignment operator.
When building with C++11 and above, the methods are explicitly
deleted, causing a compile-time error if something tries to copy.
For C++03, this just declares the methods, causing a link-time
error if the methods end up called (assuming you don't
define them). For C++03, for best results, place the macro
under the private: access specifier, so that most attempts at
copy are caught at compile-time. */
Thanks,
Pedro Alves
On 2017-07-18 05:38 PM, Pedro Alves wrote:
>
> On 07/18/2017 04:13 PM, Yao Qi wrote:
>> Pedro Alves <palves@redhat.com> writes:
>>
>>> Yes, please. I've been meaning to add something like this for a while.
>>>
>>> IMO, this could go in include/ansidecl.h, with a fallback version for
>>> #if __cplusplus < C++11 that declares the methods without =delete (you
>>> get a link error instead).
>>>
>>
>> I thought about adding this into include/, but can't find the right
>> file. I'll move the macro to include/ansidecl.h.
>
> Thanks. Note that that file is maintained by gcc.
>
>>
>>>> /* Pull in gdb::unique_xmalloc_ptr. */
>>>> #include "common/gdb_unique_ptr.h"
>>>>
>>>> +#define DISABLE_COPY_AND_ASSIGN(TYPE) \
>>>> + TYPE (const TYPE&) = delete; \
>>>> + void operator= (const TYPE &) = delete;
>>>> +
>>>
>>> Should this have an intro comment?
>>>
>>
>> I thought it is too simple to have a comment :) How about this?
>>
>> /* A macro to disable the copy constructor and assignment operator.
>> When building with C++ 11, explicitly delete these methods.
>> Otherwise, place this macro in the private: declarations of a class. */
>
> How about this tweak:
>
> /* A macro to disable the copy constructor and assignment operator.
> When building with C++11 and above, the methods are explicitly
> deleted, causing a compile-time error if something tries to copy.
> For C++03, this just declares the methods, causing a link-time
> error if the methods end up called (assuming you don't
> define them). For C++03, for best results, place the macro
> under the private: access specifier, so that most attempts at
> copy are caught at compile-time. */
>
> Thanks,
> Pedro Alves
>
Hi Yao,
Out of curiosity, did you make any progress on this?
Thanks,
Simon
Simon Marchi <simon.marchi@ericsson.com> writes:
> Out of curiosity, did you make any progress on this?
I pinged Ian in GNU Cauldron, and he is "basically OK" with the patch.
https://gcc.gnu.org/ml/gcc-patches/2017-09/msg00532.html
I need to address his comments and commit it.
>
> gdb:
>
> 2017-07-18 Yao Qi <yao.qi@linaro.org>
>
> * common/common-defs.h (DISABLE_COPY_AND_ASSIGN): New macro.
> * annotate.h (struct annotate_arg_emitter): Use
> DISABLE_COPY_AND_ASSIGN.
> * common/refcounted-object.h (refcounted_object): Likewise.
> * completer.h (struct completion_result): Likewise.
> * dwarf2read.c (struct dwarf2_per_objfile): Likewise.
> * filename-seen-cache.h (filename_seen_cache): Likewise.
> * gdbcore.h (thread_section_name): Likewise.
> * gdb_regex.h (compiled_regex): Likewise.
> * gdbthread.h (scoped_restore_current_thread): Likewise.
> * inferior.h (scoped_restore_current_inferior): Likewise.
> * jit.c (jit_reader): Likewise.
> * linespec.h (struct linespec_result): Likewise.
> * mi/mi-parse.h (struct mi_parse): Likewise.
> * nat/fork-inferior.c (execv_argv): Likewise.
> * progspace.h (scoped_restore_current_program_space): Likewise.
> * python/python-internal.h (class gdbpy_enter): Likewise.
> * regcache.h (regcache): Likewise.
> * target-descriptions.c (struct tdesc_reg): Likewise.
> (struct tdesc_type): Likewise.
> (struct tdesc_feature): Likewise.
> * ui-out.h (public:): Likewise.
DISABLE_COPY_AND_ASSIGN was committed
to include/ansidecl.h, so I pushed this patch in.
@@ -81,8 +81,7 @@ struct annotate_arg_emitter
annotate_arg_emitter () { annotate_arg_begin (); }
~annotate_arg_emitter () { annotate_arg_end (); }
- annotate_arg_emitter (const annotate_arg_emitter &) = delete;
- annotate_arg_emitter &operator= (const annotate_arg_emitter &) = delete;
+ DISABLE_COPY_AND_ASSIGN (annotate_arg_emitter);
};
extern void annotate_source (char *, int, int, int,
@@ -91,4 +91,8 @@
/* Pull in gdb::unique_xmalloc_ptr. */
#include "common/gdb_unique_ptr.h"
+#define DISABLE_COPY_AND_ASSIGN(TYPE) \
+ TYPE (const TYPE&) = delete; \
+ void operator= (const TYPE &) = delete;
+
#endif /* COMMON_DEFS_H */
@@ -19,6 +19,8 @@
#ifndef REFCOUNTED_OBJECT_H
#define REFCOUNTED_OBJECT_H
+#include "common-defs.h"
+
/* Base class of intrusively reference-countable objects.
Incrementing and decrementing the reference count is an external
responsibility. */
@@ -45,9 +47,7 @@ public:
int refcount () const { return m_refcount; }
private:
- /* Disable copy. */
- refcounted_object (const refcounted_object &) = delete;
- refcounted_object &operator=(const refcounted_object &) = delete;
+ DISABLE_COPY_AND_ASSIGN (refcounted_object);
/* The reference count. */
int m_refcount = 0;
@@ -85,9 +85,7 @@ struct completion_result
/* Destroy a result. */
~completion_result ();
- /* Disable copying, since we don't need it. */
- completion_result (const completion_result &rhs) = delete;
- void operator= (const completion_result &rhs) = delete;
+ DISABLE_COPY_AND_ASSIGN (completion_result);
/* Move a result. */
completion_result (completion_result &&rhs);
@@ -146,9 +144,7 @@ public:
completion_tracker ();
~completion_tracker ();
- /* Disable copy. */
- completion_tracker (const completion_tracker &rhs) = delete;
- void operator= (const completion_tracker &rhs) = delete;
+ DISABLE_COPY_AND_ASSIGN (completion_tracker);
/* Add the completion NAME to the list of generated completions if
it is not there already. If too many completions were already
@@ -232,9 +232,7 @@ struct dwarf2_per_objfile
~dwarf2_per_objfile ();
- /* Disable copy. */
- dwarf2_per_objfile (const dwarf2_per_objfile &) = delete;
- void operator= (const dwarf2_per_objfile &) = delete;
+ DISABLE_COPY_AND_ASSIGN (dwarf2_per_objfile);
/* Free all cached compilation units. */
void free_cached_comp_units ();
@@ -28,9 +28,7 @@ public:
filename_seen_cache ();
~filename_seen_cache ();
- /* Disable copy. */
- filename_seen_cache (const filename_seen_cache &) = delete;
- void operator= (const filename_seen_cache &) = delete;
+ DISABLE_COPY_AND_ASSIGN (filename_seen_cache);
/* Empty the cache, but do not delete it. */
void clear ();
@@ -43,9 +43,7 @@ public:
~compiled_regex ();
- /* Disable copy. */
- compiled_regex (const compiled_regex&) = delete;
- void operator= (const compiled_regex&) = delete;
+ DISABLE_COPY_AND_ASSIGN (compiled_regex);
/* Wrapper around ::regexec. */
int exec (const char *string,
@@ -258,9 +258,7 @@ public:
const char *c_str () const
{ return m_section_name; }
- /* Disable copy. */
- thread_section_name (const thread_section_name &) = delete;
- void operator= (const thread_section_name &) = delete;
+ DISABLE_COPY_AND_ASSIGN (thread_section_name);
private:
/* Either a pointer into M_STORAGE, or a pointer to the name passed
@@ -597,11 +597,7 @@ public:
scoped_restore_current_thread ();
~scoped_restore_current_thread ();
- /* Disable copy. */
- scoped_restore_current_thread
- (const scoped_restore_current_thread &) = delete;
- void operator=
- (const scoped_restore_current_thread &) = delete;
+ DISABLE_COPY_AND_ASSIGN (scoped_restore_current_thread);
private:
thread_info *m_thread;
@@ -532,11 +532,7 @@ public:
~scoped_restore_current_inferior ()
{ set_current_inferior (m_saved_inf); }
- /* Disable copy. */
- scoped_restore_current_inferior
- (const scoped_restore_current_inferior &) = delete;
- void operator=
- (const scoped_restore_current_inferior &) = delete;
+ DISABLE_COPY_AND_ASSIGN (scoped_restore_current_inferior);
private:
inferior *m_saved_inf;
@@ -163,8 +163,7 @@ struct jit_reader
functions->destroy (functions);
}
- jit_reader (const jit_reader &) = delete;
- jit_reader &operator= (const jit_reader &) = delete;
+ DISABLE_COPY_AND_ASSIGN (jit_reader);
struct gdb_reader_funcs *functions;
gdb_dlhandle_up handle;
@@ -68,8 +68,7 @@ struct linespec_result
~linespec_result ();
- linespec_result (const linespec_result &) = delete;
- linespec_result &operator= (const linespec_result &) = delete;
+ DISABLE_COPY_AND_ASSIGN (linespec_result);
/* If non-zero, the linespec should be displayed to the user. This
is used by "unusual" linespecs where the ordinary `info break'
@@ -44,8 +44,7 @@ struct mi_parse
mi_parse ();
~mi_parse ();
- mi_parse (const mi_parse &) = delete;
- mi_parse &operator= (const mi_parse &) = delete;
+ DISABLE_COPY_AND_ASSIGN (mi_parse);
enum mi_command_type op;
char *command;
@@ -56,9 +56,7 @@ public:
}
private:
- /* Disable copying. */
- execv_argv (const execv_argv &) = delete;
- void operator= (const execv_argv &) = delete;
+ DISABLE_COPY_AND_ASSIGN (execv_argv);
/* Helper methods for constructing the argument vector. */
@@ -273,11 +273,7 @@ public:
~scoped_restore_current_program_space ()
{ set_current_program_space (m_saved_pspace); }
- /* Disable copy. */
- scoped_restore_current_program_space
- (const scoped_restore_current_program_space &) = delete;
- void operator=
- (const scoped_restore_current_program_space &) = delete;
+ DISABLE_COPY_AND_ASSIGN (scoped_restore_current_program_space);
private:
program_space *m_saved_pspace;
@@ -650,8 +650,7 @@ class gdbpy_enter
~gdbpy_enter ();
- gdbpy_enter (const gdbpy_enter &) = delete;
- gdbpy_enter &operator= (const gdbpy_enter &) = delete;
+ DISABLE_COPY_AND_ASSIGN (gdbpy_enter);
private:
@@ -255,8 +255,7 @@ public:
/* Create a readonly regcache from a non-readonly regcache. */
regcache (readonly_t, const regcache &src);
- regcache (const regcache &) = delete;
- void operator= (const regcache &) = delete;
+ DISABLE_COPY_AND_ASSIGN (regcache);
/* class regcache is only extended in unit test, so only mark it
virtual when selftest is enabled. */
@@ -69,9 +69,7 @@ typedef struct tdesc_reg
xfree (group);
}
- /* Disable copying. */
- tdesc_reg (const tdesc_reg &) = delete;
- tdesc_reg &operator= (const tdesc_reg &) = delete;
+ DISABLE_COPY_AND_ASSIGN (tdesc_reg);
/* The name of this register. In standard features, it may be
recognized by the architecture support code, or it may be purely
@@ -185,9 +183,8 @@ typedef struct tdesc_type
}
xfree ((char *) name);
}
- /* Disable copying. */
- tdesc_type (const tdesc_type &) = delete;
- tdesc_type &operator= (const tdesc_type &) = delete;
+
+ DISABLE_COPY_AND_ASSIGN (tdesc_type);
/* The name of this type. If this type is a built-in type, this is
a pointer to a constant string. Otherwise, it's a
@@ -243,9 +240,7 @@ typedef struct tdesc_feature
xfree (name);
}
- /* Disable copying. */
- tdesc_feature (const tdesc_feature &) = delete;
- tdesc_feature &operator= (const tdesc_feature &) = delete;
+ DISABLE_COPY_AND_ASSIGN (tdesc_feature);
/* The name of this feature. It may be recognized by the architecture
support code. */
@@ -208,9 +208,7 @@ public:
m_uiout->end (Type);
}
- ui_out_emit_type (const ui_out_emit_type<Type> &) = delete;
- ui_out_emit_type<Type> &operator= (const ui_out_emit_type<Type> &)
- = delete;
+ DISABLE_COPY_AND_ASSIGN (ui_out_emit_type<Type>);
private: