the semantics of SyntaxErrorNotification resume:

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

the semantics of SyntaxErrorNotification resume:

Nicolas Cellier
Hi all,
I see some new tests failing
- testResumeEarlySyntaxError
- testResumeLateSyntaxError

This is entirely my fault because of the "fix" of exception handling.
Indeed, I have not rearmed the exception stack in resume: action, and
I think I should have done so.

However, I see that SyntaxErrorNotification>>resume: launch a new
compilation entirely.
This sounds un-necessary to me because we already have
ReparseAfterSourceEditing for that purpose...
(again I'm the original author of that, Christoph only improved it)

IMO, the purpose is not really to resume: but rather to retry
compilation using newSource.
We (I ?) have used resume: only because notify:at: does signal a
SyntaxErrorNotification, then immediately retries if the exception
resumes.

    (notification := SyntaxErrorNotification
                cue: (cue copy
                    source: (source contents asText
                        copyReplaceFrom: location
                        to: location - 1
                        with: messageText);
                    yourself)
                doitFlag: doitFlag
                errorMessage: string
                location: location) signal.
            notification tryNewSourceIfAvailable

But if the handler does retryWithNewSource: newSource instead of
resume: newSource, then we just have to let retryWithNewSource perform
a self resignalAs: ReparseAfterSourceEditing new.

I think that it's the right solution, rather than bending the
semantics of resume: (the argument of resume: is normally the value
that should be returned to the sender of signal...).

So if you see some changes of SyntaxErrorNotification in trunk, you'll
know a bit more about it
(I'm not sure whether I can redact a commit message with that level of
explanation).

Nicolas

Reply | Threaded
Open this post in threaded view
|

Re: the semantics of SyntaxErrorNotification resume:

Nicolas Cellier
Le ven. 16 avr. 2021 à 20:35, Nicolas Cellier
<[hidden email]> a écrit :

>
> Hi all,
> I see some new tests failing
> - testResumeEarlySyntaxError
> - testResumeLateSyntaxError
>
> This is entirely my fault because of the "fix" of exception handling.
> Indeed, I have not rearmed the exception stack in resume: action, and
> I think I should have done so.
>
> However, I see that SyntaxErrorNotification>>resume: launch a new
> compilation entirely.
> This sounds un-necessary to me because we already have
> ReparseAfterSourceEditing for that purpose...
> (again I'm the original author of that, Christoph only improved it)
>
> IMO, the purpose is not really to resume: but rather to retry
> compilation using newSource.
> We (I ?) have used resume: only because notify:at: does signal a
> SyntaxErrorNotification, then immediately retries if the exception
> resumes.
>
>     (notification := SyntaxErrorNotification
>                 cue: (cue copy
>                     source: (source contents asText
>                         copyReplaceFrom: location
>                         to: location - 1
>                         with: messageText);
>                     yourself)
>                 doitFlag: doitFlag
>                 errorMessage: string
>                 location: location) signal.
>             notification tryNewSourceIfAvailable
>
> But if the handler does retryWithNewSource: newSource instead of
> resume: newSource, then we just have to let retryWithNewSource perform
> a self resignalAs: ReparseAfterSourceEditing new.
>
> I think that it's the right solution, rather than bending the
> semantics of resume: (the argument of resume: is normally the value
> that should be returned to the sender of signal...).
>
> So if you see some changes of SyntaxErrorNotification in trunk, you'll
> know a bit more about it
> (I'm not sure whether I can redact a commit message with that level of
> explanation).
>
> Nicolas

On the other hand, I cannot entirely remove tryNewSourceIfAvailable,
because it is still the resolution path for SyntaxError window...
So I have two alternatives: resignalAs: or resume: (with
reactivateHandlers fix).

retryWithNewSource: source
    "Retry the compilation with new source code.
    Assume that there is already a handler of
ReparseAfterSourceEditing on the sender stack.
    All I have to do then, is to resignal myself as such and let the
existing handler do its business."

    ^ self resignalAs: (ReparseAfterSourceEditing new withNewSource: source)

Or:

retryWithNewSource: source
    "Retry the compilation with new source code.
    Assume:
    - that the signallerContext will tryNewSourceIfAvailable
    - and the presence of a handler of ReparseAfterSourceEditing on
the sender stack"

    newSource := source.
    ^ self resume: self defaultResumeValue

I'm still in favour of naming it #retryWithNewSource: rather than
resume: because super resume: has another semantic than we shall
better not bend.

Reply | Threaded
Open this post in threaded view
|

Re: the semantics of SyntaxErrorNotification resume:

Nicolas Cellier
Le ven. 16 avr. 2021 à 21:52, Nicolas Cellier
<[hidden email]> a écrit :

>
> Le ven. 16 avr. 2021 à 20:35, Nicolas Cellier
> <[hidden email]> a écrit :
> >
> > Hi all,
> > I see some new tests failing
> > - testResumeEarlySyntaxError
> > - testResumeLateSyntaxError
> >
> > This is entirely my fault because of the "fix" of exception handling.
> > Indeed, I have not rearmed the exception stack in resume: action, and
> > I think I should have done so.
> >
> > However, I see that SyntaxErrorNotification>>resume: launch a new
> > compilation entirely.
> > This sounds un-necessary to me because we already have
> > ReparseAfterSourceEditing for that purpose...
> > (again I'm the original author of that, Christoph only improved it)
> >
> > IMO, the purpose is not really to resume: but rather to retry
> > compilation using newSource.
> > We (I ?) have used resume: only because notify:at: does signal a
> > SyntaxErrorNotification, then immediately retries if the exception
> > resumes.
> >
> >     (notification := SyntaxErrorNotification
> >                 cue: (cue copy
> >                     source: (source contents asText
> >                         copyReplaceFrom: location
> >                         to: location - 1
> >                         with: messageText);
> >                     yourself)
> >                 doitFlag: doitFlag
> >                 errorMessage: string
> >                 location: location) signal.
> >             notification tryNewSourceIfAvailable
> >
> > But if the handler does retryWithNewSource: newSource instead of
> > resume: newSource, then we just have to let retryWithNewSource perform
> > a self resignalAs: ReparseAfterSourceEditing new.
> >
> > I think that it's the right solution, rather than bending the
> > semantics of resume: (the argument of resume: is normally the value
> > that should be returned to the sender of signal...).
> >
> > So if you see some changes of SyntaxErrorNotification in trunk, you'll
> > know a bit more about it
> > (I'm not sure whether I can redact a commit message with that level of
> > explanation).
> >
> > Nicolas
>
> On the other hand, I cannot entirely remove tryNewSourceIfAvailable,
> because it is still the resolution path for SyntaxError window...
> So I have two alternatives: resignalAs: or resume: (with
> reactivateHandlers fix).
>
> retryWithNewSource: source
>     "Retry the compilation with new source code.
>     Assume that there is already a handler of
> ReparseAfterSourceEditing on the sender stack.
>     All I have to do then, is to resignal myself as such and let the
> existing handler do its business."
>
>     ^ self resignalAs: (ReparseAfterSourceEditing new withNewSource: source)
>
> Or:
>
> retryWithNewSource: source
>     "Retry the compilation with new source code.
>     Assume:
>     - that the signallerContext will tryNewSourceIfAvailable
>     - and the presence of a handler of ReparseAfterSourceEditing on
> the sender stack"
>
>     newSource := source.
>     ^ self resume: self defaultResumeValue
>
> I'm still in favour of naming it #retryWithNewSource: rather than
> resume: because super resume: has another semantic than we shall
> better not bend.

I opted for the second solution.
I hope that the commit message explains the reason for renaming
sufficiently clearly.