[m-rev.] for review: fix mantis bug 544
Zoltan Somogyi
zoltan.somogyi at runbox.com
Mon Feb 7 17:34:09 AEDT 2022
2022-02-07 17:19 GMT+11:00 "Julien Fischer" <jfischer at opturion.com>:
> Once this is committed,
It is committed, but you may want to wait for ...
> On Mon, 7 Feb 2022, Zoltan Somogyi wrote:
>
>> For review by Julien.
>>
>> As mentioned in a comment, we could eliminate one source
>> of overhead added by this diff by adding a common_subset
>> predicate to assoc_list.m. Would anyone object to this?
>
> No objections; ideally map.m and assoc_list.map should support the same
> (or at least a very similar) set of operations, both being map ADTs.
... this, which I am starting on now.
> That's one reason the LLDS code generator is not affected; the other is
> simply that it introduces trail operations directly and does not use
> add_trail_ops.m.
I added a mention.
>> + % closure to the values associated with the key in MapA and MapB.
>> % Fail if and only if this closure fails on the values associated
>> % with some common key.
>
>
> More generally, I would note that there does not seem to be anything in the test
> suite that exercises common_subset.
Unfortunately, there are many more stdlib preds in that same boat.
>> --- a/trace/mercury_trace_cmd_browsing.c
>> +++ b/trace/mercury_trace_cmd_browsing.c
>> @@ -847,6 +847,7 @@ MR_trace_cmd_dump(char **words, int word_count, MR_TraceCmdInfo *cmd,
>> const char *name;
>>
>> MR_convert_arg_to_var_spec(words[1], &var_spec);
>> + // MR_trace_parse_var_path
>> problem = MR_lookup_unambiguous_var_spec(var_spec,
>> &type_info, &value, &name);
>> if (problem == NULL) {
>
> Did you mean to add this?
No. I followed all your other suggestions.
> The rest looks fine -- thanks for looking into that.
You are welcome.
Zoltan.
More information about the reviews
mailing list