Parser Pragma Issue and Fix

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

Parser Pragma Issue and Fix

Patrick R.
Hi everyone,

while working on a parser framework I recently discovered an issue when parsing the following method:

someTestMethod
    '<' first

This method can not be parsed by the current Squeak parser, as it expects a closing angle bracket after the message send. The parser misinterprets the bracket in the string as the beginning of a pragma. The fault is in the Parser class which did not respect the type of the token to be parsed. The attached changeset is a proposed fix for the fault. It boils down to checking for the #binary token type in the the responsible parser method:

Parser>>#pragmaSequence
        "Parse a sequence of method pragmas."
       
        [ (hereType == #binary and: [self matchToken: #<])
                        ifFalse: [ ^ self ].
                self pragmaStatement.
                (self matchToken: #>)
                        ifFalse: [ ^ self expected: '>' ] ] repeat

As this is somewhat of a fundamental change, I wanted to bring this up for discussion before merging it.

Thanks and best wishes,
Patrick


ParserPragmaFix.1.cs (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Parser Pragma Issue and Fix

Eliot Miranda-2
Hi Patrick,

    please push to trunk ASAP.  What a terrible blunder on my part!!

_,,,^..^,,,_ (phone)

> On Aug 17, 2020, at 4:58 AM, Rein, Patrick <[hidden email]> wrote:
>
> Hi everyone,
>
> while working on a parser framework I recently discovered an issue when parsing the following method:
>
> someTestMethod
>    '<' first
>
> This method can not be parsed by the current Squeak parser, as it expects a closing angle bracket after the message send. The parser misinterprets the bracket in the string as the beginning of a pragma. The fault is in the Parser class which did not respect the type of the token to be parsed. The attached changeset is a proposed fix for the fault. It boils down to checking for the #binary token type in the the responsible parser method:
>
> Parser>>#pragmaSequence
>    "Parse a sequence of method pragmas."
>    
>    [    (hereType == #binary and: [self matchToken: #<])
>            ifFalse: [ ^ self ].
>        self pragmaStatement.
>        (self matchToken: #>)
>            ifFalse: [ ^ self expected: '>' ] ] repeat
>
> As this is somewhat of a fundamental change, I wanted to bring this up for discussion before merging it.
>
> Thanks and best wishes,
> Patrick
> <ParserPragmaFix.1.cs>
>

Reply | Threaded
Open this post in threaded view
|

Re: Parser Pragma Issue and Fix

Levente Uzonyi
In reply to this post by Patrick R.
Hi Patrick,

On Mon, 17 Aug 2020, Rein, Patrick wrote:

> Hi everyone,
>
> while working on a parser framework I recently discovered an issue when parsing the following method:
>
> someTestMethod
>    '<' first
>
> This method can not be parsed by the current Squeak parser, as it expects a closing angle bracket after the message send. The parser misinterprets the bracket in the string as the beginning of a pragma. The fault is in the Parser class which did not respect the type of the token to be parsed. The attached changeset is a proposed fix for the fault. It boils down to checking for the #binary token type in the the responsible parser method:
>
> Parser>>#pragmaSequence
> "Parse a sequence of method pragmas."
>
> [ (hereType == #binary and: [self matchToken: #<])
> ifFalse: [ ^ self ].
> self pragmaStatement.
> (self matchToken: #>)
> ifFalse: [ ^ self expected: '>' ] ] repeat
>
> As this is somewhat of a fundamental change, I wanted to bring this up for discussion before merging it.

The issue exists the other way around with the closing bracket as well.
After applying your fix compile:

someTestMethod

  <itsAPragma'>'

  ^1

The same fix seems to work there too:

  ...
  (hereType == #binary and: [self matchToken: #>])
  ifFalse: [ ^ self expected: '>' ] ] repeat

Levente

>
> Thanks and best wishes,
> Patrick

Reply | Threaded
Open this post in threaded view
|

Re: Parser Pragma Issue and Fix

Patrick R.
Ah thanks a lot Levente!!! I had a test case covering that during exploring the issue but somehow discarded it at one point. I'll see to adjusting the tests and merging the change in the next few days.

(The hereType == #binary solution is not perfect, as the bracket does not serve as a binary message here, but anything more sophisticated would probably require major changes to the Scanner which is not worth the effort, I think)

Bests
Patrick
________________________________________
From: Squeak-dev <[hidden email]> on behalf of Levente Uzonyi <[hidden email]>
Sent: Tuesday, August 18, 2020 9:49:38 PM
To: The general-purpose Squeak developers list
Subject: Re: [squeak-dev] Parser Pragma Issue and Fix

Hi Patrick,

On Mon, 17 Aug 2020, Rein, Patrick wrote:

> Hi everyone,
>
> while working on a parser framework I recently discovered an issue when parsing the following method:
>
> someTestMethod
>    '<' first
>
> This method can not be parsed by the current Squeak parser, as it expects a closing angle bracket after the message send. The parser misinterprets the bracket in the string as the beginning of a pragma. The fault is in the Parser class which did not respect the type of the token to be parsed. The attached changeset is a proposed fix for the fault. It boils down to checking for the #binary token type in the the responsible parser method:
>
> Parser>>#pragmaSequence
>       "Parse a sequence of method pragmas."
>
>       [       (hereType == #binary and: [self matchToken: #<])
>                       ifFalse: [ ^ self ].
>               self pragmaStatement.
>               (self matchToken: #>)
>                       ifFalse: [ ^ self expected: '>' ] ] repeat
>
> As this is somewhat of a fundamental change, I wanted to bring this up for discussion before merging it.

The issue exists the other way around with the closing bracket as well.
After applying your fix compile:

someTestMethod

        <itsAPragma'>'

        ^1

The same fix seems to work there too:

                ...
                (hereType == #binary and: [self matchToken: #>])
                        ifFalse: [ ^ self expected: '>' ] ] repeat

Levente

>
> Thanks and best wishes,
> Patrick