Need help with Rewrite Rule - Instantiations case 49471

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

Need help with Rewrite Rule - Instantiations case 49471

NiallRoss
Dear Solveig and Joachim,
   this is an effect of how the checking code that Adriaan and I added
to the RB interacts with a lacuna in the RBParser code.

The checking code is

RewriteSingleMetaCodeTool>>verifyForUnknownVariableOf: node in:
oldNode
        "A pattern variable in the replace text must correspond to a pattern
variable in the search text.  Mistyping is a common cause of new user
frustration."

        (node isPatternNode and: [node isVariable]) ifFalse: [^true].
        ((oldNode references: node name) or: [oldNode defines: node name])
ifTrue: [^true].
        self insertError: 'MetaVar not in search text ' at: node start.
        ^false

It fails because
        RBMethodNode>>defines:
only accepts the method's arguments as being defined by the method
node, nt its temporaries.  It passes the
        RBMethodNode>>references:
call toits body (which is an RBSequenceNode) and
        RBSequenceNode>>references:
only asks its statements what it references, not its temporaries.
Instead
        RBSequenceNode>>defines:
looks at temporaries.  Thus the test
        (oldNode references: node name) or: [oldNode defines: node name]
almost always works nevertheless because usually either (a) the match
code is
        | `@temps |
        ``@.stats
(i.e. without method name, i.e. with the All Method box unticked) and
then
        RBSequenceNode>>defines:
finds the temp expression, or else (b) if a method defines a temp then
its statements reference it, and if a specific temp is replaced by a
pattern then the same pattern will be in the statements replacing its
reference.

Joachim's code fails because in

someMethodName
        | `@temps |
        ``@.stats

(BTW there is no need for a second ` in `@temps - you can't recurse
into a temp declaration) the checking code is sent to an RBMethodNode
and its #defines: does not look at temps, while its #references: asks
the body which only looks in the statements and `@temps is not
mentioned in ``@.stats (of course).

My feeling is that a method node should return true to either
#defines: or #references: for its temp declarations but I should look
at whether this has any side effects.  Meanwhile, hopefuly this will
let Joachim and/or Solveig experiment with fixes.

        Yours faithfully
                Niall Ross

--
You received this message because you are subscribed to the Google Groups "VA Smalltalk" group.
To post to this group, send email to [hidden email].
To unsubscribe from this group, send email to [hidden email].
For more options, visit this group at http://groups.google.com/group/va-smalltalk?hl=en.

Reply | Threaded
Open this post in threaded view
|

Re: Need help with Rewrite Rule - Instantiations case 49471

jtuchel
Niall,

thanks a lot for your pointers. I guess this will help us fix the
problem a lot faster. So our rewrite experiment hit a spot ;-) This also
means that the probability of rewrites being broken for multiple
dialects is high...? Where would we send a possible fix to? The
Refactory? You?

Again, thanks a lot for this prompt and precise reaction.

Joachim

Am 09.04.12 23:32, schrieb NiallRoss:

> Dear Solveig and Joachim,
>     this is an effect of how the checking code that Adriaan and I added
> to the RB interacts with a lacuna in the RBParser code.
>
> The checking code is
>
> RewriteSingleMetaCodeTool>>verifyForUnknownVariableOf: node in:
> oldNode
> "A pattern variable in the replace text must correspond to a pattern
> variable in the search text.  Mistyping is a common cause of new user
> frustration."
>
> (node isPatternNode and: [node isVariable]) ifFalse: [^true].
> ((oldNode references: node name) or: [oldNode defines: node name])
> ifTrue: [^true].
> self insertError: 'MetaVar not in search text ' at: node start.
> ^false
>
> It fails because
> RBMethodNode>>defines:
> only accepts the method's arguments as being defined by the method
> node, nt its temporaries.  It passes the
> RBMethodNode>>references:
> call toits body (which is an RBSequenceNode) and
> RBSequenceNode>>references:
> only asks its statements what it references, not its temporaries.
> Instead
> RBSequenceNode>>defines:
> looks at temporaries.  Thus the test
> (oldNode references: node name) or: [oldNode defines: node name]
> almost always works nevertheless because usually either (a) the match
> code is
> | `@temps |
> ``@.stats
> (i.e. without method name, i.e. with the All Method box unticked) and
> then
> RBSequenceNode>>defines:
> finds the temp expression, or else (b) if a method defines a temp then
> its statements reference it, and if a specific temp is replaced by a
> pattern then the same pattern will be in the statements replacing its
> reference.
>
> Joachim's code fails because in
>
> someMethodName
> | `@temps |
> ``@.stats
>
> (BTW there is no need for a second ` in `@temps - you can't recurse
> into a temp declaration) the checking code is sent to an RBMethodNode
> and its #defines: does not look at temps, while its #references: asks
> the body which only looks in the statements and `@temps is not
> mentioned in ``@.stats (of course).
>
> My feeling is that a method node should return true to either
> #defines: or #references: for its temp declarations but I should look
> at whether this has any side effects.  Meanwhile, hopefuly this will
> let Joachim and/or Solveig experiment with fixes.
>
> Yours faithfully
> Niall Ross




