[m-rev.] for post-commit review: getters and setters in deep profiler

[Paul Bone]

On Fri, Jan 02, 2009 at 01:58:52PM +1100, Zoltan Somogyi wrote:
> Add a query to the deep profiler for reporting on the getter and setter
> predicates in a module. This can be used to decide whether a data structure
> should be split in two, and if so, which fields should go into the
> substructure.
> 
> At the same time, improve the displays we generate for some other queries
> by making more table column headers capable of sorting the table.
> 
> deep_profiler/query.m:
> 	Add the new query type.
> 
> deep_profiler/report.m:
> 	Add the definition of the new report.
> 
> 	Include the identity of the callee, if known, in call site
> 	descriptions, to allow sorting on this.
> 
> deep_profiler/create_report.m:
> 	Add code to execute the new kind of query.
> 
> 	Include the new field in call site descriptions.
> 
> 	Remove "dump" from the name of the predicate that builds
> 	procrep_coverage reports, since the result isn't a raw dump.
> 
> deep_profiler/display_report.m:
> 	Add code to display the new kind of report.
> 
> 	Add a link to the new report to display we generate for the module
> 	query.
> 
> 	Allow the table in the procedure report to be sorted by context
> 	or by callee.
> 
> deep_profiler/display.m:
> 	Add a new table column class for displaying field names.
> 
> deep_profiler/html_format.m:
> 	Handle the new table column class.
> 
> deep_profiler/old_html_format.m:
> deep_profiler/old_query.m:
> 	"Handle" the new query.
> 
> Zoltan.
> 
> cvs diff: Diffing .
> Index: create_report.m
> ===================================================================
> RCS file: /home/mercury/mercury1/repository/mercury/deep_profiler/create_report.m,v
> retrieving revision 1.15
> diff -u -b -r1.15 create_report.m
> --- create_report.m	5 Nov 2008 03:38:40 -0000	1.15
> +++ create_report.m	30 Dec 2008 08:14:10 -0000
> @@ -564,6 +571,184 @@
*SNIP*
> +
> +:- pred is_getter_or_setter(string_proc_label::in, getter_or_setter::out,
> +    data_struct_name::out, field_name::out) is semidet.
> +
> +is_getter_or_setter(StringProcLabel, GetterSetter, DataStructName, FieldName) :-
> +    StringProcLabel = str_ordinary_proc_label(_PorF, DeclModule, DefModule,
> +        Name, Arity, _Mode),
> +    DeclModule = DefModule,
> +    string.to_char_list(Name, NameChars),
> +    is_getter_or_setter_2(NameChars, GetterSetter, DataStructNameChars,
> +        FieldNameChars),
> +    (
> +        GetterSetter = getter,
> +        Arity = 2
> +    ;
> +        GetterSetter = setter,
> +        Arity = 3
> +    ),
> +    string.from_char_list(DataStructNameChars, DataStructNameStr),
> +    string.from_char_list(FieldNameChars, FieldNameStr),
> +    DataStructName = data_struct_name(DataStructNameStr),
> +    FieldName = field_name(FieldNameStr).
> +
> +:- pred is_getter_or_setter_2(list(char)::in, getter_or_setter::out,
> +    list(char)::out, list(char)::out) is semidet.
> +
> +is_getter_or_setter_2(NameChars, GetterSetter, DataStructNameChars,
> +        FieldNameChars) :-
> +    ( NameChars = ['_', 'g', 'e', 't', '_' | FieldNameCharsPrime] ->
> +        GetterSetter = getter,
> +        DataStructNameChars = [],
> +        FieldNameChars = FieldNameCharsPrime
> +    ; NameChars = ['_', 's', 'e', 't', '_' | FieldNameCharsPrime] ->
> +        GetterSetter = setter,
> +        DataStructNameChars = [],
> +        FieldNameChars = FieldNameCharsPrime
> +    ;
> +        NameChars = [FirstNameChar | LaterNameChars],
> +        is_getter_or_setter_2(LaterNameChars, GetterSetter,
> +            LaterDataStructNameChars, FieldNameChars),
> +        DataStructNameChars = [FirstNameChar | LaterDataStructNameChars]
> +    ).
> +
> +%-----------------------------------------------------------------------------%
> +%

Is it possible to have a getter or setter named "_get_FIELD" or
"_set_FIELD", that is with a blank data structure name?
If not this predicate should be split in two such that a datastructure
name of non-zero length is enforced.  I beleive that this would be more
readable.

In any case it's probably best to simply split the string on the '_'
delimiter and ensure that each of the three components matches the
strings in is_getter_or_setter 


Everything else is fine.

Thanks.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mercurylang.org/archives/reviews/attachments/20090106/8619f350/attachment.sig>