Re: PetitJava

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

Re: PetitJava

Uko2
Hi,

so there are a few parts.

1) Visitors
I suggest that abstract visitor should guide you how to visit this node. For example just do 'visitNodes: aPrimaryWithSelectorsNode codeList' so when someone will overwrite it he will know that he should pay attention to codeList data. Also comment will be lovely to have and this is a tough part as almost no classes in PJ have comments.

2). AST nodes
The reason why we create an AST is that PetitParser's native parse result are arrays, and we want more structured data. For instance this.getCommand().getContext().getUser().getActiveProfile() is a method call. It has a selector "getActiveProfile" it has no parameters and it's invoked on this.getCommand().getContext().getUser(). I may be wrong about this, but on the other hand current AST is constructed like this. So maybe if you need some other kind of AST you should create it and make one more subclass of parser that will construct that.

Also I'll sent a copy to moose-dev because maybe people there will have better vision.

Cheers
Uko 


On 29 трав. 2013, at 19:33, Chris Cunningham <[hidden email]> wrote:

Hi,

I'm not sure what exactly you want out of the visitor pattern.  It seems like the current accept.. methods (that aren't either 'TO DO' or #subclassResponsibility, that is) just ask you to #visitNode: or #visitNodes: for the various instance variables in the AST classes.  Am I missing something here?  I must say that I haven't used the visitor pattern with parsed results before, and am not sure what  you want and/or need out of this.

As for PrimaryWithSelectorsNode, after looking back at it, I can see that I have obscured what the 'primary' was from when it was parsed out.   however, in looking at what could constitute a primary (versus the selectors part), I find myself confused with what answering a primary would be.  I could by most anything - for a single item (boolean, literal, variable) to something very complex (such as a string of identifiers hanging off of an identifier with or without message sends on the end).  Instead I attempted to simplify it into an array of the parts so that I could iterate over them as needed.  sometimes I would like to find the first in the list, sometimes the last, and sometimes I'd like to search the whole list for a specific call (if present) in the middle.

so, take the code:
    this.getCommand().getContext().getUser().getActiveProfile()
The current PJPrimaryWithSelectorsNode will have these in the codeList array:
  this
  getCommand()
  getContext()
  getUser()
  getActiveProfile()
I did change the behaviour of the parsed #primaryWithselectors - previously it would have bundled this and getCommand() together into one unit identified as 'primary' and all of the other methods as an array of 'selectors'.

similarly, this code:
  new SearchUrl(Search.class).set(parm1, p1).set(parm2, params.get2(p2)).set(parm3, p3).getHref()
will return the codeList array:
  new SearchUrl(Search.class)
  set(parm1, p1)
  set(parm2, params.get2(p2))
  set(parm3, p3).getHref()
Previously, it would identify the first item of the array as the primary, and the rest as the selectors.  Which does make a lot of sense to me.

finally, this code:
  this.context.getUser().getActiveProfile().getProfileProperties()
will return in the code array:
  this
  context
  getUser()
  getActiveProfile()
  getProfileProperties()
Previously, primary would have consisted of the array 
  this
  context 
  getUser()
while selectors would have been the array of
  getActiveProfile()
  getProfileProperties()

I should note that I'm not a Java coder myself and am not clear on how Java coders identify the parts of their code.  I have tried to mostly follow what was previously there in the parser as it was clearer than what I'd likely come up with.  However, I can't really see myself why the 'primary' was primary in the previous examples - is it clear and I should revert back, and have the visitor visit the restored primary and selectors? or should I have the visitor visit each part of the stacked structure that the Java coder has presented us?  Which way would you prefer it - I'll modify it to fit your desires.

Thanks,
Chris

On Tue, May 28, 2013 at 10:55 PM, Yuriy Tymchuk <[hidden email]> wrote:
HI,

I've checked the changes. `acceptPrimaryWithSelectorsNode:` has only a flag so I didn't get how should I accept it and started to check out what the idea is. I still don't get it. It has some code list that is usually an array. Why do we need that node? What code entity does it represent? It looks like instead of building an AST from parsed arrays we are wrapping them in other classes.

Yuriy

On 28 трав. 2013, at 21:30, Chris Cunningham <[hidden email]> wrote:

Hi.

I've added the missing method (as well as related missing #acceptVisitor: methods on most of the other nodes that I've added) in the latest change.

If you have any other questions or requests, please let me know.

Thanks,
Chris Cunningham


On Tue, May 28, 2013 at 7:45 AM, Yuriy Tymchuk <[hidden email]> wrote:
Thank you

Надіслано з iPhone

28 трав. 2013 о 17:18 Chris <[hidden email]> написав(ла):

> Thanks for letting me know.  I'll fix that today.
>
> Sent from my iPhone
>
> On May 28, 2013, at 1:59 AM, Yuriy Tymchuk <[hidden email]> wrote:
>
>> Hi,
>>
>> your changes to PetitJava break my builds on fast. The problem is that `acceptVisitor:` of PJPrimaryWithSelectorsNode is not implementing `acceptVisitor: aVisitor` method, and so PJASTNodeVisitor is not implementing some visiting method that can give a hint on what should I do in my visitor subclass.
>>
>> Thank you for your contributions.
>> Uko





_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
Reply | Threaded
Open this post in threaded view
|

Re: PetitJava

Tudor Girba-2
Hi,

First, thanks for having this discussion on the mailing list. Please keep it like that :).

I looked briefly at the code and here are some remarks:


- The PJASTNodeVisitor has confusing methods names (like they are also confusing in the RB visitor). For example, the method acceptArgumentsNode: should be called visitArgumentsNode:. That is because the Visitor visits nodes, and the subject accepts them.


- visitNode: should be renamed into visit:, and implement another visitNode: for visiting the root AST node:

PJASTNode>>acceptVisitor: aVisitor
        ^self subclassResponsibility

be:
PJASTNode>>acceptVisitor: aVisitor
        ^aVisitor visitNode: self


- Ideally, the Visitor methods should mirror the AST hierarchy. For example:
acceptTypeArguments: aTypeArguments
        self flag: 'TODO'

should be:
visitTypeArguments: aTypeArguments
        ^ self visitArgumentsNode: aTypeArguments

Like this, I can subclass the visitor and provide a default behavior in visitNode: and only override a couple of other visiting methods where needed.


Cheers,
Doru


On May 30, 2013, at 11:14 AM, Yuriy Tymchuk <[hidden email]> wrote:

> Hi,
>
> so there are a few parts.
>
> 1) Visitors
> I suggest that abstract visitor should guide you how to visit this node. For example just do 'visitNodes: aPrimaryWithSelectorsNode codeList' so when someone will overwrite it he will know that he should pay attention to codeList data. Also comment will be lovely to have and this is a tough part as almost no classes in PJ have comments.
>
> 2). AST nodes
> The reason why we create an AST is that PetitParser's native parse result are arrays, and we want more structured data. For instance this.getCommand().getContext().getUser().getActiveProfile() is a method call. It has a selector "getActiveProfile" it has no parameters and it's invoked on this.getCommand().getContext().getUser(). I may be wrong about this, but on the other hand current AST is constructed like this. So maybe if you need some other kind of AST you should create it and make one more subclass of parser that will construct that.
>
> Also I'll sent a copy to moose-dev because maybe people there will have better vision.
>
> Cheers
> Uko
>
>
> On 29 трав. 2013, at 19:33, Chris Cunningham <[hidden email]> wrote:
>
>> Hi,
>>
>> I'm not sure what exactly you want out of the visitor pattern.  It seems like the current accept.. methods (that aren't either 'TO DO' or #subclassResponsibility, that is) just ask you to #visitNode: or #visitNodes: for the various instance variables in the AST classes.  Am I missing something here?  I must say that I haven't used the visitor pattern with parsed results before, and am not sure what  you want and/or need out of this.
>>
>> As for PrimaryWithSelectorsNode, after looking back at it, I can see that I have obscured what the 'primary' was from when it was parsed out.   however, in looking at what could constitute a primary (versus the selectors part), I find myself confused with what answering a primary would be.  I could by most anything - for a single item (boolean, literal, variable) to something very complex (such as a string of identifiers hanging off of an identifier with or without message sends on the end).  Instead I attempted to simplify it into an array of the parts so that I could iterate over them as needed.  sometimes I would like to find the first in the list, sometimes the last, and sometimes I'd like to search the whole list for a specific call (if present) in the middle.
>>
>> so, take the code:
>>     this.getCommand().getContext().getUser().getActiveProfile()
>> The current PJPrimaryWithSelectorsNode will have these in the codeList array:
>>   this
>>   getCommand()
>>   getContext()
>>   getUser()
>>   getActiveProfile()
>> I did change the behaviour of the parsed #primaryWithselectors - previously it would have bundled this and getCommand() together into one unit identified as 'primary' and all of the other methods as an array of 'selectors'.
>>
>> similarly, this code:
>>   new SearchUrl(Search.class).set(parm1, p1).set(parm2, params.get2(p2)).set(parm3, p3).getHref()
>> will return the codeList array:
>>   new SearchUrl(Search.class)
>>   set(parm1, p1)
>>   set(parm2, params.get2(p2))
>>   set(parm3, p3).getHref()
>> Previously, it would identify the first item of the array as the primary, and the rest as the selectors.  Which does make a lot of sense to me.
>>
>> finally, this code:
>>   this.context.getUser().getActiveProfile().getProfileProperties()
>> will return in the code array:
>>   this
>>   context
>>   getUser()
>>   getActiveProfile()
>>   getProfileProperties()
>> Previously, primary would have consisted of the array
>>   this
>>   context
>>   getUser()
>> while selectors would have been the array of
>>   getActiveProfile()
>>   getProfileProperties()
>>
>> I should note that I'm not a Java coder myself and am not clear on how Java coders identify the parts of their code.  I have tried to mostly follow what was previously there in the parser as it was clearer than what I'd likely come up with.  However, I can't really see myself why the 'primary' was primary in the previous examples - is it clear and I should revert back, and have the visitor visit the restored primary and selectors? or should I have the visitor visit each part of the stacked structure that the Java coder has presented us?  Which way would you prefer it - I'll modify it to fit your desires.
>>
>> Thanks,
>> Chris
>>
>> On Tue, May 28, 2013 at 10:55 PM, Yuriy Tymchuk <[hidden email]> wrote:
>> HI,
>>
>> I've checked the changes. `acceptPrimaryWithSelectorsNode:` has only a flag so I didn't get how should I accept it and started to check out what the idea is. I still don't get it. It has some code list that is usually an array. Why do we need that node? What code entity does it represent? It looks like instead of building an AST from parsed arrays we are wrapping them in other classes.
>>
>> Yuriy
>>
>> On 28 трав. 2013, at 21:30, Chris Cunningham <[hidden email]> wrote:
>>
>>> Hi.
>>>
>>> I've added the missing method (as well as related missing #acceptVisitor: methods on most of the other nodes that I've added) in the latest change.
>>>
>>> If you have any other questions or requests, please let me know.
>>>
>>> Thanks,
>>> Chris Cunningham
>>>
>>>
>>> On Tue, May 28, 2013 at 7:45 AM, Yuriy Tymchuk <[hidden email]> wrote:
>>> Thank you
>>>
>>> Надіслано з iPhone
>>>
>>> 28 трав. 2013 о 17:18 Chris <[hidden email]> написав(ла):
>>>
>>> > Thanks for letting me know.  I'll fix that today.
>>> >
>>> > Sent from my iPhone
>>> >
>>> > On May 28, 2013, at 1:59 AM, Yuriy Tymchuk <[hidden email]> wrote:
>>> >
>>> >> Hi,
>>> >>
>>> >> your changes to PetitJava break my builds on fast. The problem is that `acceptVisitor:` of PJPrimaryWithSelectorsNode is not implementing `acceptVisitor: aVisitor` method, and so PJASTNodeVisitor is not implementing some visiting method that can give a hint on what should I do in my visitor subclass.
>>> >>
>>> >> Thank you for your contributions.
>>> >> Uko
>>>
>>
>>
>
> _______________________________________________
> Moose-dev mailing list
> [hidden email]
> https://www.iam.unibe.ch/mailman/listinfo/moose-dev

--
www.tudorgirba.com

"Every now and then stop and ask yourself if the war you're fighting is the right one."




_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
Reply | Threaded
Open this post in threaded view
|

Re: PetitJava

Uko2
Yes, so we both agree on the last part of your message that visiting methods
of the general visitor should visit contents of the node it accepts.

Now second thing is about the method names. It makes sense that visitors
visit and nodes accept. But I think that we should change that once and for
all through all system.

And then there is still and open question: "should we model AST as method
invocation that has a receiver that is method invocation itself etc… or we
should put that all into a node that is actually an Array wrapper?"

Cheers.
Uko



--
View this message in context: http://moose-dev.97923.n3.nabble.com/Re-PetitJava-tp4027589p4027625.html
Sent from the moose-dev mailing list archive at Nabble.com.

_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
Reply | Threaded
Open this post in threaded view
|

Re: PetitJava

Tudor Girba-2
Hi,


On Fri, May 31, 2013 at 12:20 PM, Yuriy Tymchuk <[hidden email]> wrote:
Yes, so we both agree on the last part of your message that visiting methods
of the general visitor should visit contents of the node it accepts.

Now second thing is about the method names. It makes sense that visitors
visit and nodes accept. But I think that we should change that once and for
all through all system.

Yes, it would be good. But, for that to happen, we should start somewhere. So, let's just start with PetitJava :)
 
And then there is still and open question: "should we model AST as method
invocation that has a receiver that is method invocation itself etc… or we
should put that all into a node that is actually an Array wrapper?"

I do not understand the question. Can you provide more info?

Cheers,
Doru

 
Cheers.
Uko



--
View this message in context: http://moose-dev.97923.n3.nabble.com/Re-PetitJava-tp4027589p4027625.html
Sent from the moose-dev mailing list archive at Nabble.com.

_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev



--

"Every thing has its own flow"

_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
Reply | Threaded
Open this post in threaded view
|

Re: PetitJava

Uko2

On 31 трав. 2013, at 14:09, "Tudor Girba-2 [via moose-dev]" <[hidden email]> wrote:

Hi,


On Fri, May 31, 2013 at 12:20 PM, Yuriy Tymchuk <<a href="x-msg://609/user/SendEmail.jtp?type=node&amp;node=4027627&amp;i=0" target="_top" rel="nofollow" link="external">[hidden email]> wrote:
Yes, so we both agree on the last part of your message that visiting methods
of the general visitor should visit contents of the node it accepts.

Now second thing is about the method names. It makes sense that visitors
visit and nodes accept. But I think that we should change that once and for
all through all system.

Yes, it would be good. But, for that to happen, we should start somewhere. So, let's just start with PetitJava :)
 