smime.p7s (7K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Need help with Rewrite Rule - Instantiations case 49471

NiallRoss
Dear Joachim and Solveig,

Joachim Tuchel wrote:

> thanks a lot for your pointers. I guess this will help us fix the
> problem a lot faster. So our rewrite experiment hit a spot ;-)

Yes indeed.  It was a good case to expose the exact problem.

> This also means that the probability of rewrites being broken for
> multiple dialects is high...? Where would we send a possible fix to?
> The Refactory? You?

Me, definitely:  as far as I know, the custom refactoring project is the
one maintaining compatibility of the cross-dialect RB model in
VASmalltalk and VW these days.  We also maintain it in Smalltalk/X (so I
will pass the fix to Jan) and in Pharo/Squeak (I'll fix and publish
and/or let them know) - those two are insofar as I have the time (which
is often not very far and/or a bit late).

I will try and fix this in VW and VASmalltalk this weekend but by all
means you and/or Solveig get in ahead of me for VASmalltalk.  I think
the most likely fix is that things referred to by an RBMethodNode should
include things defined by its body, but I must think about whether
instead they should be treated as also defined by the method node.  
Either is fairly easy to implement - it is merely whether either could
have a side-effect and which is conceptually right.  (Also, VW allows
methods to define temps that override names in their scope whereas
VASmalltalk, I think, does not - I'm hoping that will not discriminate
between which of RBMethodNode>>defines: and RBMethodNode>>references:
should be added to in the two dialects.)

I aim to fix this - or test a fix you email me - this weekend.  (Being
just back from holiday, I have a full email  in-box re VW work, so all
VA must wait till the weekend. :-) )

By all means copy John Brant on any emails if you wish - he'd doubtless
be interested and may have insights.  AFAIK, John is not actively
maintaining the RB these days.  Travis is the TOOLs guru for VW - I'll
be informing him.

                Yours faithfully
                      Niall Ross

