492643 - Rework how dehydra handles typedefs

Status: RESOLVED FIXED

(Developer Infrastructure :: Source Code Analysis, defect)

Description

[Benjamin Smedberg]

Currently, dehydra typedefs are split:

* the typedef decl object, which normally only shows up in process_decl
* the typedef type object. A typedef-of-int has a .name and a .typedef, but it doesn't have any of the normal properties of an int type, which means that any analysis code needs to explicitly walk through typedefs

Instead, I think we should have a single dehydra object which represents a typedef. It should have all the properties of the underlying type, as well as .name and .typedef, which is the actual underlying type.

I'm still not sure what should be done about the decls which are typedefs... I'm tempted to say we should forward them to process_type, *not* process_decl (that happens to make the impelementation much easier as well).

Comment 1

[Benjamin Smedberg]

Attachment: Dehydra typedefs, rev. 1

Comment 2

[Benjamin Smedberg]

This patch also fixes all the regressions mentioned in bug 492454

Comment 3

[(dormant account)]

Comment on attachment 377178 [details] [diff] [review]
Dehydra typedefs, rev. 1

> static jsval dehydra_convert2 (Dehydra *this, tree type, JSObject *obj) {
>+  xassert (TYPE_P(type));

I like the extra asserts. Please also do s/dehydra_convert/dehydra_convert_type/.


+static void dehydra_attachTypeTypedef(Dehydra *this, JSObject *obj, tree type) {
+  tree type_decl = TYPE_NAME (type);
+  if (type_decl && TREE_CODE(type_decl) == TYPE_DECL) {
+    tree original_type = DECL_ORIGINAL_TYPE (type_decl);
+    if (original_type) {
+      dehydra_defineStringProperty (this, obj, NAME,
+                                    IDENTIFIER_POINTER(DECL_NAME(type_decl)));
+      jsval subval = dehydra_convert (this, original_type);
+      dehydra_defineProperty (this, obj, TYPEDEF, subval);
+      dehydra_setLoc (this, obj, type_decl);
+    }
+  }
+}

Here the logic should be if(!typedef) return. Whole function bodies shouldn't be in conditionals.

>+  tree type_decl;

I don't like these variables outside of switch statements.

>   case FIXED_POINT_TYPE:
> #endif
>     /* The following code is ported from GCC c-pretty-print.c:pp_c_type_specifier */
adding {} is the preferred way to go here:
{ tree type_decl...
>+    type_decl = TYPE_NAME (type);
>     if (type_decl) {
>       dehydra_defineStringProperty (this, obj, NAME, IDENTIFIER_POINTER(TREE_CODE(type_decl) == TYPE_DECL ?
>                                                                         DECL_NAME(type_decl) : type_decl));
>     }
} is

>     else {
>       int prec = TYPE_PRECISION (type);
>       dehydra_defineProperty (this, obj, BITFIELD, INT_TO_JSVAL (prec));
> 
>@@ -472,16 +471,17 @@ static jsval dehydra_convert2 (Dehydra *
>     }
>   default:
>     warning (1, "Unhandled %s: %s", tree_code_name[TREE_CODE(type)],
>            type_as_string (type, TFF_CHASE_TYPEDEF));
>     dehydra_defineStringProperty (this, obj, NAME, type_as_string (type, 0));
>     break;
>   }
>   dehydra_attachTypeAttributes (this, obj, type);
>+  dehydra_attachTypeTypedef (this, obj, type);
>   if (next_type != NULL_TREE) {
>     dehydra_defineProperty (this, obj, TYPE, dehydra_convert (this, next_type));
>   }
>   return OBJECT_TO_JSVAL (obj);
> }
> 
>+  // if (TYPE_READONLY(type))
>+  //   quals.push('const ');

Delete code, don't comment it out

Excellent stuff overall. I like it when dehydra stuff gets prettier, instead of more convoluted. I would appreciate some docs on the wiki on typedefs.

Comment 4

[Benjamin Smedberg]

http://hg.mozilla.org/users/tglek_mozilla.com/dehydra-gcc/rev/675a6d467b66

I'll have MDC updated in just a bit.

Comment 5

[(dormant account)]

gcc45 compat:
http://hg.mozilla.org/users/tglek_mozilla.com/dehydra-gcc/rev/05f5b749f4aa