And then there is still and open question: "should we model AST as method
invocation that has a receiver that is method invocation itself etc… or we
should put that all into a node that is actually an Array wrapper?"

I do not understand the question. Can you provide more info?

Sure.

So Chris introduced a new AST node PJPrimaryWithSelectorsNode that has and array attribute called codeList and when you parse a code like: this.getCommand().getContext().getUser().getActiveProfile() a PJPrimaryWithSelectorsNode is created and codeList is filled with:
this, getCommand(), getContext(), getUser(), getActiveProfile().

Now I tell that is you parse this.getCommand().getContext().getUser().getActiveProfile() a method invocation is created. It has a selector "getActiveProfile" it has no parameters and it's invoked on this.getCommand().getContext().getUser() that is one more method invocation node.

As we are 2 persons with different ideas, I posted this on the Moose-dev list, so more people can debate on how do we define AST.

Uko


Cheers,
Doru

 
Cheers.
Uko



--
View this message in context: http://moose-dev.97923.n3.nabble.com/Re-PetitJava-tp4027589p4027625.html
Sent from the moose-dev mailing list archive at Nabble.com.

_______________________________________________
Moose-dev mailing list
<a href="x-msg://609/user/SendEmail.jtp?type=node&amp;node=4027627&amp;i=1" target="_top" rel="nofollow" link="external">[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev



--

"Every thing has its own flow"

_______________________________________________
Moose-dev mailing list
<a href="x-msg://609/user/SendEmail.jtp?type=node&amp;node=4027627&amp;i=2" target="_top" rel="nofollow" link="external">[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev



If you reply to this email, your message will be added to the discussion below:
http://moose-dev.97923.n3.nabble.com/Re-PetitJava-tp4027589p4027627.html
To unsubscribe from Re: PetitJava, click here.
NAML



View this message in context: Re: PetitJava
Sent from the moose-dev mailing list archive at Nabble.com.

_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
cbc
Reply | Threaded
Open this post in threaded view
|

Re: PetitJava

cbc

On Fri, May 31, 2013 at 5:04 AM, Yuriy Tymchuk <[hidden email]> wrote:

On 31 трав. 2013, at 14:09, "Tudor Girba-2 [via moose-dev]" <[hidden email]> wrote:

Hi,


On Fri, May 31, 2013 at 12:20 PM, Yuriy Tymchuk <<a href="x-msg://609/user/SendEmail.jtp?type=node&amp;node=4027627&amp;i=0" target="_top" rel="nofollow" link="external">[hidden email]> wrote:
Yes, so we both agree on the last part of your message that visiting methods
of the general visitor should visit contents of the node it accepts.

Now second thing is about the method names. It makes sense that visitors
visit and nodes accept. But I think that we should change that once and for
all through all system.

Yes, it would be good. But, for that to happen, we should start somewhere. So, let's just start with PetitJava :)
 
And then there is still and open question: "should we model AST as method
invocation that has a receiver that is method invocation itself etc… or we
should put that all into a node that is actually an Array wrapper?"

I do not understand the question. Can you provide more info?

Sure.

So Chris introduced a new AST node PJPrimaryWithSelectorsNode that has and array attribute called codeList and when you parse a code like: this.getCommand().getContext().getUser().getActiveProfile() a PJPrimaryWithSelectorsNode is created and codeList is filled with:
this, getCommand(), getContext(), getUser(), getActiveProfile().

Now I tell that is you parse this.getCommand().getContext().getUser().getActiveProfile() a method invocation is created. It has a selector "getActiveProfile" it has no parameters and it's invoked on this.getCommand().getContext().getUser() that is one more method invocation node.

Uko, I like this idea a lot - certainly better than sticking with PrimaryWithSelectors, and better than my array.  I'll instead have #primaryWithselectors build AST nodes called PJMethodInvocationNode (or PJVariableAccessorNode for variable access such as this.context) and have the receiver on the methodInvocation be, potentially, another method invocation that may recurse a while (3 times in this example).  I think that actually represents what's going on much more clearly.

And, unless you get to it first, I'll also make the visitor changes that Doru suggested.  I'll work on that this weekend - if you'd like to go a different way (or are already doing some of this), let me know.

-Chris
As we are 2 persons with different ideas, I posted this on the Moose-dev list, so more people can debate on how do we define AST.

Uko


Cheers,
Doru

 
Cheers.
Uko



--
View this message in context: http://moose-dev.97923.n3.nabble.com/Re-PetitJava-tp4027589p4027625.html
Sent from the moose-dev mailing list archive at Nabble.com.

_______________________________________________
Moose-dev mailing list
<a href="x-msg://609/user/SendEmail.jtp?type=node&amp;node=4027627&amp;i=1" target="_top" rel="nofollow" link="external">[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev



--

"Every thing has its own flow"

_______________________________________________
Moose-dev mailing list
<a href="x-msg://609/user/SendEmail.jtp?type=node&amp;node=4027627&amp;i=2" target="_top" rel="nofollow" link="external">[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev



If you reply to this email, your message will be added to the discussion below:
http://moose-dev.97923.n3.nabble.com/Re-PetitJava-tp4027589p4027627.html
To unsubscribe from Re: PetitJava, click here.
NAML



View this message in context: Re: PetitJava

Sent from the moose-dev mailing list archive at Nabble.com.

_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev



_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
Reply | Threaded
Open this post in threaded view
|

Re: PetitJava

Uko2
Yes Chris, I'll be thankful for your work. I haven't done anything yet, and when I'll start to do something I'll send you a letter.

Uko

Надіслано з iPhone

1 черв. 2013 о 00:46 "Chris Cunningham [via moose-dev]" <[hidden email]> написав(ла):


On Fri, May 31, 2013 at 5:04 AM, Yuriy Tymchuk <[hidden email]> wrote:

On 31 трав. 2013, at 14:09, "Tudor Girba-2 [via moose-dev]" <[hidden email]> wrote:

Hi,


On Fri, May 31, 2013 at 12:20 PM, Yuriy Tymchuk <<a href="x-msg://609/user/SendEmail.jtp?type=node&amp;node=4027627&amp;i=0" target="_top" rel="nofollow" link="external">[hidden email]> wrote:
Yes, so we both agree on the last part of your message that visiting methods
of the general visitor should visit contents of the node it accepts.

Now second thing is about the method names. It makes sense that visitors
visit and nodes accept. But I think that we should change that once and for
all through all system.

Yes, it would be good. But, for that to happen, we should start somewhere. So, let's just start with PetitJava :)
 
And then there is still and open question: "should we model AST as method
invocation that has a receiver that is method invocation itself etc… or we
should put that all into a node that is actually an Array wrapper?"

I do not understand the question. Can you provide more info?

Sure.

So Chris introduced a new AST node PJPrimaryWithSelectorsNode that has and array attribute called codeList and when you parse a code like: this.getCommand().getContext().getUser().getActiveProfile() a PJPrimaryWithSelectorsNode is created and codeList is filled with:
this, getCommand(), getContext(), getUser(), getActiveProfile().

Now I tell that is you parse this.getCommand().getContext().getUser().getActiveProfile() a method invocation is created. It has a selector "getActiveProfile" it has no parameters and it's invoked on this.getCommand().getContext().getUser() that is one more method invocation node.

Uko, I like this idea a lot - certainly better than sticking with PrimaryWithSelectors, and better than my array.  I'll instead have #primaryWithselectors build AST nodes called PJMethodInvocationNode (or PJVariableAccessorNode for variable access such as this.context) and have the receiver on the methodInvocation be, potentially, another method invocation that may recurse a while (3 times in this example).  I think that actually represents what's going on much more clearly.

And, unless you get to it first, I'll also make the visitor changes that Doru suggested.  I'll work on that this weekend - if you'd like to go a different way (or are already doing some of this), let me know.

-Chris
As we are 2 persons with different ideas, I posted this on the Moose-dev list, so more people can debate on how do we define AST.

Uko


Cheers,
Doru

 
Cheers.
Uko



--
View this message in context: http://moose-dev.97923.n3.nabble.com/Re-PetitJava-tp4027589p4027625.html
Sent from the moose-dev mailing list archive at Nabble.com.

_______________________________________________
Moose-dev mailing list
<a href="x-msg://609/user/SendEmail.jtp?type=node&amp;node=4027627&amp;i=1" target="_top" rel="nofollow" link="external">[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev



--

"Every thing has its own flow"

_______________________________________________
Moose-dev mailing list
<a href="x-msg://609/user/SendEmail.jtp?type=node&amp;node=4027627&amp;i=2" target="_top" rel="nofollow" link="external">[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev



If you reply to this email, your message will be added to the discussion below:
http://moose-dev.97923.n3.nabble.com/Re-PetitJava-tp4027589p4027627.html
To unsubscribe from Re: PetitJava, click here.
NAML



View this message in context: Re: PetitJava
_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev



_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev



If you reply to this email, your message will be added to the discussion below:
http://moose-dev.97923.n3.nabble.com/Re-PetitJava-tp4027589p4027639.html
To unsubscribe from Re: PetitJava, click here.
NAML


View this message in context: Re: PetitJava
Sent from the moose-dev mailing list archive at Nabble.com.

_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
Reply | Threaded
Open this post in threaded view
|

Re: PetitJava

Stéphane Ducasse
In reply to this post by Tudor Girba-2

On May 31, 2013, at 7:09 AM, Tudor Girba <[hidden email]> wrote:

> Hi,
>
> First, thanks for having this discussion on the mailing list. Please keep it like that :).
>
> I looked briefly at the code and here are some remarks:
>
>
> - The PJASTNodeVisitor has confusing methods names (like they are also confusing in the RB visitor).

not anymore :)
In 30 the RB visitor is clean and nice. The AST was cleaned too :)


> For example, the method acceptArgumentsNode: should be called visitArgumentsNode:. That is because the Visitor visits nodes, and the subject accepts them.

Yes and now this is like that in Pharo too ;)


> - visitNode: should be renamed into visit:, and implement another visitNode: for visiting the root AST node:
>
> PJASTNode>>acceptVisitor: aVisitor
> ^self subclassResponsibility
>
> be:
> PJASTNode>>acceptVisitor: aVisitor
> ^aVisitor visitNode: self
>
>
> - Ideally, the Visitor methods should mirror the AST hierarchy. For example:
> acceptTypeArguments: aTypeArguments
> self flag: 'TODO'
>
> should be:
> visitTypeArguments: aTypeArguments
> ^ self visitArgumentsNode: aTypeArguments

Why that? Why do you double all the visit method with Nodes: ?

>
> Like this, I can subclass the visitor and provide a default behavior in visitNode: and only override a couple of other visiting methods where needed.
>
>
> Cheers,
> Doru
>
>
> On May 30, 2013, at 11:14 AM, Yuriy Tymchuk <[hidden email]> wrote:
>
>> Hi,
>>
>> so there are a few parts.
>>
>> 1) Visitors
>> I suggest that abstract visitor should guide you how to visit this node. For example just do 'visitNodes: aPrimaryWithSelectorsNode codeList' so when someone will overwrite it he will know that he should pay attention to codeList data. Also comment will be lovely to have and this is a tough part as almost no classes in PJ have comments.
>>
>> 2). AST nodes
>> The reason why we create an AST is that PetitParser's native parse result are arrays, and we want more structured data. For instance this.getCommand().getContext().getUser().getActiveProfile() is a method call. It has a selector "getActiveProfile" it has no parameters and it's invoked on this.getCommand().getContext().getUser(). I may be wrong about this, but on the other hand current AST is constructed like this. So maybe if you need some other kind of AST you should create it and make one more subclass of parser that will construct that.
>>
>> Also I'll sent a copy to moose-dev because maybe people there will have better vision.
>>
>> Cheers
>> Uko
>>
>>
>> On 29 трав. 2013, at 19:33, Chris Cunningham <[hidden email]> wrote:
>>
>>> Hi,
>>>
>>> I'm not sure what exactly you want out of the visitor pattern.  It seems like the current accept.. methods (that aren't either 'TO DO' or #subclassResponsibility, that is) just ask you to #visitNode: or #visitNodes: for the various instance variables in the AST classes.  Am I missing something here?  I must say that I haven't used the visitor pattern with parsed results before, and am not sure what  you want and/or need out of this.
>>>
>>> As for PrimaryWithSelectorsNode, after looking back at it, I can see that I have obscured what the 'primary' was from when it was parsed out.   however, in looking at what could constitute a primary (versus the selectors part), I find myself confused with what answering a primary would be.  I could by most anything - for a single item (boolean, literal, variable) to something very complex (such as a string of identifiers hanging off of an identifier with or without message sends on the end).  Instead I attempted to simplify it into an array of the parts so that I could iterate over them as needed.  sometimes I would like to find the first in the list, sometimes the last, and sometimes I'd like to search the whole list for a specific call (if present) in the middle.
>>>
>>> so, take the code:
>>>    this.getCommand().getContext().getUser().getActiveProfile()
>>> The current PJPrimaryWithSelectorsNode will have these in the codeList array:
>>>  this
>>>  getCommand()
>>>  getContext()
>>>  getUser()
>>>  getActiveProfile()
>>> I did change the behaviour of the parsed #primaryWithselectors - previously it would have bundled this and getCommand() together into one unit identified as 'primary' and all of the other methods as an array of 'selectors'.
>>>
>>> similarly, this code:
>>>  new SearchUrl(Search.class).set(parm1, p1).set(parm2, params.get2(p2)).set(parm3, p3).getHref()
>>> will return the codeList array:
>>>  new SearchUrl(Search.class)
>>>  set(parm1, p1)
>>>  set(parm2, params.get2(p2))
>>>  set(parm3, p3).getHref()
>>> Previously, it would identify the first item of the array as the primary, and the rest as the selectors.  Which does make a lot of sense to me.
>>>
>>> finally, this code:
>>>  this.context.getUser().getActiveProfile().getProfileProperties()
>>> will return in the code array:
>>>  this
>>>  context
>>>  getUser()
>>>  getActiveProfile()
>>>  getProfileProperties()
>>> Previously, primary would have consisted of the array
>>>  this
>>>  context
>>>  getUser()
>>> while selectors would have been the array of
>>>  getActiveProfile()
>>>  getProfileProperties()
>>>
>>> I should note that I'm not a Java coder myself and am not clear on how Java coders identify the parts of their code.  I have tried to mostly follow what was previously there in the parser as it was clearer than what I'd likely come up with.  However, I can't really see myself why the 'primary' was primary in the previous examples - is it clear and I should revert back, and have the visitor visit the restored primary and selectors? or should I have the visitor visit each part of the stacked structure that the Java coder has presented us?  Which way would you prefer it - I'll modify it to fit your desires.
>>>
>>> Thanks,
>>> Chris
>>>
>>> On Tue, May 28, 2013 at 10:55 PM, Yuriy Tymchuk <[hidden email]> wrote:
>>> HI,
>>>
>>> I've checked the changes. `acceptPrimaryWithSelectorsNode:` has only a flag so I didn't get how should I accept it and started to check out what the idea is. I still don't get it. It has some code list that is usually an array. Why do we need that node? What code entity does it represent? It looks like instead of building an AST from parsed arrays we are wrapping them in other classes.
>>>
>>> Yuriy
>>>
>>> On 28 трав. 2013, at 21:30, Chris Cunningham <[hidden email]> wrote:
>>>
>>>> Hi.
>>>>
>>>> I've added the missing method (as well as related missing #acceptVisitor: methods on most of the other nodes that I've added) in the latest change.
>>>>
>>>> If you have any other questions or requests, please let me know.
>>>>
>>>> Thanks,
>>>> Chris Cunningham
>>>>
>>>>
>>>> On Tue, May 28, 2013 at 7:45 AM, Yuriy Tymchuk <[hidden email]> wrote:
>>>> Thank you
>>>>
>>>> Надіслано з iPhone
>>>>
>>>> 28 трав. 2013 о 17:18 Chris <[hidden email]> написав(ла):
>>>>
>>>>> Thanks for letting me know.  I'll fix that today.
>>>>>
>>>>> Sent from my iPhone
>>>>>
>>>>> On May 28, 2013, at 1:59 AM, Yuriy Tymchuk <[hidden email]> wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> your changes to PetitJava break my builds on fast. The problem is that `acceptVisitor:` of PJPrimaryWithSelectorsNode is not implementing `acceptVisitor: aVisitor` method, and so PJASTNodeVisitor is not implementing some visiting method that can give a hint on what should I do in my visitor subclass.
>>>>>>
>>>>>> Thank you for your contributions.
>>>>>> Uko
>>>>
>>>
>>>
>>
>> _______________________________________________
>> Moose-dev mailing list
>> [hidden email]
>> https://www.iam.unibe.ch/mailman/listinfo/moose-dev
>
> --
> www.tudorgirba.com
>
> "Every now and then stop and ask yourself if the war you're fighting is the right one."
>
>
>
>
> _______________________________________________
> Moose-dev mailing list
> [hidden email]
> https://www.iam.unibe.ch/mailman/listinfo/moose-dev