>
> Again, thanks a lot for this prompt and precise reaction.
>
> Joachim
>
> Am 09.04.12 23:32, schrieb NiallRoss:
>
>> Dear Solveig and Joachim,
>>     this is an effect of how the checking code that Adriaan and I added
>> to the RB interacts with a lacuna in the RBParser code.
>>
>> The checking code is
>>
>> RewriteSingleMetaCodeTool>>verifyForUnknownVariableOf: node in:
>> oldNode
>>     "A pattern variable in the replace text must correspond to a pattern
>> variable in the search text.  Mistyping is a common cause of new user
>> frustration."
>>
>>     (node isPatternNode and: [node isVariable]) ifFalse: [^true].
>>     ((oldNode references: node name) or: [oldNode defines: node name])
>> ifTrue: [^true].
>>     self insertError: 'MetaVar not in search text ' at: node start.
>>     ^false
>>
>> It fails because
>>     RBMethodNode>>defines:
>> only accepts the method's arguments as being defined by the method
>> node, nt its temporaries.  It passes the
>>     RBMethodNode>>references:
>> call toits body (which is an RBSequenceNode) and
>>     RBSequenceNode>>references:
>> only asks its statements what it references, not its temporaries.
>> Instead
>>     RBSequenceNode>>defines:
>> looks at temporaries.  Thus the test
>>     (oldNode references: node name) or: [oldNode defines: node name]
>> almost always works nevertheless because usually either (a) the match
>> code is
>>     | `@temps |
>>     ``@.stats
>> (i.e. without method name, i.e. with the All Method box unticked) and
>> then
>>     RBSequenceNode>>defines:
>> finds the temp expression, or else (b) if a method defines a temp then
>> its statements reference it, and if a specific temp is replaced by a
>> pattern then the same pattern will be in the statements replacing its
>> reference.
>>
>> Joachim's code fails because in
>>
>> someMethodName
>>     | `@temps |
>>     ``@.stats
>>
>> (BTW there is no need for a second ` in `@temps - you can't recurse
>> into a temp declaration) the checking code is sent to an RBMethodNode
>> and its #defines: does not look at temps, while its #references: asks
>> the body which only looks in the statements and `@temps is not
>> mentioned in ``@.stats (of course).
>>
>> My feeling is that a method node should return true to either
>> #defines: or #references: for its temp declarations but I should look
>> at whether this has any side effects.  Meanwhile, hopefuly this will
>> let Joachim and/or Solveig experiment with fixes.
>>
>>     Yours faithfully
>>         Niall Ross
>
>
>
>
>


______________________________________________________________________
This email has been scanned by the Symantec Email Security.cloud service.
For more information please visit http://www.symanteccloud.com
______________________________________________________________________

--
You received this message because you are subscribed to the Google Groups "VA Smalltalk" group.
To post to this group, send email to [hidden email].
To unsubscribe from this group, send email to [hidden email].
For more options, visit this group at http://groups.google.com/group/va-smalltalk?hl=en.

Reply | Threaded
Open this post in threaded view
|

1) Rewrite Rule fix - Instantiations case 49471 2) New case

NiallRoss
Dear Solveig,
    I attach a zip with two fixes.

1) The file-out of #verifyForUnknownVariableOf:in: (RBSmallintUIApp), I
recommend as the correct fix for the issue Joachim found - case 49471.  
(I am incorporating this into my current work on the RB but the next
version may not be out for a while, so by all means add it right away to
your current development maps / fixes / whatever.)  I've made the
checking code handle the case where the oldNode isMethod and so its
body's definitions need also to be checked.

2) I notice that Class>>definitionMessage has changed.  Previously it
only added the 'classInstanceVariableNames:' part of the class
definition message if the class had any class instance variables.  Now
it always adds it.  This interacts with an aspect of the RB framework to
cause
    RBNamespaceTest>>testDefineClassAfterDeletedChange
to fail.  I attach a fileOut of a change to that test
(RefactoryTestingApp) to make it pass.


(The rest of this email is just remarks about this second case  ignore
unless interested.)

I have not seen other issues related to this, but detail the cause below
it in case anyone else has.

    a) AddClassChange>>isValidMessageName: in VA (and VW3) only accepts
class creation messages without 'classInstanceVariableNames:'.  This
part of the RB framework has separate handling of class and metaclass
(the isMeta variable in the AddClassChange), so class instance variables
are parsed as the instance variables of the metaclass, not in the same
definition string as that of the class.  (VW7, by contrast, does have a
single message, so the RB code is slightly different there.)

    b) RBNamespaceTest>>testDefineClassAfterDeletedChange sendt
#definitionString to self to get the definition it needs to make an
AddClassChange for the test.  This now fails to parse, since
#isValidMessageName: rejects it, so the test fails.  The rather trivial
fix is to replace
    RBNamespaceTest>>testDefineClassAfterDeletedChange
        ...
        st defineClass: self class definitionString in: RefactoryTestingApp
        ...
with hardcoding of all but the class name part of the string of the
class' trivial message.

Since there was always the option for this keyword to be there (i.e. if
the class had classInstanceVariables), I don't think is an actual bug
(we must have seen failures on every class with classInstanceVariables
if the code was not robust to it);  rather, it is an undesirable feature
which this change reveals to us.  In the past, one could create an
AddClassChange in a script via
    AddClassChange definition: someClass definitionString
if the class had no classInstanceVariables, but not if it did.  Now you
can never use it.  Except in this one test, it is not used in the base.  
Maybe people who hacked scripts in the RB framework sometimes used it
and were sometimes caught out when a class had classInstanceVariables.

(N.B. it would not be right to change the framework just to add the new
messages to #isValidMessageName:, since  in
    AddClassChange>>fillOutDefinitions
there is hardcoding of
    instancevariableNames := self namesIn: (parseTree arguments at: 2)
value.
    ... ... arguments at: 3 ...
etc., and this would need to be changed as well to handle both messages
with keyword #classInstanceVariables: and those without.)

             Yours faithfully
                   Niall Ross

Niall Ross wrote:

>>> this is an effect of how the checking code that Adriaan and I added
>>> to the RB interacts with a lacuna in the RBParser code.
>>>
>>> The checking code is
>>>
>>> RewriteSingleMetaCodeTool>>verifyForUnknownVariableOf: node in:
>>> oldNode
>>>     "A pattern variable in the replace text must correspond to a
>>> pattern
>>> variable in the search text.  Mistyping is a common cause of new user
>>> frustration."
>>>
>>>     (node isPatternNode and: [node isVariable]) ifFalse: [^true].
>>>     ((oldNode references: node name) or: [oldNode defines: node name])
>>> ifTrue: [^true].
>>>     self insertError: 'MetaVar not in search text ' at: node start.
>>>     ^false
>>>
>>> It fails because
>>>     RBMethodNode>>defines:
>>> only accepts the method's arguments as being defined by the method
>>> node, nt its temporaries.  It passes the
>>>     RBMethodNode>>references:
>>> call toits body (which is an RBSequenceNode) and
>>>     RBSequenceNode>>references:
>>> only asks its statements what it references, not its temporaries.
>>> Instead
>>>     RBSequenceNode>>defines:
>>> looks at temporaries.  Thus the test
>>>     (oldNode references: node name) or: [oldNode defines: node name]
>>> almost always works nevertheless because usually either (a) the match
>>> code is
>>>     | `@temps |
>>>     ``@.stats
>>> (i.e. without method name, i.e. with the All Method box unticked) and
>>> then
>>>     RBSequenceNode>>defines:
>>> finds the temp expression, or else (b) if a method defines a temp then
>>> its statements reference it, and if a specific temp is replaced by a
>>> pattern then the same pattern will be in the statements replacing its
>>> reference.
>>>
>>> Joachim's code fails because in
>>>
>>> someMethodName
>>>     | `@temps |
>>>     ``@.stats
>>>
>>> (BTW there is no need for a second ` in `@temps - you can't recurse
>>> into a temp declaration) the checking code is sent to an RBMethodNode
>>> and its #defines: does not look at temps, while its #references: asks
>>> the body which only looks in the statements and `@temps is not
>>> mentioned in ``@.stats (of course).
>>

