[v3,02/11,PR,gdb/14441] gdb: gdbtypes: change {lookup,make}_reference_type() API
Commit Message
Parameterize lookup_reference_type() and make_reference_type() by the kind of
reference type we want to look up. Create two wrapper functions
lookup_{lvalue,rvalue}_reference_type() for lookup_reference_type() to simplify
the API. Change all callers to use the new API.
gdb/Changelog:
2016-03-04 Artemiy Volkov <artemiyv@acm.org>
* dwarf2read.c (read_tag_reference_type): Use
lookup_lvalue_reference_type() instead of lookup_reference_type().
* eval.c (evaluate_subexp_standard): Likewise.
* f-exp.y: Likewise.
* gdbtypes.c (make_reference_type, lookup_reference_type):
Generalize with rvalue reference types.
(lookup_lvalue_reference_type, lookup_rvalue_reference_type): New
convenience wrappers for lookup_reference_type().
* gdbtypes.h (make_reference_type, lookup_reference_type): Add a
reference kind parameter.
(lookup_lvalue_reference_type, lookup_rvalue_reference_type): Add
wrappers for lookup_reference_type().
* guile/scm-type.c (gdbscm_type_reference): Use
lookup_lvalue_reference_type() instead of lookup_reference_type().
* guile/scm-value.c (gdbscm_value_dynamic_type): Likewise.
* parse.c (follow_types): Likewise.
* python/py-type.c (typy_reference, typy_lookup_type): Likewise.
* python/py-value.c (valpy_get_dynamic_type, valpy_getitem):
Likewise.
* python/py-xmethods.c (gdbpy_get_xmethod_result_type)
(gdbpy_invoke_xmethod): Likewise.
* stabsread.c: Provide extra argument to make_reference_type()
call.
* valops.c (value_ref, value_rtti_indirect_type): Use
lookup_lvalue_reference_type() instead of lookup_reference_type().
---
gdb/dwarf2read.c | 2 +-
gdb/eval.c | 2 +-
gdb/f-exp.y | 2 +-
gdb/gdbtypes.c | 43 ++++++++++++++++++++++++++++++++++---------
gdb/gdbtypes.h | 8 ++++++--
gdb/guile/scm-type.c | 2 +-
gdb/guile/scm-value.c | 2 +-
gdb/parse.c | 2 +-
gdb/python/py-type.c | 4 ++--
gdb/python/py-value.c | 5 +++--
gdb/python/py-xmethods.c | 4 ++--
gdb/stabsread.c | 3 ++-
gdb/valops.c | 4 ++--
13 files changed, 57 insertions(+), 26 deletions(-)
Comments
On 03/04/2016 07:19 PM, Artemiy Volkov wrote:
> Parameterize lookup_reference_type() and make_reference_type() by the kind of
> reference type we want to look up. Create two wrapper functions
> lookup_{lvalue,rvalue}_reference_type() for lookup_reference_type() to simplify
> the API. Change all callers to use the new API.
Just an FYI: When the time comes to commit this, GIT will require a
single-line description/summary of the patch, followed by a blank line.
> diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
> index f129b0e..a99e878 100644
> --- a/gdb/gdbtypes.c
> +++ b/gdb/gdbtypes.c
> @@ -384,15 +384,21 @@ lookup_pointer_type (struct type *type)
> /* Lookup a C++ `reference' to a type TYPE. TYPEPTR, if nonzero,
> points to a pointer to memory where the reference type should be
> stored. If *TYPEPTR is zero, update it to point to the reference
> - type we return. We allocate new memory if needed. */
> + type we return. We allocate new memory if needed. REFCODE denotes
Need two spaces after "if needed."
> + the kind of reference type to lookup (lvalue or rvalue reference). */
>
> struct type *
> -make_reference_type (struct type *type, struct type **typeptr)
> +make_reference_type (struct type *type, struct type **typeptr,
> + enum type_code refcode)
> {
> struct type *ntype; /* New type */
> + struct type **reftype;
> struct type *chain;
>
> - ntype = TYPE_REFERENCE_TYPE (type);
> + gdb_assert (refcode == TYPE_CODE_REF || refcode == TYPE_CODE_RVALUE_REF);
> +
> + ntype = (refcode == TYPE_CODE_REF ? TYPE_REFERENCE_TYPE (type)
> + : TYPE_RVALUE_REFERENCE_TYPE (type));
>
> if (ntype)
> {
> @@ -421,7 +427,11 @@ make_reference_type (struct type *type, struct type **typeptr)
> }
>
> TYPE_TARGET_TYPE (ntype) = type;
> - TYPE_REFERENCE_TYPE (type) = ntype;
> + reftype = (refcode == TYPE_CODE_REF ? &TYPE_REFERENCE_TYPE (type)
> + : &TYPE_RVALUE_REFERENCE_TYPE (type));
> +
> + if(*reftype != NULL)
> + *reftype = ntype;
>
> /* FIXME! Assume the machine has only one representation for
> references, and that it matches the (only) representation for
I would prefer to do:
if (refcode == TYPE_CODE_REF)
TYPE_REFERENCE_TYPE (type) = ntype;
else
TYPE_RVALUE_REFERENCE_TYPE (type) = ntype;
It's cleaner/smaller/easier to read (at least for me). [Otherwise, a
space is missing between "if" and "(".] See next comment, though.
> @@ -429,10 +439,9 @@ make_reference_type (struct type *type, struct type **typeptr)
>
> TYPE_LENGTH (ntype) =
> gdbarch_ptr_bit (get_type_arch (type)) / TARGET_CHAR_BIT;
> - TYPE_CODE (ntype) = TYPE_CODE_REF;
> + TYPE_CODE (ntype) = refcode;
>
> - if (!TYPE_REFERENCE_TYPE (type)) /* Remember it, if don't have one. */
> - TYPE_REFERENCE_TYPE (type) = ntype;
> + *reftype = ntype;
>
This is slightly different from the original, which only set the new
type if it was unset in the type. Your revised version will
unconditionally do it every time. I have no idea if it makes a
difference or not. Do you?
> /* Update the length of all the other variants of this type. */
> chain = TYPE_CHAIN (ntype);
Keith
On Wed, Mar 16, 2016 at 03:19:13PM -0700, Keith Seitz wrote:
[snip]
> > + the kind of reference type to lookup (lvalue or rvalue reference). */
> >
> > struct type *
> > -make_reference_type (struct type *type, struct type **typeptr)
> > +make_reference_type (struct type *type, struct type **typeptr,
> > + enum type_code refcode)
> > {
> > struct type *ntype; /* New type */
> > + struct type **reftype;
> > struct type *chain;
> >
> > - ntype = TYPE_REFERENCE_TYPE (type);
> > + gdb_assert (refcode == TYPE_CODE_REF || refcode == TYPE_CODE_RVALUE_REF);
> > +
> > + ntype = (refcode == TYPE_CODE_REF ? TYPE_REFERENCE_TYPE (type)
> > + : TYPE_RVALUE_REFERENCE_TYPE (type));
> >
> > if (ntype)
> > {
> > @@ -421,7 +427,11 @@ make_reference_type (struct type *type, struct type **typeptr)
> > }
> >
> > TYPE_TARGET_TYPE (ntype) = type;
> > - TYPE_REFERENCE_TYPE (type) = ntype;
> > + reftype = (refcode == TYPE_CODE_REF ? &TYPE_REFERENCE_TYPE (type)
> > + : &TYPE_RVALUE_REFERENCE_TYPE (type));
> > +
> > + if(*reftype != NULL)
> > + *reftype = ntype;
> >
> > /* FIXME! Assume the machine has only one representation for
> > references, and that it matches the (only) representation for
>
> I would prefer to do:
>
> if (refcode == TYPE_CODE_REF)
> TYPE_REFERENCE_TYPE (type) = ntype;
> else
> TYPE_RVALUE_REFERENCE_TYPE (type) = ntype;
>
> It's cleaner/smaller/easier to read (at least for me). [Otherwise, a
> space is missing between "if" and "(".] See next comment, though.
My idea was to create a new variable 'reftype' and use it as a
pointer to the member of struct main_type that we're dealing with here.
Without it, we would have to paste your fragment (more readable,
granted) two times rather than just do "*reftype = ntype" two times. I
think the latter is better since it's several lines shorter in total and
reduces the amount of copy-paste code.
>
>
> > @@ -429,10 +439,9 @@ make_reference_type (struct type *type, struct type **typeptr)
> >
> > TYPE_LENGTH (ntype) =
> > gdbarch_ptr_bit (get_type_arch (type)) / TARGET_CHAR_BIT;
> > - TYPE_CODE (ntype) = TYPE_CODE_REF;
> > + TYPE_CODE (ntype) = refcode;
> >
> > - if (!TYPE_REFERENCE_TYPE (type)) /* Remember it, if don't have one. */
> > - TYPE_REFERENCE_TYPE (type) = ntype;
> > + *reftype = ntype;
> >
>
> This is slightly different from the original, which only set the new
> type if it was unset in the type. Your revised version will
> unconditionally do it every time. I have no idea if it makes a
> difference or not. Do you?
AFAICT *reftype is provably unequal to 0 here, because it was set to
ntype by the previous assignment, and ntype was unequal to 0 itself. I
went ahead and simplified this a little bit. Is this kind of code
change justified here?
>
> > /* Update the length of all the other variants of this type. */
> > chain = TYPE_CHAIN (ntype);
Thanks!
Artemiy
@@ -14337,7 +14337,7 @@ read_tag_reference_type (struct die_info *die, struct dwarf2_cu *cu)
if (type)
return type;
- type = lookup_reference_type (target_type);
+ type = lookup_lvalue_reference_type (target_type);
attr = dwarf2_attr (die, DW_AT_byte_size, cu);
if (attr)
{
@@ -2789,7 +2789,7 @@ evaluate_subexp_standard (struct type *expect_type,
if (TYPE_CODE (check_typedef (type)) != TYPE_CODE_REF)
{
- type = lookup_reference_type (type);
+ type = lookup_lvalue_reference_type (type);
result = allocate_value (type);
}
}
@@ -567,7 +567,7 @@ ptype : typebase
follow_type = lookup_pointer_type (follow_type);
break;
case tp_reference:
- follow_type = lookup_reference_type (follow_type);
+ follow_type = lookup_lvalue_reference_type (follow_type);
break;
case tp_array:
array_size = pop_type_int ();
@@ -384,15 +384,21 @@ lookup_pointer_type (struct type *type)
/* Lookup a C++ `reference' to a type TYPE. TYPEPTR, if nonzero,
points to a pointer to memory where the reference type should be
stored. If *TYPEPTR is zero, update it to point to the reference
- type we return. We allocate new memory if needed. */
+ type we return. We allocate new memory if needed. REFCODE denotes
+ the kind of reference type to lookup (lvalue or rvalue reference). */
struct type *
-make_reference_type (struct type *type, struct type **typeptr)
+make_reference_type (struct type *type, struct type **typeptr,
+ enum type_code refcode)
{
struct type *ntype; /* New type */
+ struct type **reftype;
struct type *chain;
- ntype = TYPE_REFERENCE_TYPE (type);
+ gdb_assert (refcode == TYPE_CODE_REF || refcode == TYPE_CODE_RVALUE_REF);
+
+ ntype = (refcode == TYPE_CODE_REF ? TYPE_REFERENCE_TYPE (type)
+ : TYPE_RVALUE_REFERENCE_TYPE (type));
if (ntype)
{
@@ -421,7 +427,11 @@ make_reference_type (struct type *type, struct type **typeptr)
}
TYPE_TARGET_TYPE (ntype) = type;
- TYPE_REFERENCE_TYPE (type) = ntype;
+ reftype = (refcode == TYPE_CODE_REF ? &TYPE_REFERENCE_TYPE (type)
+ : &TYPE_RVALUE_REFERENCE_TYPE (type));
+
+ if(*reftype != NULL)
+ *reftype = ntype;
/* FIXME! Assume the machine has only one representation for
references, and that it matches the (only) representation for
@@ -429,10 +439,9 @@ make_reference_type (struct type *type, struct type **typeptr)
TYPE_LENGTH (ntype) =
gdbarch_ptr_bit (get_type_arch (type)) / TARGET_CHAR_BIT;
- TYPE_CODE (ntype) = TYPE_CODE_REF;
+ TYPE_CODE (ntype) = refcode;
- if (!TYPE_REFERENCE_TYPE (type)) /* Remember it, if don't have one. */
- TYPE_REFERENCE_TYPE (type) = ntype;
+ *reftype = ntype;
/* Update the length of all the other variants of this type. */
chain = TYPE_CHAIN (ntype);
@@ -449,9 +458,25 @@ make_reference_type (struct type *type, struct type **typeptr)
details. */
struct type *
-lookup_reference_type (struct type *type)
+lookup_reference_type (struct type *type, enum type_code refcode)
+{
+ return make_reference_type (type, (struct type **) 0, refcode);
+}
+
+/* Lookup the lvalue reference type for the type TYPE. */
+
+struct type *
+lookup_lvalue_reference_type (struct type *type)
+{
+ return lookup_reference_type (type, TYPE_CODE_REF);
+}
+
+/* Lookup the rvalue reference type for the type TYPE. */
+
+struct type *
+lookup_rvalue_reference_type (struct type *type)
{
- return make_reference_type (type, (struct type **) 0);
+ return lookup_reference_type (type, TYPE_CODE_RVALUE_REF);
}
/* Lookup a function type that returns type TYPE. TYPEPTR, if
@@ -1724,9 +1724,13 @@ extern void append_flags_type_flag (struct type *type, int bitpos, char *name);
extern void make_vector_type (struct type *array_type);
extern struct type *init_vector_type (struct type *elt_type, int n);
-extern struct type *lookup_reference_type (struct type *);
+extern struct type *lookup_reference_type (struct type *, enum type_code);
+extern struct type *lookup_lvalue_reference_type (struct type *);
+extern struct type *lookup_rvalue_reference_type (struct type *);
-extern struct type *make_reference_type (struct type *, struct type **);
+
+extern struct type *make_reference_type (struct type *, struct type **,
+ enum type_code);
extern struct type *make_cv_type (int, int, struct type *, struct type **);
@@ -854,7 +854,7 @@ gdbscm_type_reference (SCM self)
TRY
{
- type = lookup_reference_type (type);
+ type = lookup_lvalue_reference_type (type);
}
CATCH (except, RETURN_MASK_ALL)
{
@@ -601,7 +601,7 @@ gdbscm_value_dynamic_type (SCM self)
if (was_pointer)
type = lookup_pointer_type (type);
else
- type = lookup_reference_type (type);
+ type = lookup_lvalue_reference_type (type);
}
}
else if (TYPE_CODE (type) == TYPE_CODE_STRUCT)
@@ -1695,7 +1695,7 @@ follow_types (struct type *follow_type)
make_addr_space = 0;
break;
case tp_reference:
- follow_type = lookup_reference_type (follow_type);
+ follow_type = lookup_lvalue_reference_type (follow_type);
if (make_const)
follow_type = make_cv_type (make_const,
TYPE_VOLATILE (follow_type),
@@ -667,7 +667,7 @@ typy_reference (PyObject *self, PyObject *args)
TRY
{
- type = lookup_reference_type (type);
+ type = lookup_lvalue_reference_type (type);
}
CATCH (except, RETURN_MASK_ALL)
{
@@ -827,7 +827,7 @@ typy_lookup_type (struct demangle_component *demangled,
switch (demangled_type)
{
case DEMANGLE_COMPONENT_REFERENCE:
- rtype = lookup_reference_type (type);
+ rtype = lookup_lvalue_reference_type (type);
break;
case DEMANGLE_COMPONENT_POINTER:
rtype = lookup_pointer_type (type);
@@ -376,7 +376,7 @@ valpy_get_dynamic_type (PyObject *self, void *closure)
if (was_pointer)
type = lookup_pointer_type (type);
else
- type = lookup_reference_type (type);
+ type = lookup_lvalue_reference_type (type);
}
}
else if (TYPE_CODE (type) == TYPE_CODE_STRUCT)
@@ -766,7 +766,8 @@ valpy_getitem (PyObject *self, PyObject *key)
if (TYPE_CODE (val_type) == TYPE_CODE_PTR)
res_val = value_cast (lookup_pointer_type (base_class_type), tmp);
else if (TYPE_CODE (val_type) == TYPE_CODE_REF)
- res_val = value_cast (lookup_reference_type (base_class_type), tmp);
+ res_val = value_cast (lookup_lvalue_reference_type (base_class_type),
+ tmp);
else
res_val = value_cast (base_class_type, tmp);
}
@@ -550,7 +550,7 @@ gdbpy_get_xmethod_result_type (const struct extension_language_defn *extlang,
}
else if (TYPE_CODE (obj_type) == TYPE_CODE_REF)
{
- struct type *this_ref = lookup_reference_type (this_type);
+ struct type *this_ref = lookup_lvalue_reference_type (this_type);
if (!types_equal (obj_type, this_ref))
obj = value_cast (this_ref, obj);
@@ -636,7 +636,7 @@ gdbpy_invoke_xmethod (const struct extension_language_defn *extlang,
}
else if (TYPE_CODE (obj_type) == TYPE_CODE_REF)
{
- struct type *this_ref = lookup_reference_type (this_type);
+ struct type *this_ref = lookup_lvalue_reference_type (this_type);
if (!types_equal (obj_type, this_ref))
obj = value_cast (this_ref, obj);
@@ -1772,7 +1772,8 @@ again:
case '&': /* Reference to another type */
type1 = read_type (pp, objfile);
- type = make_reference_type (type1, dbx_lookup_type (typenums, objfile));
+ type = make_reference_type (type1, dbx_lookup_type (typenums, objfile),
+ TYPE_CODE_REF);
break;
case 'f': /* Function returning another type */
@@ -1508,7 +1508,7 @@ value_ref (struct value *arg1)
return arg1;
arg2 = value_addr (arg1);
- deprecated_set_value_type (arg2, lookup_reference_type (type));
+ deprecated_set_value_type (arg2, lookup_lvalue_reference_type (type));
return arg2;
}
@@ -3616,7 +3616,7 @@ value_rtti_indirect_type (struct value *v, int *full,
real_type = make_cv_type (TYPE_CONST (target_type),
TYPE_VOLATILE (target_type), real_type, NULL);
if (TYPE_CODE (type) == TYPE_CODE_REF)
- real_type = lookup_reference_type (real_type);
+ real_type = lookup_lvalue_reference_type (real_type);
else if (TYPE_CODE (type) == TYPE_CODE_PTR)
real_type = lookup_pointer_type (real_type);
else