_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
Reply | Threaded
Open this post in threaded view
|

Re: PetitJava

Tudor Girba-2
Hi,

On Jun 1, 2013, at 11:24 PM, Stéphane Ducasse <[hidden email]> wrote:

>
> On May 31, 2013, at 7:09 AM, Tudor Girba <[hidden email]> wrote:
>
>> Hi,
>>
>> First, thanks for having this discussion on the mailing list. Please keep it like that :).
>>
>> I looked briefly at the code and here are some remarks:
>>
>>
>> - The PJASTNodeVisitor has confusing methods names (like they are also confusing in the RB visitor).
>
> not anymore :)
> In 30 the RB visitor is clean and nice. The AST was cleaned too :)

Great.
[ ... ]

>> - Ideally, the Visitor methods should mirror the AST hierarchy. For example:
>> acceptTypeArguments: aTypeArguments
>> self flag: 'TODO'
>>
>> should be:
>> visitTypeArguments: aTypeArguments
>> ^ self visitArgumentsNode: aTypeArguments
>
> Why that? Why do you double all the visit method with Nodes: ?

I meant to say that to code should be:

visitTypeArguments: aTypeArguments
        ^ self visitArguments: aTypeArguments

so that the specific visit method calls by default the more generic one by following the AST hierarchy.