______________________________________________________________________
This email has been scanned by the Symantec Email Security.cloud service.
For more information please visit http://www.symanteccloud.com
______________________________________________________________________

--
You received this message because you are subscribed to the Google Groups "VA Smalltalk" group.
To post to this group, send email to [hidden email].
To unsubscribe from this group, send email to [hidden email].
For more options, visit this group at http://groups.google.com/group/va-smalltalk?hl=en.


fixRewriteRuleAndTest.zip (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: 1) Rewrite Rule fix - Instantiations case 49471 2) New case

jtuchel
Hi Niall,

I tried your fix for #verifyForUnknownVariableOf:in: and now the above mentioned rewriting works like intended.
Sorry for my late reaction, but you know how life and especially work tends to be from time to time..

So thank you very much for your fix and the explanations! I hope Instantiations will inlcude your fix into the next release of VAST.

Joachim


Am Montag, 16. April 2012 15:58:09 UTC+2 schrieb NiallRoss:
Dear Solveig,
    I attach a zip with two fixes.

1) The file-out of #verifyForUnknownVariableOf:in: (RBSmallintUIApp), I
recommend as the correct fix for the issue Joachim found - case 49471.  
(I am incorporating this into my current work on the RB but the next
version may not be out for a while, so by all means add it right away to
your current development maps / fixes / whatever.)  I've made the
checking code handle the case where the oldNode isMethod and so its
body's definitions need also to be checked.

