| paolo.bonzini@gmail.com 2007-01-11, 4:16 am |
| Stephen Compall, a user of GNU Smalltalk, found two bugs in the
ParseTreeRewriter (parts of this message are taken from Stephen's posts
to the gst mailing list). I wanted to ask if they are known or relied
upon in any usage of the refactoring engine.
1) The first is that if there is a cascade, PTR doesn't descend into
arguments of the original node in this case, and, as a result, it won't
descend to the receiver. More clearly, this code:
stream display: obj.
(stream display: z) display: (stream display: x);
display: y; nextPut: $q
with the rule:
from: '``@receiver display: ``@object'
to: '``@object'
is rewritten to
obj.
(stream display: z) display: (stream display: x);
display: y; nextPut: $q
rather than
obj.
z display: x; display: y; nextPut: $q
2) The second issue is more complex. lookForMoreMatchesInContext:
doesn't copy its values. As a result, replacement in successful
replacements later rejected by acceptCascadeNode: (after
lookForMoreMatchesInContext: is already sent) depends on where in the
subtree a match happened.
The testcase is:
qqq display: (qqq display: sss value value);
display: [qqq display: sss value value]
with the rules
from: '``@recv display: ``@obj' to: '[``@obj]'
from: '`@recv value' to: '`@recv';
When the first rule is applied, the block node [qqq: display: sss value
value] itself is rewritten into [sss value value]. The two block nodes
are actually == so it doesn't matter whether we accept the new ``@obj
in the surrounding cascade; even if we don't, the originally matched
message now is qqq display: [[sss value value]].
IIUC, this causes some rules to be applied twice. Now, if you fix the
other oddity, the two rules result in the following
qqq display: [sss value];
display: [[sss]]
(instead the second block should read [sss value]).
The fixes for these bugs are easy. Would they be worthwhile? I'm a
bit leary of modifying the Brant&Roberts code because it's the same in
all the Smalltalks I know of. If there's consensus that these bugs are
to be fixed, however, I'll be happy to lead and change GNU Smalltalk.
To fix bug 1, modify acceptCascadeNode: in ParseTreeRewriter. After
the warning it prints on the transcript, add this line:
notFound add: each.
To fix bug 2, change this method:
lookForMoreMatchesInContext: oldContext
oldContext keysAndValuesDo:
[:key :value |
(key isString not and: [key recurseInto])
ifTrue:
[oldContext at: key put: (value collect: [:each |
self visitNode: each])]]
replacing the last "each" with "each copy".
Thanks for any ideas,
Paolo
|