Cheers,
Doru

>
>>
>> Like this, I can subclass the visitor and provide a default behavior in visitNode: and only override a couple of other visiting methods where needed.
>>
>>
>> Cheers,
>> Doru
>>
>>
>> On May 30, 2013, at 11:14 AM, Yuriy Tymchuk <[hidden email]> wrote:
>>
>>> Hi,
>>>
>>> so there are a few parts.
>>>
>>> 1) Visitors
>>> I suggest that abstract visitor should guide you how to visit this node. For example just do 'visitNodes: aPrimaryWithSelectorsNode codeList' so when someone will overwrite it he will know that he should pay attention to codeList data. Also comment will be lovely to have and this is a tough part as almost no classes in PJ have comments.
>>>
>>> 2). AST nodes
>>> The reason why we create an AST is that PetitParser's native parse result are arrays, and we want more structured data. For instance this.getCommand().getContext().getUser().getActiveProfile() is a method call. It has a selector "getActiveProfile" it has no parameters and it's invoked on this.getCommand().getContext().getUser(). I may be wrong about this, but on the other hand current AST is constructed like this. So maybe if you need some other kind of AST you should create it and make one more subclass of parser that will construct that.
>>>
>>> Also I'll sent a copy to moose-dev because maybe people there will have better vision.
>>>
>>> Cheers
>>> Uko
>>>
>>>
>>> On 29 трав. 2013, at 19:33, Chris Cunningham <[hidden email]> wrote:
>>>
>>>> Hi,
>>>>
>>>> I'm not sure what exactly you want out of the visitor pattern.  It seems like the current accept.. methods (that aren't either 'TO DO' or #subclassResponsibility, that is) just ask you to #visitNode: or #visitNodes: for the various instance variables in the AST classes.  Am I missing something here?  I must say that I haven't used the visitor pattern with parsed results before, and am not sure what  you want and/or need out of this.
>>>>
>>>> As for PrimaryWithSelectorsNode, after looking back at it, I can see that I have obscured what the 'primary' was from when it was parsed out.   however, in looking at what could constitute a primary (versus the selectors part), I find myself confused with what answering a primary would be.  I could by most anything - for a single item (boolean, literal, variable) to something very complex (such as a string of identifiers hanging off of an identifier with or without message sends on the end).  Instead I attempted to simplify it into an array of the parts so that I could iterate over them as needed.  sometimes I would like to find the first in the list, sometimes the last, and sometimes I'd like to search the whole list for a specific call (if present) in the middle.
>>>>
>>>> so, take the code:
>>>>   this.getCommand().getContext().getUser().getActiveProfile()
>>>> The current PJPrimaryWithSelectorsNode will have these in the codeList array:
>>>> this
>>>> getCommand()
>>>> getContext()
>>>> getUser()
>>>> getActiveProfile()
>>>> I did change the behaviour of the parsed #primaryWithselectors - previously it would have bundled this and getCommand() together into one unit identified as 'primary' and all of the other methods as an array of 'selectors'.
>>>>
>>>> similarly, this code:
>>>> new SearchUrl(Search.class).set(parm1, p1).set(parm2, params.get2(p2)).set(parm3, p3).getHref()
>>>> will return the codeList array:
>>>> new SearchUrl(Search.class)
>>>> set(parm1, p1)
>>>> set(parm2, params.get2(p2))
>>>> set(parm3, p3).getHref()
>>>> Previously, it would identify the first item of the array as the primary, and the rest as the selectors.  Which does make a lot of sense to me.
>>>>
>>>> finally, this code:
>>>> this.context.getUser().getActiveProfile().getProfileProperties()
>>>> will return in the code array:
>>>> this
>>>> context
>>>> getUser()
>>>> getActiveProfile()
>>>> getProfileProperties()
>>>> Previously, primary would have consisted of the array
>>>> this
>>>> context
>>>> getUser()
>>>> while selectors would have been the array of
>>>> getActiveProfile()
>>>> getProfileProperties()
>>>>
>>>> I should note that I'm not a Java coder myself and am not clear on how Java coders identify the parts of their code.  I have tried to mostly follow what was previously there in the parser as it was clearer than what I'd likely come up with.  However, I can't really see myself why the 'primary' was primary in the previous examples - is it clear and I should revert back, and have the visitor visit the restored primary and selectors? or should I have the visitor visit each part of the stacked structure that the Java coder has presented us?  Which way would you prefer it - I'll modify it to fit your desires.
>>>>
>>>> Thanks,
>>>> Chris
>>>>
>>>> On Tue, May 28, 2013 at 10:55 PM, Yuriy Tymchuk <[hidden email]> wrote:
>>>> HI,
>>>>
>>>> I've checked the changes. `acceptPrimaryWithSelectorsNode:` has only a flag so I didn't get how should I accept it and started to check out what the idea is. I still don't get it. It has some code list that is usually an array. Why do we need that node? What code entity does it represent? It looks like instead of building an AST from parsed arrays we are wrapping them in other classes.
>>>>
>>>> Yuriy
>>>>
>>>> On 28 трав. 2013, at 21:30, Chris Cunningham <[hidden email]> wrote:
>>>>
>>>>> Hi.
>>>>>
>>>>> I've added the missing method (as well as related missing #acceptVisitor: methods on most of the other nodes that I've added) in the latest change.
>>>>>
>>>>> If you have any other questions or requests, please let me know.
>>>>>
>>>>> Thanks,
>>>>> Chris Cunningham
>>>>>
>>>>>
>>>>> On Tue, May 28, 2013 at 7:45 AM, Yuriy Tymchuk <[hidden email]> wrote:
>>>>> Thank you
>>>>>
>>>>> Надіслано з iPhone
>>>>>
>>>>> 28 трав. 2013 о 17:18 Chris <[hidden email]> написав(ла):
>>>>>
>>>>>> Thanks for letting me know.  I'll fix that today.
>>>>>>
>>>>>> Sent from my iPhone
>>>>>>
>>>>>> On May 28, 2013, at 1:59 AM, Yuriy Tymchuk <[hidden email]> wrote:
>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> your changes to PetitJava break my builds on fast. The problem is that `acceptVisitor:` of PJPrimaryWithSelectorsNode is not implementing `acceptVisitor: aVisitor` method, and so PJASTNodeVisitor is not implementing some visiting method that can give a hint on what should I do in my visitor subclass.
>>>>>>>
>>>>>>> Thank you for your contributions.
>>>>>>> Uko
>>>>>
>>>>
>>>>
>>>
>>> _______________________________________________
>>> Moose-dev mailing list
>>> [hidden email]
>>> https://www.iam.unibe.ch/mailman/listinfo/moose-dev
>>
>> --
>> www.tudorgirba.com
>>
>> "Every now and then stop and ask yourself if the war you're fighting is the right one."
>>
>>
>>
>>
>> _______________________________________________
>> Moose-dev mailing list
>> [hidden email]
>> https://www.iam.unibe.ch/mailman/listinfo/moose-dev
>
>
> _______________________________________________
> Moose-dev mailing list
> [hidden email]
> https://www.iam.unibe.ch/mailman/listinfo/moose-dev

