Re: [Chicken-hackers] [PATCH] Use vertical space more liberally in some scrutinizer messages

[megane]

Evan Hanson <address@hidden> writes:

> Hi folks,
>

Hi Evan!

[snip]
>   * Patch 0012 - I don't think the name provides much benefit, and in
>     any case the way it was printed (with parens) was visually
>     confusing. Better to use "; <name>" or the like, later.

I didn't like that either. I just didn't have the guts to drop
information present in the old messages.

I think the name part could be dropped completely from the procedure
types without losing any information. The relevant names can be dug up
from the nodes at hand.
>
>   * Patch 0017 - I think the effect of this patch is important, but I
>     still feel that mapping back to the user-specified names is better
>     than regex-based number stripping, to avoid munging tvs that really
>     do include a number.

It feels there would have to be some serious identifier renaming black
magic to get the original names back after being renamed by
simplify-types a million times. I've tried to fix this but failed.

This is an opportunity to think about alternative for the current type
variable renaming scheme. I think the gensym version is not the right
way.

I'd like to replace the gensym with a simple (module internal) global
fixnum counter. This would leave strip-syntax purely for printing
messages to the user.

Currently a TV is renamed effectively like this:

'(list a123) -> `(list ,(gensym 'a123)), where the symbol a123 is the
name of the TV. This symbol is used to store a value in the unification
trail.

One alternative representation for a TV would be a simple list:

'(list (typevar 1 a123)) -> `(list (typevar ,(next-counter!) a123))

Here the current 'a123 is replaced with (typevar <counter>
original-name). The <counter> is used for the unification trail lookups.

The alternative I would use is the mutating record version I have
described in some earlier post (in the discussions about the recent
renaming fixes). This would help to simplify the scrutinizer in the
future by removing the typeenv.

I can make patch for this if this sounds OK.

>
> Anyway, the best way to understand the effect of this patch set is
> probably not to go through each patch individually (I've already done
> that, so you don't have to!) but rather to have a look at the "expected"
> files for the scrutiny tests to see what the resulting output looks
> like. Compare them to what we have currently and I think you'll see a
> nice improvement.
>
> Cheers,
>
> Evan
>
> _______________________________________________
> Chicken-hackers mailing list
> address@hidden
> https://lists.nongnu.org/mailman/listinfo/chicken-hackers