[m-rev.] for post-commit review: add workarounds for bugs #184 and #214

Mark Brown mark at mercurylang.org
Wed Jun 29 20:14:28 AEST 2016


Hi Julien,

On Wed, Jun 29, 2016 at 4:23 PM, Julien Fischer <jfischer at opturion.com> wrote:
>
> For post-commit review by Mark or Zoltan.
>
> The diff below addresses the symptoms rather than the cause, which I suspect
> is
> either a bug in the type checker or the type checker not keeping track of
> enough information to produce an error message in the first place.

It doesn't keep track of enough information for us to produce *the*
error message we are trying to. I don't see anything that indicates a
bug in the type checker, since the errors it tries to report are
indeed type errors.

>  I am
> committing this now since, for bug #184 at least, it is an improvement over
> the
> status quo.  The request for a review concerns the XXXs below, in
> particular,
> do you now of there are some invariants on the structure of constraint_maps
> that are not documented?

No, and in particular it's not an invariant that the constraint map
contains all constraints that need to be satisfied. Moreover, the
documentation on constraint_id (the key type of constraint_map)
includes this:

% Only identifiers for constraints appearing directly on a goal are needed
% at the moment, so there is no way to represent the appropriate identifier
% for the superclass of such a constraint.

Hence there is no way to represent the constraint_id from bug #184, for example.

> compiler/post_typecheck.m:
>      Do not assume that all unproven constraints will appears as values
>      in the constraint map: that isn't true for the program in bug #184.

This is the right fix for the time being.

Instead of the XXXs, it would be better to say what we need to record
in order to improve the error messages further. Maybe more information
should be kept with the unproven constraints to begin with, in case
they end up in error messages? That would avoid the need to try to
figure out where the constraints came from.

Mark


More information about the reviews mailing list