--
www.tudorgirba.com

"Some battles are better lost than fought."




_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
cbc
Reply | Threaded
Open this post in threaded view
|

Re: PetitJava

cbc
Hello,

I have just committed version 114 to SmalltalkHub that refactors the PrimaryWithSelectors into MethodInvocation and VariableAccessor.

This commit also changes the Visitor syntax to follow what was discussed in this email chain - at least to the level that I understood it (once I went back and read the discussion today, I had a harder time understanding what was said.  I think I'm following, but am not positive).  No other nodes in PetitJava visitors were altered in this commit.

About PJVariableAccessorNode - I like most of what is going on here.  However, things like this, super, and class also make it into a 'VariableAccessor', which I don't really think is right.  Maybe these should be a special subclass - where it has the actual type it is altering (if available) and the fact that it is addressing a special aspect of that type.  Or, is this sufficient for what you may be interested in?

-Chris


On Sat, Jun 1, 2013 at 2:54 PM, Tudor Girba <[hidden email]> wrote:
Hi,

On Jun 1, 2013, at 11:24 PM, Stéphane Ducasse <[hidden email]> wrote:

>
> On May 31, 2013, at 7:09 AM, Tudor Girba <[hidden email]> wrote:
>
>> Hi,
>>
>> First, thanks for having this discussion on the mailing list. Please keep it like that :).
>>
>> I looked briefly at the code and here are some remarks:
>>
>>
>> - The PJASTNodeVisitor has confusing methods names (like they are also confusing in the RB visitor).
>
> not anymore :)
> In 30 the RB visitor is clean and nice. The AST was cleaned too :)

Great.
[ ... ]

>> - Ideally, the Visitor methods should mirror the AST hierarchy. For example:
>> acceptTypeArguments: aTypeArguments
>>      self flag: 'TODO'
>>
>> should be:
>> visitTypeArguments: aTypeArguments
>>      ^ self visitArgumentsNode: aTypeArguments
>
> Why that? Why do you double all the visit method with Nodes: ?

I meant to say that to code should be:

visitTypeArguments: aTypeArguments
        ^ self visitArguments: aTypeArguments

so that the specific visit method calls by default the more generic one by following the AST hierarchy.

Cheers,
Doru

