This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 1/2] New field la_varobj_ops in struct language_defn


> 2013-10-18  Yao Qi  <yao@codesourcery.com>
> 
> 	* language.h (struct lang_varobj_ops): Declare.
> 	(struct language_defn) <la_varobj_ops>: New field.
> 	* ada-lang.c: Include "varobj.h"
> 	(defn ada_language_defn): Initialize field 'la_varobj_ops' with
> 	ada_varobj_ops.
> 	* c-lang.c: Include "varobj.h"
> 	(c_language_defn): Initialize field 'la_varobj_ops' with
> 	c_varobj_ops.
> 	(cplus_language_defn): Initialize field 'la_varobj_ops' with
> 	cplus_varobj_ops.
> 	(asm_language_defn): Initialize field 'la_varobj_ops' with
> 	default_varobj_ops.
> 	(minimal_language_defn): Likewise.
> 	* d-lang.c (d_language_defn): Likewise.
> 	* f-lang.c (f_language_defn): Likewise.
> 	* go-lang.c (go_language_defn): Likewise.
> 	* m2-lang.c (m2_language_defn): Likewise.
> 	* objc-lang.c (objc_language_defn): Likewise.
> 	* opencl-lang.c (opencl_language_defn): Likewise.
> 	* p-lang.c (pascal_language_defn): Likewise.
> 	* language.c (unknown_language_defn): Likewise.
> 	(auto_language_defn): Likewise.
> 	(local_language_defn): Likewise.
> 	* jv-lang.c (java_language_defn): Initialize field
> 	'la_varobj_ops' with java_varobj_ops.
> 	* varobj.c (varobj_create): Update.
> 	* varobj.h (default_varobj_ops): Define macro.

The change looks mostly OK to me.

My only real comment is that I'd rather we define default_varobj_ops
without the '&' so that uses of that macro is similar to the case
where the macro isn't used. OK with that change.

Also, as a followup, I think it would be beneficial if we renamed
field "lang" in the varobj_root into "lang_ops". I think it's more
descriptive, especially since "lang" is used elsewhere with different
meanings (and types).

> --- a/gdb/varobj.h
> +++ b/gdb/varobj.h
> @@ -230,6 +230,7 @@ const struct lang_varobj_ops cplus_varobj_ops;
>  const struct lang_varobj_ops java_varobj_ops;
>  const struct lang_varobj_ops ada_varobj_ops;
>  
> +#define default_varobj_ops &c_varobj_ops
>  /* API functions */
>  
>  extern struct varobj *varobj_create (char *objname,
> -- 
> 1.7.7.6

-- 
Joel


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]