2) I notice that Class>>definitionMessage has changed.  Previously it
only added the 'classInstanceVariableNames:' part of the class
definition message if the class had any class instance variables.  Now
it always adds it.  This interacts with an aspect of the RB framework to
cause
    RBNamespaceTest>>testDefineClassAfterDeletedChange
to fail.  I attach a fileOut of a change to that test
(RefactoryTestingApp) to make it pass.


(The rest of this email is just remarks about this second case  ignore
unless interested.)

I have not seen other issues related to this, but detail the cause below
it in case anyone else has.

    a) AddClassChange>>isValidMessageName: in VA (and VW3) only accepts
class creation messages without 'classInstanceVariableNames:'.  This
part of the RB framework has separate handling of class and metaclass
(the isMeta variable in the AddClassChange), so class instance variables
are parsed as the instance variables of the metaclass, not in the same
definition string as that of the class.  (VW7, by contrast, does have a
single message, so the RB code is slightly different there.)

    b) RBNamespaceTest>>testDefineClassAfterDeletedChange sendt
#definitionString to self to get the definition it needs to make an
AddClassChange for the test.  This now fails to parse, since
#isValidMessageName: rejects it, so the test fails.  The rather trivial
fix is to replace
    RBNamespaceTest>>testDefineClassAfterDeletedChange
        ...
        st defineClass: self class definitionString in: RefactoryTestingApp
        ...
with hardcoding of all but the class name part of the string of the
class' trivial message.

Since there was always the option for this keyword to be there (i.e. if
the class had classInstanceVariables), I don't think is an actual bug
(we must have seen failures on every class with classInstanceVariables
if the code was not robust to it);  rather, it is an undesirable feature
which this change reveals to us.  In the past, one could create an
AddClassChange in a script via
    AddClassChange definition: someClass definitionString
if the class had no classInstanceVariables, but not if it did.  Now you
can never use it.  Except in this one test, it is not used in the base.  
Maybe people who hacked scripts in the RB framework sometimes used it
and were sometimes caught out when a class had classInstanceVariables.