>
>>
>> Like this, I can subclass the visitor and provide a default behavior in visitNode: and only override a couple of other visiting methods where needed.
>>
>>
>> Cheers,
>> Doru
>>
>>
>> On May 30, 2013, at 11:14 AM, Yuriy Tymchuk <[hidden email]> wrote:
>>
>>> Hi,
>>>
>>> so there are a few parts.
>>>
>>> 1) Visitors
>>> I suggest that abstract visitor should guide you how to visit this node. For example just do 'visitNodes: aPrimaryWithSelectorsNode codeList' so when someone will overwrite it he will know that he should pay attention to codeList data. Also comment will be lovely to have and this is a tough part as almost no classes in PJ have comments.
>>>
>>> 2). AST nodes
>>> The reason why we create an AST is that PetitParser's native parse result are arrays, and we want more structured data. For instance this.getCommand().getContext().getUser().getActiveProfile() is a method call. It has a selector "getActiveProfile" it has no parameters and it's invoked on this.getCommand().getContext().getUser(). I may be wrong about this, but on the other hand current AST is constructed like this. So maybe if you need some other kind of AST you should create it and make one more subclass of parser that will construct that.
>>>
>>> Also I'll sent a copy to moose-dev because maybe people there will have better vision.
>>>
>>> Cheers
>>> Uko
>>>
>>>
>>> On 29 трав. 2013, at 19:33, Chris Cunningham <[hidden email]> wrote:
>>>
>>>> Hi,
>>>>
>>>> I'm not sure what exactly you want out of the visitor pattern.  It seems like the current accept.. methods (that aren't either 'TO DO' or #subclassResponsibility, that is) just ask you to #visitNode: or #visitNodes: for the various instance variables in the AST classes.  Am I missing something here?  I must say that I haven't used the visitor pattern with parsed results before, and am not sure what  you want and/or need out of this.
>>>>
>>>> As for PrimaryWithSelectorsNode, after looking back at it, I can see that I have obscured what the 'primary' was from when it was parsed out.   however, in looking at what could constitute a primary (versus the selectors part), I find myself confused with what answering a primary would be.  I could by most anything - for a single item (boolean, literal, variable) to something very complex (such as a string of identifiers hanging off of an identifier with or without message sends on the end).  Instead I attempted to simplify it into an array of the parts so that I could iterate over them as needed.  sometimes I would like to find the first in the list, sometimes the last, and sometimes I'd like to search the whole list for a specific call (if present) in the middle.
>>>>
>>>> so, take the code:
>>>>   this.getCommand().getContext().getUser().getActiveProfile()
>>>> The current PJPrimaryWithSelectorsNode will have these in the codeList array:
>>>> this
>>>> getCommand()
>>>> getContext()
>>>> getUser()
>>>> getActiveProfile()
>>>> I did change the behaviour of the parsed #primaryWithselectors - previously it would have bundled this and getCommand() together into one unit identified as 'primary' and all of the other methods as an array of 'selectors'.
>>>>
>>>> similarly, this code:
>>>> new SearchUrl(Search.class).set(parm1, p1).set(parm2, params.get2(p2)).set(parm3, p3).getHref()
>>>> will return the codeList array:
>>>> new SearchUrl(Search.class)
>>>> set(parm1, p1)
>>>> set(parm2, params.get2(p2))
>>>> set(parm3, p3).getHref()
>>>> Previously, it would identify the first item of the array as the primary, and the rest as the selectors.  Which does make a lot of sense to me.
>>>>
>>>> finally, this code:
>>>> this.context.getUser().getActiveProfile().getProfileProperties()
>>>> will return in the code array:
>>>> this
>>>> context
>>>> getUser()
>>>> getActiveProfile()
>>>> getProfileProperties()
>>>> Previously, primary would have consisted of the array
>>>> this
>>>> context
>>>> getUser()
>>>> while selectors would have been the array of
>>>> getActiveProfile()
>>>> getProfileProperties()
>>>>
>>>> I should note that I'm not a Java coder myself and am not clear on how Java coders identify the parts of their code.  I have tried to mostly follow what was previously there in the parser as it was clearer than what I'd likely come up with.  However, I can't really see myself why the 'primary' was primary in the previous examples - is it clear and I should revert back, and have the visitor visit the restored primary and selectors? or should I have the visitor visit each part of the stacked structure that the Java coder has presented us?  Which way would you prefer it - I'll modify it to fit your desires.
>>>>
>>>> Thanks,
>>>> Chris
>>>>
>>>> On Tue, May 28, 2013 at 10:55 PM, Yuriy Tymchuk <[hidden email]> wrote:
>>>> HI,
>>>>
>>>> I've checked the changes. `acceptPrimaryWithSelectorsNode:` has only a flag so I didn't get how should I accept it and started to check out what the idea is. I still don't get it. It has some code list that is usually an array. Why do we need that node? What code entity does it represent? It looks like instead of building an AST from parsed arrays we are wrapping them in other classes.
>>>>
>>>> Yuriy
>>>>
>>>> On 28 трав. 2013, at 21:30, Chris Cunningham <[hidden email]> wrote:
>>>>
>>>>> Hi.
>>>>>
>>>>> I've added the missing method (as well as related missing #acceptVisitor: methods on most of the other nodes that I've added) in the latest change.
>>>>>
>>>>> If you have any other questions or requests, please let me know.
>>>>>
>>>>> Thanks,
>>>>> Chris Cunningham
>>>>>
>>>>>
>>>>> On Tue, May 28, 2013 at 7:45 AM, Yuriy Tymchuk <[hidden email]> wrote:
>>>>> Thank you
>>>>>
>>>>> Надіслано з iPhone
>>>>>
>>>>> 28 трав. 2013 о 17:18 Chris <[hidden email]> написав(ла):
>>>>>
>>>>>> Thanks for letting me know.  I'll fix that today.
>>>>>>
>>>>>> Sent from my iPhone
>>>>>>
>>>>>> On May 28, 2013, at 1:59 AM, Yuriy Tymchuk <[hidden email]> wrote:
>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> your changes to PetitJava break my builds on fast. The problem is that `acceptVisitor:` of PJPrimaryWithSelectorsNode is not implementing `acceptVisitor: aVisitor` method, and so PJASTNodeVisitor is not implementing some visiting method that can give a hint on what should I do in my visitor subclass.
>>>>>>>
>>>>>>> Thank you for your contributions.
>>>>>>> Uko
>>>>>
>>>>
>>>>
>>>
>>> _______________________________________________
>>> Moose-dev mailing list
>>> [hidden email]
>>> https://www.iam.unibe.ch/mailman/listinfo/moose-dev
>>
>> --
>> www.tudorgirba.com
>>
>> "Every now and then stop and ask yourself if the war you're fighting is the right one."
>>
>>
>>
>>
>> _______________________________________________
>> Moose-dev mailing list
>> [hidden email]
>> https://www.iam.unibe.ch/mailman/listinfo/moose-dev
>
>
> _______________________________________________
> Moose-dev mailing list
> [hidden email]
> https://www.iam.unibe.ch/mailman/listinfo/moose-dev

--
www.tudorgirba.com

"Some battles are better lost than fought."




_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev


_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
Reply | Threaded
Open this post in threaded view
|

Re: PetitJava

Uko2
By the way, do you think we really need a `visit:` method? Because at the moment the only thing that `visitNode:` does is calling `visit:`. Maybe we can use only `visitNode:` and send `acceptVisitor:` from there? What do you think?

Надіслано з iPhone

3 черв. 2013 о 23:55 "Chris Cunningham [via moose-dev]" <[hidden email]> написав(ла):

Hello,

I have just committed version 114 to SmalltalkHub that refactors the PrimaryWithSelectors into MethodInvocation and VariableAccessor.

This commit also changes the Visitor syntax to follow what was discussed in this email chain - at least to the level that I understood it (once I went back and read the discussion today, I had a harder time understanding what was said.  I think I'm following, but am not positive).  No other nodes in PetitJava visitors were altered in this commit.

About PJVariableAccessorNode - I like most of what is going on here.  However, things like this, super, and class also make it into a 'VariableAccessor', which I don't really think is right.  Maybe these should be a special subclass - where it has the actual type it is altering (if available) and the fact that it is addressing a special aspect of that type.  Or, is this sufficient for what you may be interested in?

-Chris


On Sat, Jun 1, 2013 at 2:54 PM, Tudor Girba <[hidden email]> wrote:
Hi,

On Jun 1, 2013, at 11:24 PM, Stéphane Ducasse <[hidden email]> wrote:

>
> On May 31, 2013, at 7:09 AM, Tudor Girba <[hidden email]> wrote:
>
>> Hi,
>>
>> First, thanks for having this discussion on the mailing list. Please keep it like that :).
>>
>> I looked briefly at the code and here are some remarks:
>>
>>
>> - The PJASTNodeVisitor has confusing methods names (like they are also confusing in the RB visitor).
>
> not anymore :)
> In 30 the RB visitor is clean and nice. The AST was cleaned too :)

Great.
[ ... ]

>> - Ideally, the Visitor methods should mirror the AST hierarchy. For example:
>> acceptTypeArguments: aTypeArguments
>>      self flag: 'TODO'
>>
>> should be:
>> visitTypeArguments: aTypeArguments
>>      ^ self visitArgumentsNode: aTypeArguments
>
> Why that? Why do you double all the visit method with Nodes: ?

I meant to say that to code should be:

visitTypeArguments: aTypeArguments
        ^ self visitArguments: aTypeArguments

so that the specific visit method calls by default the more generic one by following the AST hierarchy.

Cheers,
Doru

