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

Julien Fischer jfischer at opturion.com
Wed Jun 29 21:53:47 AEST 2016


Hi Mark,

On Wed, 29 Jun 2016, Mark Brown wrote:

> 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.

Ok, so it's the third possibility, post_typecheck is doing something
with the constraint_maps it shouldn't.

>>  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.

Ah, that's what I was looking for.

> 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.

I'll add something along the lines you suggest; the XXXs should stay,
the error message definitely need to be improved as well.

Julien.


More information about the reviews mailing list