[squeak-dev] [BUG] Parser does not detect syntax error with double colon

Levente Uzonyi leves at caesar.elte.hu
Fri Mar 13 00:06:01 UTC 2020


Hi Christoph,

On Sat, 7 Mar 2020, Thiede, Christoph wrote:

> 
> Hi Levente,
> 
> 
> could we agree on this: "redundant blocks" can be useful as the first step to identify the missing structure of a too-long method, but the next steps are to split up that method and transform the blocks into single methods?

No. By splitting up methods that way, you would create unnecessary small 
methods, which would result in hard-to-understand code.
Just use temporaries, and follow common sense.

> 
> 
> Beside of this interesting idiomatic discussion, I was actually interested whether you would rather install a fix in #xLetter or #messagePart:repeat: :-)

It depends. I would say #xLetter, but there a comment saying "Allow any 
number of embedded colons in literal symbols", and it's been there at
least since 2002.
Perhaps someone more familiar with Compiler's code can give you a better 
answer. :)


Levente

> 
> 
> Best,
> 
> Christoph
> 
> __________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________
> Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org> im Auftrag von Levente Uzonyi <leves at caesar.elte.hu>
> Gesendet: Mittwoch, 4. März 2020 20:15:41
> An: The general-purpose Squeak developers list
> Betreff: Re: [squeak-dev] [BUG] Parser does not detect syntax error with double colon  
> Hi Christoph,
> 
> On Wed, 4 Mar 2020, Thiede, Christoph wrote:
> 
> >
> > Hi all,
> >
> >
> > thanks for your reactions!
> >
> >
> > > Well, those symbols actually don't exist at all. It's just a nice thing about the syntax that blocks look like as if they had those "empty selectors".
> > Wow, that's an interesting comparison :-)
> >
> > Re: code review: I wrote "something like this" to indicate pseudo smalltalk :)
> 
> All right, I'll read it that way next time.
> 
> >
> > Yes, you're right, this should be #next: instead of #take:.
> >
> > > What's wrong with temporaries?
> >
> > Their scope is possibly too large. Usually, this smell suggests you to introduce new methods (in this example: #checkKeywordsSelector:words:, for example). But in this context, it wouldn't match the overall abstraction level.
> > I'm rather a fan of the arbitrary method length in Metacello, by the way ... Plus, "readStream in:" introduces a nice analogy to #streamContents: IMHO. However, it was a pseudo script only ;-)
> > #messagePart:repeat: is a bit messy anyway. It changes the semantics of selector.
> >
> > > My image doesn't have #indexOf:ifPresent:.
> >
> > Neither does mine :-) I should propose them in another commit. #indexOf:[ifPresent:][ifAbsent:] ...
> >
> > So let me know which approach you recommend!
> 
> Neither. I would only want to have those methods if the language didn't
> support temporaries.
> Of course, one could play with the idea to only use blocks and no method
> temporaries (as with #in:, #ifPresent:, #ifFound:, etc). It could even be
> a way to implement some sort of parallel "assignment". But that would be
> like using a different language, one with long lists of closing square
> brackets.
> 
> 
> Levente
> 
> >
> > Best,
> > Christoph
> >
> > _________________________________________________________________________________________________________________________________________________________________________________________________________________________________
> > Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org> im Auftrag von Levente Uzonyi <leves at caesar.elte.hu>
> > Gesendet: Mittwoch, 4. März 2020 15:38 Uhr
> > An: The general-purpose Squeak developers list
> > Betreff: Re: [squeak-dev] [BUG] Parser does not detect syntax error with double colon  
> > Hi Nicolas,
> >
> > On Wed, 4 Mar 2020, Nicolas Cellier wrote:
> >
> > > Hi all,
> > > on the other hand, there are remnants of very old "alternate syntax" experiments
> > > I have some "normalization" of syntax pending in order to reduce unjustified distance to cousin dialects.
> > > http://source.squeak.org/inbox/Compiler-nice.267.diff
> > >
> > > Le mer. 4 mars 2020 à 14:04, Levente Uzonyi <leves at caesar.elte.hu> a écrit :
> > >       Hi All,
> > >
> > >       Answers inline. Rant alert!
> > >
> > >       On Wed, 4 Mar 2020, Thiede, Christoph wrote:
> > >
> > >       >
> > >       > Hi all,
> > >       >
> > >       >
> > >       > I took a look into the parser/scanner, let me ask you a few questions:
> > >       >
> > >       >
> > >       > Is it an important requirement that the compiler must be able to understand symbols such as #value::? Should these by valid symbols at all?
> > >
> > >       What are the consequences if you consider them not valid? What will break?
> > >       Why shouldn't they be valid? The most common "method names" are
> > >       #: #:: #::: #::::, etc (as in [ :x | ... ], [ :x :y | ... ], etc)
> > >
> > > The question is as well why/when did we ever needed to introduce those selectors, because we do not lookup for blocks thru a selector...
> > > Adding something is easy, removing is super hard (thus the changes aimed at removal tend to rot in inbox).
> >
> > Well, those symbols actually don't exist at all. It's just a nice thing
> > about the syntax that blocks look like as if they had those
> > "empty selectors".
> >
> >
> > Levente
> >
> >
> > >
> > >       >
> > >       > If not, we could change the following line in Scanner >> #xLetter:
> > >       >
> > >       >       tokenType := (type == #xColon and: [aheadChar ~~ $=] and: [aheadChar ~= $:])
> > >       >
> > >       > Otherwise, we could insert a validation of selector in #messagePart:repeat:. Something like:
> > >       >
> > >       >
> > >       >       selector readStream in: [:stream |
> > >
> > >       What's wrong with temporaries?
> > >       I see #in: spreading like plague in methods. IMHO #in: is fine in scripts,
> > >       but I see no reason to use it as a replacement of temporaries in methods.
> > >
> > > +1
> > >
> > >       >   words do: [:word |
> > >       >   (stream take: word size - 1)
> > >
> > >       I had to look up what Stream >> #take: does. It's ^self any:
> > >       maxNumberOfElements. So, I had do look up what Stream >> #any: does.
> > >       Surprise, it's ^self next: numberOfElements.
> > >       Perhaps I'm getting too old, but I see no reason to use these methods
> > >       when you know the receiver is a stream.
> > >
> > >       Unfortunately neither methods have comments (#any: #take:), so one could
> > >       easily come to the conclusion that they are not very useful despite having
> > >       such generic names.
> > >
> > >       >   indexOf: $:
> > >       >   ifPresent: [:index | self notify: 'Argument expected' at: word start + index].
> > >
> > >       My image doesn't have #indexOf:ifPresent:.
> > >
> > >
> > >       Levente
> > >
> > >       >   stream skip: 1]].
> > >       >
> > >       >
> > >       > Looking forward to your thoughts!
> > >       >
> > >       >
> > >       > Best,
> > >       >
> > >       > Christoph
> > >       >
> > >       >
> > >      >_______________________________________________________________________________________________________________________________________________________________________________________________________________________________
> > _
> > >       _
> > >       > Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org> im Auftrag von Taeumel, Marcel
> > >       > Gesendet: Freitag, 28. Februar 2020 14:17:20
> > >       > An: John Pfersich via Squeak-dev
> > >       > Betreff: Re: [squeak-dev] [BUG] Parser does not detect syntax error with double colon  
> > >       > Hi Christoph.
> > >       > Thanks for pointing this out. This issue has been around for like forever. :-) In Squeak 3.8, however, the list of choices was bigger:
> > >       >
> > >       > [IMAGE]
> > >       >
> > >       > Let's improve this in 6.0alpha :-)
> > >       >
> > >       > Best,
> > >       > Marcel
> > >       >
> > >       >       Am 28.02.2020 13:49:03 schrieb Thiede, Christoph <christoph.thiede at student.hpi.uni-potsdam.de>:
> > >       >
> > >       >       Steps to reproduce:
> > >       >
> > >       >       Try doIt this (for example, in a workspace):
> > >       >
> > >       >       2 raisedTo:: 3.
> > >       >
> > >       >
> > >       >       Expected behavior:
> > >       >
> > >       >       2 raisedTo:"Argument expected ->": 3.
> > >       >
> > >       >
> > >       >       Actual behavior:
> > >       >
> > >       >       [IMAGE]
> > >       >
> > >       >       If you are thumb enough to choose #raisedTo:: (I was), you get a subsequent error:
> > >       >
> > >       >       [IMAGE]
> > >       >
> > >       >       Even worse: If you try to doIt the same code snippet again, no parser warning/error will be raised at all but you get a runtime error:
> > >       >
> > >       >       MessageNotUnderstood: SmallInteger>>raisedTo::.
> > >       >
> > >       >
> > >       >       Best,
> > >       >
> > >       >       Christoph
> > >       >
> > >       >
> > >       >
> > >
> > >
> > >
> >
> >
> >
> > _________________________________________________________________________________________________________________________________________________________________________________________________________________________________
> > Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org> im Auftrag von Taeumel, Marcel
> > Gesendet: Mittwoch, 4. März 2020 16:35 Uhr
> > An: Robert via Squeak-dev
> > Betreff: Re: [squeak-dev] [BUG] Parser does not detect syntax error with double colon  
> > Hi, there.
> > > Perhaps I'm getting too old, but I see no reason to use these methods 
> > when you know the receiver is a stream.
> > That's right. :-) Both #any: and #take: are generic forms of #anyOne and #first: and similar for collections that are not necessarily sequenceable. The partial adaptation of the collection protocol for streams is still
> > experimental. It is useful in scripting scenarios.
> >
> > "stream take: 42" does not make any sense.
> > "streamOrCollection take: 42" can be quite useful. :-)
> >
> > Best,
> > Marcel
> >
> >       Am 04.03.2020 14:04:27 schrieb Levente Uzonyi <leves at caesar.elte.hu>:
> >
> >       Hi All, 
> >
> >       Answers inline. Rant alert! 
> >
> >       On Wed, 4 Mar 2020, Thiede, Christoph wrote: 
> >
> >       > 
> >       > Hi all, 
> >       > 
> >       > 
> >       > I took a look into the parser/scanner, let me ask you a few questions: 
> >       > 
> >       > 
> >       > Is it an important requirement that the compiler must be able to understand symbols such as #value::? Should these by valid symbols at all? 
> >
> >       What are the consequences if you consider them not valid? What will break? 
> >       Why shouldn't they be valid? The most common "method names" are 
> >       #: #:: #::: #::::, etc (as in [ :x | ... ], [ :x :y | ... ], etc) 
> >
> >       > 
> >       > If not, we could change the following line in Scanner >> #xLetter: 
> >       > 
> >       > tokenType := (type == #xColon and: [aheadChar ~~ $=] and: [aheadChar ~= $:]) 
> >       > 
> >       > Otherwise, we could insert a validation of selector in #messagePart:repeat:. Something like: 
> >       > 
> >       > 
> >       > selector readStream in: [:stream | 
> >
> >       What's wrong with temporaries? 
> >       I see #in: spreading like plague in methods. IMHO #in: is fine in scripts, 
> >       but I see no reason to use it as a replacement of temporaries in methods. 
> >
> >       >   words do: [:word | 
> >       >   (stream take: word size - 1) 
> >
> >       I had to look up what Stream >> #take: does. It's ^self any: 
> >       maxNumberOfElements. So, I had do look up what Stream >> #any: does. 
> >       Surprise, it's ^self next: numberOfElements. 
> >       Perhaps I'm getting too old, but I see no reason to use these methods 
> >       when you know the receiver is a stream. 
> >
> >       Unfortunately neither methods have comments (#any: #take:), so one could 
> >       easily come to the conclusion that they are not very useful despite having 
> >       such generic names. 
> >
> >       >   indexOf: $: 
> >       >   ifPresent: [:index | self notify: 'Argument expected' at: word start + index]. 
> >
> >       My image doesn't have #indexOf:ifPresent:. 
> >
> >
> >       Levente 
> >
> >       >   stream skip: 1]]. 
> >       > 
> >       > 
> >       > Looking forward to your thoughts! 
> >       > 
> >       > 
> >       > Best, 
> >       > 
> >       > Christoph 
> >       > 
> >       > 
> >       >________________________________________________________________________________________________________________________________________________________________________________________________________________________________
> >       _ 
> >       > Von: Squeak-dev im Auftrag von Taeumel, Marcel 
> >       > Gesendet: Freitag, 28. Februar 2020 14:17:20 
> >       > An: John Pfersich via Squeak-dev 
> >       > Betreff: Re: [squeak-dev] [BUG] Parser does not detect syntax error with double colon   
> >       > Hi Christoph. 
> >       > Thanks for pointing this out. This issue has been around for like forever. :-) In Squeak 3.8, however, the list of choices was bigger: 
> >       > 
> >       > [IMAGE] 
> >       > 
> >       > Let's improve this in 6.0alpha :-) 
> >       > 
> >       > Best, 
> >       > Marcel 
> >       > 
> >       > Am 28.02.2020 13:49:03 schrieb Thiede, Christoph : 
> >       > 
> >       > Steps to reproduce: 
> >       > 
> >       > Try doIt this (for example, in a workspace): 
> >       > 
> >       > 2 raisedTo:: 3. 
> >       > 
> >       > 
> >       > Expected behavior: 
> >       > 
> >       > 2 raisedTo:"Argument expected ->": 3. 
> >       > 
> >       > 
> >       > Actual behavior: 
> >       > 
> >       > [IMAGE] 
> >       > 
> >       > If you are thumb enough to choose #raisedTo:: (I was), you get a subsequent error: 
> >       > 
> >       > [IMAGE] 
> >       > 
> >       > Even worse: If you try to doIt the same code snippet again, no parser warning/error will be raised at all but you get a runtime error: 
> >       > 
> >       > MessageNotUnderstood: SmallInteger>>raisedTo::. 
> >       > 
> >       > 
> >       > Best, 
> >       > 
> >       > Christoph 
> >       > 
> >       > 
> >       >
> >
> >
> >
> >
> 
>


More information about the Squeak-dev mailing list