>
>>
>> Like this, I can subclass the visitor and provide a default behavior in visitNode: and only override a couple of other visiting methods where needed.
>>
>>
>> Cheers,
>> Doru
>>
>>
>> On May 30, 2013, at 11:14 AM, Yuriy Tymchuk <[hidden email]> wrote:
>>
>>> Hi,
>>>
>>> so there are a few parts.
>>>
>>> 1) Visitors
>>> I suggest that abstract visitor should guide you how to visit this node. For example just do 'visitNodes: aPrimaryWithSelectorsNode codeList' so when someone will overwrite it he will know that he should pay attention to codeList data. Also comment will be lovely to have and this is a tough part as almost no classes in PJ have comments.
>>>
>>> 2). AST nodes
>>> The reason why we create an AST is that PetitParser's native parse result are arrays, and we want more structured data. For instance this.getCommand().getContext().getUser().getActiveProfile() is a method call. It has a selector "getActiveProfile" it has no parameters and it's invoked on this.getCommand().getContext().getUser(). I may be wrong about this, but on the other hand current AST is constructed like this. So maybe if you need some other kind of AST you should create it and make one more subclass of parser that will construct that.
>>>
>>> Also I'll sent a copy to moose-dev because maybe people there will have better vision.
>>>
>>> Cheers
>>> Uko
>>>
>>>
>>> On 29 трав. 2013, at 19:33, Chris Cunningham <[hidden email]> wrote:
>>>
>>>> Hi,
>>>>
>>>> I'm not sure what exactly you want out of the visitor pattern.  It seems like the current accept.. methods (that aren't either 'TO DO' or #subclassResponsibility, that is) just ask you to #visitNode: or #visitNodes: for the various instance variables in the AST classes.  Am I missing something here?  I must say that I haven't used the visitor pattern with parsed results before, and am not sure what  you want and/or need out of this.
>>>>
>>>> As for PrimaryWithSelectorsNode, after looking back at it, I can see that I have obscured what the 'primary' was from when it was parsed out.   however, in looking at what could constitute a primary (versus the selectors part), I find myself confused with what answering a primary would be.  I could by most anything - for a single item (boolean, literal, variable) to something very complex (such as a string of identifiers hanging off of an identifier with or without message sends on the end).  Instead I attempted to simplify it into an array of the parts so that I could iterate over them as needed.  sometimes I would like to find the first in the list, sometimes the last, and sometimes I'd like to search the whole list for a specific call (if present) in the middle.
>>>>
>>>> so, take the code:
>>>>   this.getCommand().getContext().getUser().getActiveProfile()
>>>> The current PJPrimaryWithSelectorsNode will have these in the codeList array:
>>>> this
>>>> getCommand()
>>>> getContext()
>>>> getUser()
>>>> getActiveProfile()
>>>> I did change the behaviour of the parsed #primaryWithselectors - previously it would have bundled this and getCommand() together into one unit identified as 'primary' and all of the other methods as an array of 'selectors'.
>>>>
>>>> similarly, this code:
>>>> new SearchUrl(Search.class).set(parm1, p1).set(parm2, params.get2(p2)).set(parm3, p3).getHref()
>>>> will return the codeList array:
>>>> new SearchUrl(Search.class)
>>>> set(parm1, p1)
>>>> set(parm2, params.get2(p2))
>>>> set(parm3, p3).getHref()
>>>> Previously, it would identify the first item of the array as the primary, and the rest as the selectors.  Which does make a lot of sense to me.
>>>>
>>>> finally, this code:
>>>> this.context.getUser().getActiveProfile().getProfileProperties()
>>>> will return in the code array:
>>>> this
>>>> context
>>>> getUser()
>>>> getActiveProfile()
>>>> getProfileProperties()
>>>> Previously, primary would have consisted of the array
>>>> this
>>>> context
>>>> getUser()
>>>> while selectors would have been the array of
>>>> getActiveProfile()
>>>> getProfileProperties()
>>>>
>>>> I should note that I'm not a Java coder myself and am not clear on how Java coders identify the parts of their code.  I have tried to mostly follow what was previously there in the parser as it was clearer than what I'd likely come up with.  However, I can't really see myself why the 'primary' was primary in the previous examples - is it clear and I should revert back, and have the visitor visit the restored primary and selectors? or should I have the visitor visit each part of the stacked structure that the Java coder has presented us?  Which way would you prefer it - I'll modify it to fit your desires.
>>>>
>>>> Thanks,
>>>> Chris
>>>>
>>>> On Tue, May 28, 2013 at 10:55 PM, Yuriy Tymchuk <[hidden email]> wrote:
>>>> HI,
>>>>
>>>> I've checked the changes. `acceptPrimaryWithSelectorsNode:` has only a flag so I didn't get how should I accept it and started to check out what the idea is. I still don't get it. It has some code list that is usually an array. Why do we need that node? What code entity does it represent? It looks like instead of building an AST from parsed arrays we are wrapping them in other classes.
>>>>
>>>> Yuriy
>>>>
>>>> On 28 трав. 2013, at 21:30, Chris Cunningham <[hidden email]> wrote:
>>>>
>>>>> Hi.
>>>>>
>>>>> I've added the missing method (as well as related missing #acceptVisitor: methods on most of the other nodes that I've added) in the latest change.
>>>>>
>>>>> If you have any other questions or requests, please let me know.
>>>>>
>>>>> Thanks,
>>>>> Chris Cunningham
>>>>>
>>>>>
>>>>> On Tue, May 28, 2013 at 7:45 AM, Yuriy Tymchuk <[hidden email]> wrote:
>>>>> Thank you
>>>>>
>>>>> Надіслано з iPhone
>>>>>
>>>>> 28 трав. 2013 о 17:18 Chris <[hidden email]> написав(ла):
>>>>>
>>>>>> Thanks for letting me know.  I'll fix that today.
>>>>>>
>>>>>> Sent from my iPhone
>>>>>>
>>>>>> On May 28, 2013, at 1:59 AM, Yuriy Tymchuk <[hidden email]> wrote:
>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> your changes to PetitJava break my builds on fast. The problem is that `acceptVisitor:` of PJPrimaryWithSelectorsNode is not implementing `acceptVisitor: aVisitor` method, and so PJASTNodeVisitor is not implementing some visiting method that can give a hint on what should I do in my visitor subclass.
>>>>>>>
>>>>>>> Thank you for your contributions.
>>>>>>> Uko
>>>>>
>>>>
>>>>
>>>
>>> _______________________________________________
>>> Moose-dev mailing list
>>> [hidden email]
>>> https://www.iam.unibe.ch/mailman/listinfo/moose-dev
>>
>> --
>> www.tudorgirba.com
>>
>> "Every now and then stop and ask yourself if the war you're fighting is the right one."
>>
>>
>>
>>
>> _______________________________________________
>> Moose-dev mailing list
>> [hidden email]
>> https://www.iam.unibe.ch/mailman/listinfo/moose-dev
>
>
> _______________________________________________
> Moose-dev mailing list
> [hidden email]
> https://www.iam.unibe.ch/mailman/listinfo/moose-dev

--
www.tudorgirba.com

"Some battles are better lost than fought."




_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev


_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev



If you reply to this email, your message will be added to the discussion below:
http://moose-dev.97923.n3.nabble.com/Re-PetitJava-tp4027589p4027688.html
To unsubscribe from Re: PetitJava, click here.
NAML


View this message in context: Re: PetitJava
Sent from the moose-dev mailing list archive at Nabble.com.

_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev