On 6/5/19 8:34 PM, Richard Pospesel wrote:
Signed-off-by: Richard Pospesel richard@torproject.org
tools/widl/parser.y | 2 +- tools/widl/typetree.c | 22 +++++++++++++++------- tools/widl/typetree.h | 4 ++-- tools/widl/widltypes.h | 2 +- 4 files changed, 19 insertions(+), 11 deletions(-)
You're changing around two instances of duptype() here; I'd like to see them split into individual patches.
diff --git a/tools/widl/parser.y b/tools/widl/parser.y index 54dd3421c2..25bfa959f8 100644 --- a/tools/widl/parser.y +++ b/tools/widl/parser.y @@ -1652,7 +1652,7 @@ static var_t *declare_var(attr_list_t *attrs, decl_spec_t *declspec, const decla * store an offset to the typeformat string in the type object, but * two typeformat strings may be written depending on whether the * pointer is a toplevel parameter or not */
*pt = duptype(*pt, 1);
*pt = dup_pointer_type(*pt);
Well...
I firstly think dup_pointer_type() as a separate function is pretty pointless; you can just inline it here.
That said, I know you're not making things any worse with this series as far as this hack goes, but as long as we're going down the road of identifying types uniquely by their pointers, it behoves us to consider what a proper solution for this looks like. Consider that we might be duplicating a named typedef here; that could cause problems for us later. I think if we really want to make this unique (and that's a good guarantee to have), the best approach may be to store two different offsets in the type_t structure. Or, perhaps better, store an extra offset in the pointer_details structure.
} } else if (ptr_attr)diff --git a/tools/widl/typetree.c b/tools/widl/typetree.c index 7a4c8a50c5..d13bb71893 100644 --- a/tools/widl/typetree.c +++ b/tools/widl/typetree.c @@ -30,12 +30,16 @@ #include "typetree.h" #include "header.h"
-type_t *duptype(type_t *t, int dupname) +/* this function is only used in declare_var in parser.y, see FIXME note */ +type_t *dup_pointer_type(type_t *t) {
- type_t *d = alloc_type();
type_t *d;
assert(is_ptr(t) && t->details.pointer.def_fc != FC_RP);
d = alloc_type(); *d = *t;
- if (dupname && t->name)
if (t->name) d->name = xstrdup(t->name);
return d;
@@ -189,15 +193,19 @@ type_t *type_new_pointer(unsigned char pointer_default, type_t *ref, attr_list_t
type_t *type_new_alias(const decl_spec_t *ds, const char *name) {
- type_t *t = ds->type;
- type_t *a = duptype(t, 0);
- type_t *a = NULL;
- assert(ds != NULL);
- assert(name != NULL);
- a = alloc_type();
- /* copy over all the members before setting alias data */
- *a = *ds->type;
This is doing the same thing as duptype(), though. I know it's because of the way aliases work, but I don't think that keeping the way aliases currently work fits well with this refactor. I would propose to define a true alias_t, which simply contains a name and a pointer to the underlying type. Most places use type_get_real_type() already, so this should be pretty natural...
a->name = xstrdup(name); a->attrs = NULL; a->orig = *ds; a->is_alias = TRUE;
/* for pointer types */
a->details = t->details; init_loc_info(&a->loc_info);
return a;
diff --git a/tools/widl/typetree.h b/tools/widl/typetree.h index 2b0e8ba42d..da5deb4cef 100644 --- a/tools/widl/typetree.h +++ b/tools/widl/typetree.h @@ -53,8 +53,8 @@ type_t *type_coclass_define(type_t *coclass, ifref_list_t *ifaces); int type_is_equal(const type_t *type1, const type_t *type2); const char *type_get_name(const type_t *type, enum name_type name_type);
-/* FIXME: shouldn't need to export this */ -type_t *duptype(type_t *t, int dupname); +/* copy pointer type to deal with need for duplicate typeformat strings */ +type_t *dup_pointer_type(type_t *t);
/* un-alias the type until finding the non-alias type */ static inline type_t *type_get_real_type(const type_t *type) diff --git a/tools/widl/widltypes.h b/tools/widl/widltypes.h index b3665bc4bb..17477a604f 100644 --- a/tools/widl/widltypes.h +++ b/tools/widl/widltypes.h @@ -454,7 +454,7 @@ struct _type_t { struct bitfield_details bitfield; } details; const char *c_name;
- decl_spec_t orig; /* dup'd types */
- decl_spec_t orig; /* aliased types */ unsigned int typestring_offset; unsigned int ptrdesc; /* used for complex structs */ int typelib_idx;