(N.B. it would not be right to change the framework just to add the new
messages to #isValidMessageName:, since  in
    AddClassChange>>fillOutDefinitions
there is hardcoding of
    instancevariableNames := self namesIn: (parseTree arguments at: 2)
value.
    ... ... arguments at: 3 ...
etc., and this would need to be changed as well to handle both messages
with keyword #classInstanceVariables: and those without.)

             Yours faithfully
                   Niall Ross

Niall Ross wrote:

>>> this is an effect of how the checking code that Adriaan and I added


>>> to the RB interacts with a lacuna in the RBParser code.
>>>
>>> The checking code is
>>>
>>> RewriteSingleMetaCodeTool>>verifyForUnknownVariableOf: node in:
>>> oldNode
>>>     "A pattern variable in the replace text must correspond to a
>>> pattern
>>> variable in the search text.  Mistyping is a common cause of new user
>>> frustration."
>>>
>>>     (node isPatternNode and: [node isVariable]) ifFalse: [^true].
>>>     ((oldNode references: node name) or: [oldNode defines: node name])
>>> ifTrue: [^true].
>>>     self insertError: 'MetaVar not in search text ' at: node start.
>>>     ^false
>>>
>>> It fails because
>>>     RBMethodNode>>defines:
>>> only accepts the method's arguments as being defined by the method
>>> node, nt its temporaries.  It passes the
>>>     RBMethodNode>>references:
>>> call toits body (which is an RBSequenceNode) and
>>>     RBSequenceNode>>references:
>>> only asks its statements what it references, not its temporaries.
>>> Instead
>>>     RBSequenceNode>>defines:
>>> looks at temporaries.  Thus the test
>>>     (oldNode references: node name) or: [oldNode defines: node name]
>>> almost always works nevertheless because usually either (a) the match
>>> code is
>>>     | `@temps |
>>>     ``@.stats
>>> (i.e. without method name, i.e. with the All Method box unticked) and
>>> then
>>>     RBSequenceNode>>defines:
>>> finds the temp expression, or else (b) if a method defines a temp then
>>> its statements reference it, and if a specific temp is replaced by a
>>> pattern then the same pattern will be in the statements replacing its
>>> reference.
>>>
>>> Joachim's code fails because in
>>>
>>> someMethodName
>>>     | `@temps |
>>>     ``@.stats
>>>
>>> (BTW there is no need for a second ` in `@temps - you can't recurse
>>> into a temp declaration) the checking code is sent to an RBMethodNode
>>> and its #defines: does not look at temps, while its #references: asks
>>> the body which only looks in the statements and `@temps is not
>>> mentioned in ``@.stats (of course).
>>

______________________________________________________________________
This email has been scanned by the Symantec Email Security.cloud service.
For more information please visit http://www.symanteccloud.com
______________________________________________________________________

--
You received this message because you are subscribed to the Google Groups "VA Smalltalk" group.
To view this discussion on the web visit https://groups.google.com/d/msg/va-smalltalk/-/6cFEa3e3lJ8J.
To post to this group, send email to [hidden email].
To unsubscribe from this group, send email to [hidden email].
For more options, visit this group at http://groups.google.com/group/va-smalltalk?hl=en.
Reply | Threaded
Open this post in threaded view
|

Re: 1) Rewrite Rule fix - Instantiations case 49471 2) New case

Solveig Viste
Hi Niall,
 
It works for me as well. Regrets on the delay. Thanks for the patch.
 
Solveig
On Mon, Apr 23, 2012 at 2:16 AM, [hidden email] <[hidden email]> wrote:
Hi Niall,

I tried your fix for #verifyForUnknownVariableOf:in: and now the above mentioned rewriting works like intended.
Sorry for my late reaction, but you know how life and especially work tends to be from time to time..

So thank you very much for your fix and the explanations! I hope Instantiations will inlcude your fix into the next release of VAST.

Joachim


Am Montag, 16. April 2012 15:58:09 UTC+2 schrieb NiallRoss:
Dear Solveig,
    I attach a zip with two fixes.

1) The file-out of #verifyForUnknownVariableOf:in: (RBSmallintUIApp), I
recommend as the correct fix for the issue Joachim found - case 49471.  
(I am incorporating this into my current work on the RB but the next
version may not be out for a while, so by all means add it right away to
your current development maps / fixes / whatever.)  I've made the
checking code handle the case where the oldNode isMethod and so its
body's definitions need also to be checked.

2) I notice that Class>>definitionMessage has changed.  Previously it
only added the 'classInstanceVariableNames:' part of the class
definition message if the class had any class instance variables.  Now
it always adds it.  This interacts with an aspect of the RB framework to
cause
    RBNamespaceTest>>testDefineClassAfterDeletedChange
to fail.  I attach a fileOut of a change to that test
(RefactoryTestingApp) to make it pass.


(The rest of this email is just remarks about this second case  ignore
unless interested.)

I have not seen other issues related to this, but detail the cause below
it in case anyone else has.

    a) AddClassChange>>isValidMessageName: in VA (and VW3) only accepts
class creation messages without 'classInstanceVariableNames:'.  This
part of the RB framework has separate handling of class and metaclass
(the isMeta variable in the AddClassChange), so class instance variables
are parsed as the instance variables of the metaclass, not in the same
definition string as that of the class.  (VW7, by contrast, does have a
single message, so the RB code is slightly different there.)

    b) RBNamespaceTest>>testDefineClassAfterDeletedChange sendt
#definitionString to self to get the definition it needs to make an
AddClassChange for the test.  This now fails to parse, since
#isValidMessageName: rejects it, so the test fails.  The rather trivial
fix is to replace
    RBNamespaceTest>>testDefineClassAfterDeletedChange
        ...
        st defineClass: self class definitionString in: RefactoryTestingApp
        ...
with hardcoding of all but the class name part of the string of the
class' trivial message.

Since there was always the option for this keyword to be there (i.e. if
the class had classInstanceVariables), I don't think is an actual bug
(we must have seen failures on every class with classInstanceVariables
if the code was not robust to it);  rather, it is an undesirable feature
which this change reveals to us.  In the past, one could create an
AddClassChange in a script via
    AddClassChange definition: someClass definitionString
if the class had no classInstanceVariables, but not if it did.  Now you
can never use it.  Except in this one test, it is not used in the base.  
Maybe people who hacked scripts in the RB framework sometimes used it
and were sometimes caught out when a class had classInstanceVariables.

(N.B. it would not be right to change the framework just to add the new
messages to #isValidMessageName:, since  in
    AddClassChange>>fillOutDefinitions
there is hardcoding of
    instancevariableNames := self namesIn: (parseTree arguments at: 2)
value.
    ... ... arguments at: 3 ...
etc., and this would need to be changed as well to handle both messages
with keyword #classInstanceVariables: and those without.)

             Yours faithfully
                   Niall Ross

Niall Ross wrote:

>>> this is an effect of how the checking code that Adriaan and I added
>>> to the RB interacts with a lacuna in the RBParser code.
>>>
>>> The checking code is
>>>
>>> RewriteSingleMetaCodeTool>>verifyForUnknownVariableOf: node in:
>>> oldNode
>>>     "A pattern variable in the replace text must correspond to a
>>> pattern
>>> variable in the search text.  Mistyping is a common cause of new user
>>> frustration."
>>>
>>>     (node isPatternNode and: [node isVariable]) ifFalse: [^true].
>>>     ((oldNode references: node name) or: [oldNode defines: node name])
>>> ifTrue: [^true].
>>>     self insertError: 'MetaVar not in search text ' at: node start.
>>>     ^false
>>>
>>> It fails because
>>>     RBMethodNode>>defines:
>>> only accepts the method's arguments as being defined by the method
>>> node, nt its temporaries.  It passes the
>>>     RBMethodNode>>references:
>>> call toits body (which is an RBSequenceNode) and
>>>     RBSequenceNode>>references:
>>> only asks its statements what it references, not its temporaries.
>>> Instead
>>>     RBSequenceNode>>defines:
>>> looks at temporaries.  Thus the test
>>>     (oldNode references: node name) or: [oldNode defines: node name]
>>> almost always works nevertheless because usually either (a) the match
>>> code is
>>>     | `@temps |
>>>     ``@.stats
>>> (i.e. without method name, i.e. with the All Method box unticked) and
>>> then
>>>     RBSequenceNode>>defines:
>>> finds the temp expression, or else (b) if a method defines a temp then
>>> its statements reference it, and if a specific temp is replaced by a
>>> pattern then the same pattern will be in the statements replacing its
>>> reference.
>>>
>>> Joachim's code fails because in
>>>
>>> someMethodName
>>>     | `@temps |
>>>     ``@.stats
>>>
>>> (BTW there is no need for a second ` in `@temps - you can't recurse
>>> into a temp declaration) the checking code is sent to an RBMethodNode
>>> and its #defines: does not look at temps, while its #references: asks
>>> the body which only looks in the statements and `@temps is not
>>> mentioned in ``@.stats (of course).
>>


______________________________________________________________________
This email has been scanned by the Symantec Email Security.cloud service.
For more information please visit http://www.symanteccloud.com
______________________________________________________________________




--
Solveig Viste
Instantiations
VA Smalltalk Support
http://st.instantiations.com


--
You received this message because you are subscribed to the Google Groups "VA Smalltalk" group.
To post to this group, send email to [hidden email].
To unsubscribe from this group, send email to [hidden email].
For more options, visit this group at http://groups.google.com/group/va-smalltalk?hl=en.