[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Chicken-hackers] [PATCH] Use vertical space more liberally in some
From: |
megane |
Subject: |
Re: [Chicken-hackers] [PATCH] Use vertical space more liberally in some scrutinizer messages |
Date: |
Thu, 21 Mar 2019 08:39:07 +0200 |
User-agent: |
mu4e 1.0; emacs 25.1.1 |
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
Message not available
Re: [Chicken-hackers] [PATCH] Use vertical space more liberally in some scrutinizer messages,
megane <=
Re: [Chicken-hackers] [PATCH] Use vertical space more liberally in some scrutinizer messages, felix . winkelmann, 2019/03/21