[m-rev.] for review: fix mantis bug 354

[Julien Fischer]

Hi Zoltan,

On Thu, 28 Aug 2014, Zoltan Somogyi wrote:

> For review by anyone.
>
> Fix Mantis bug #354.
> 
> I/O tabling has two main purposes. The first and more important is to allow the
> debugger to replay parts of the program execution for the programmer, which
> requires making I/O operations idempotent (so that we get the same results on
> the second, third etc "execution" as on the first). The second purpose is to
> let the person using the debugger actually see a list of the I/O actions, and
> their results.
> 
> The root of the problem here is that the compiler can do the second part
> only if it has access to the type_infos describing the types of the arguments
> of the I/O action. With the current infrastructure for representing typeclass
> information, this is not always possible in the presence of typeclass
> constraints on I/O action predicates. The reason is that polymorphism.m can
> put the typeinfo for a type variable that is subject to a typeclass constraint
> arbitrarily deep inside the typeclass_info for that constraint, but the RTTI
> can encode such locations only up to a fixed depth (currently only the
> shallowest embedded is encodable).
> 
> Before this fix, the test case for this bug got a compiler abort when the
> I/O tabling transformation tried to figure out how to table the typeclass
> info representing the typeclass constraint on a I/O action predicate.
> 
> We still cannot table typeclass infos. We could store them (I/O tabling
> does not require anything more complicated), but the problem of deeply buried
> typeinfos inside them would still remain. So this fix consists of two parts:
> 
> - for typeclass constrained I/O primitives, recording only enough information
>   to allow them to replayed (the first purpose above), and not to print them
>   out (the second purpose), and
> - getting the runtime system to understand this, and not crash with a core dump
>   in the absence of the information required for the second purpose.
> 
> This second part requires changes to the RTTI used by I/O tabling. These
> changes BREAK BINARY COMPATIBILITY in debug grades.

...

> index b801d10..b82b419 100644
> --- a/compiler/layout.m
> +++ b/compiler/layout.m
> @@ -173,16 +173,35 @@
>                  plps_coverage_points    :: maybe({int, int})
>              ).
> 
> -:- type table_io_decl_data
> -    --->    table_io_decl_data(
> -                % defines MR_TableIoDecl
> -                tid_proc_ptr            :: layout_name,
> -                tid_num_ptis            :: int,
> -
> -                % pseudo-typeinfos for headvars
> -                tid_ptis                :: rval,
> +:- type table_io_args_data
> +    --->    table_io_args_data(
> +                % The number of head variables are stored in the answer

s/are/that are/

> +                % block. Besides the values of the output arguments,
> +                % which are needed to make the I/O action idempotent,
> +                % it also includes the values of the input arguments.
> +                % Some of these, the type_infos, mdb needs in order to
> +                % convert the type-specific representations of the outputs
> +                % to a term that the debugger's user can print or browse.
> +                % The other inputs are needed to allow the debugger's user
> +                % to judge whether the output are correct or not.
> +                tia_answerblock_arity   :: int,
> +
> +                % This rval stores a vector of answerblock_arity pseudo-
> +                % typeinfos, one for each headvar stored in the answer block.
> +                tia_ptis                :: rval,
> +
> +                % This rval represents a vector mapping each type variable
> +                % that occurs in the above pseudo-typeinfos to the location
> +                % of the type_info that is currently bound to that type
> +                % variable.
> +                tia_type_params         :: rval
> +            ).

...

> --- a/trace/mercury_trace_vars.c
> +++ b/trace/mercury_trace_vars.c
> @@ -17,6 +17,7 @@
>
>  #include "mercury_imp.h"
>  #include "mercury_array_macros.h"
> +#include "mercury_builtin_types.h"
>  #include "mercury_memory.h"
>  #include "mercury_layout_util.h"
>  #include "mercury_deconstruct.h"
> @@ -1301,16 +1302,32 @@ MR_trace_browse_action(FILE *out, MR_IoActionNum action_number,
>  {
>      MR_ConstString  proc_name;
>      MR_Word         is_func;
> +    MR_bool         have_arg_infos;
>      MR_Word         arg_list;
>      MR_bool         io_action_tabled;
>      MR_bool         saved_io_tabling_enabled;
>
>      io_action_tabled = MR_trace_get_action(action_number, &proc_name, &is_func,
> -        &arg_list);
> +        &have_arg_infos, &arg_list);
>      if (!io_action_tabled) {
>          return "I/O action number not in range";
>      }
> 
> +    if (! have_arg_infos) {
> +        MR_TypeInfo type_info;
> +        MR_Word     value;
> +        MR_Word     arg;
> +
> +        MR_restore_transient_hp();
> +        arg_list = MR_list_empty();
> +        type_info = (MR_TypeInfo) &MR_TYPE_CTOR_INFO_NAME(builtin, string, 0);
> +        value = (MR_Word) MR_make_string_const(
> +            "the arguments are not available due to the presence of one or more typeclass constraints");

Are they "typeclass constraints" or "type class constraints"?  The reference
manual uses the latter.  (The contents of the .err_exp files in tests/invalid
are divided on the subject.)

> +        MR_new_univ_on_hp(arg, type_info, value);
> +        arg_list = MR_univ_list_cons(arg, arg_list);
> +        MR_save_transient_hp();
> +    }
> +
>      saved_io_tabling_enabled = MR_io_tabling_enabled;
>      MR_io_tabling_enabled = MR_FALSE;
>      (*browser)(proc_name, arg_list, is_func, caller, format);

The change itself is fine.

Cheers,
Julien.