Problems with #caseError

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

Problems with #caseError

Christoph Thiede

Hi all,


there are two problems with #caseError.


First one:

Given the following example, you will receive a confusing error message:


#foo caseOf: {[#bar] -> [2]. [#baz] -> [3]} 
"--> Error: Case not found (nil), and no otherwise clause"

Either #caseError should be called on #foo, then this would be compiler bug.
Otherwise, I don't see any reason to print out the receiver. We could also name it #caseError: anObject and print out this object, but the current message is not helpful IMO ...

Second one:
I'm not sure whether it is expected behavior how #allLiterals works on the following example:

method := Compiler new
compiledMethodFor: '#bar caseOf: {[#bar] -> [2]}'
in: nil to: nil notifying: nil ifFail: nil.
method allLiterals.
"-->  #(#caseError #bar #caseOf:)"


Carpe Squeak!
Reply | Threaded
Open this post in threaded view
|

Re: Problems with #caseError

Christoph Thiede

(Hit send to early, see below ...)




Von: Thiede, Christoph
Gesendet: Dienstag, 18. Februar 2020 16:21 Uhr
An: Squeak Dev
Betreff: Problems with #caseError
 

Hi all,


there are two problems with #caseError.


First one:

Given the following example, you will receive a confusing error message:


#foo caseOf: {[#bar] -> [2]. [#baz] -> [3]} 
"--> Error: Case not found (nil), and no otherwise clause"

Either #caseError should be called on #foo, then this would be compiler bug.
Otherwise, I don't see any reason to print out the receiver. We could also name it #caseError: anObject and print out this object, but the current message is not helpful IMO ...

Second one:
I'm not sure whether it is expected behavior how #allLiterals works on the following example:

method := Compiler new
compiledMethodFor: '#bar caseOf: {[#bar] -> [2]}'
in: nil to: nil notifying: nil ifFail: nil.
method allLiterals.
"-->  #(#caseError #bar #caseOf:)"

This can be confusing when searching for all users of #caseError. Should we handle this edge case in #allLiterals?

Best,
Christoph


Carpe Squeak!
Reply | Threaded
Open this post in threaded view
|

Re: Problems with #caseError

Christoph Thiede

Otherwise, I don't see any reason to print out the receiver. We could also name it #caseError: anObject and print out this object


I have the feeling that this one would actually be more convenient because the error would be raised on the "failing" object. (Vividly speaking: The unrecognized object is not guilty of being unrecognized, but rather the selecting object is guilty of not recognizing the object.) But this might be a highly opinion-based discussion. On the other hand, #errorNonIntegerIndex belongs to the same category. Supporting the first argument again, #errorSubscriptBounds: is implemented on the calling receiver class ... Really not sure about this.


However, please note that a parametrized #caseError: would require something like


thisContext sender receiver caseError: self


in the non-inlined (we probably have a better term for this?) version of #caseOf:otherwise:. Would this be a code smell?

Best,
Christoph


Von: Squeak-dev <[hidden email]> im Auftrag von Thiede, Christoph
Gesendet: Dienstag, 18. Februar 2020 16:23:50
An: Squeak Dev
Betreff: Re: [squeak-dev] Problems with #caseError
 

(Hit send to early, see below ...)




Von: Thiede, Christoph
Gesendet: Dienstag, 18. Februar 2020 16:21 Uhr
An: Squeak Dev
Betreff: Problems with #caseError
 

Hi all,


there are two problems with #caseError.


First one:

Given the following example, you will receive a confusing error message:


#foo caseOf: {[#bar] -> [2]. [#baz] -> [3]} 
"--> Error: Case not found (nil), and no otherwise clause"

Either #caseError should be called on #foo, then this would be compiler bug.
Otherwise, I don't see any reason to print out the receiver. We could also name it #caseError: anObject and print out this object, but the current message is not helpful IMO ...

Second one:
I'm not sure whether it is expected behavior how #allLiterals works on the following example:

method := Compiler new
compiledMethodFor: '#bar caseOf: {[#bar] -> [2]}'
in: nil to: nil notifying: nil ifFail: nil.
method allLiterals.
"-->  #(#caseError #bar #caseOf:)"

This can be confusing when searching for all users of #caseError. Should we handle this edge case in #allLiterals?

Best,
Christoph


Carpe Squeak!
Reply | Threaded
Open this post in threaded view
|

Re: Problems with #caseError

Christoph Thiede

Here is a changeset that fixes the #caseError issue in a conservative way (no #caseError: with argument).

Unless we decide to parametrize #caseError, I think this one would be small enough for 5.3. Please review.


Please see also Question regarding compilation of #caseOf:[otherwise:] messages. I did not touch the relevant lines in this fix, but as I do not understand their function at the moment, I cannot rule out that they would also need to be adapted.


Best,

Christoph


Von: Squeak-dev <[hidden email]> im Auftrag von Thiede, Christoph
Gesendet: Samstag, 22. Februar 2020 17:35:11
An: Squeak Dev
Betreff: Re: [squeak-dev] Problems with #caseError
 

Otherwise, I don't see any reason to print out the receiver. We could also name it #caseError: anObject and print out this object


I have the feeling that this one would actually be more convenient because the error would be raised on the "failing" object. (Vividly speaking: The unrecognized object is not guilty of being unrecognized, but rather the selecting object is guilty of not recognizing the object.) But this might be a highly opinion-based discussion. On the other hand, #errorNonIntegerIndex belongs to the same category. Supporting the first argument again, #errorSubscriptBounds: is implemented on the calling receiver class ... Really not sure about this.


However, please note that a parametrized #caseError: would require something like


thisContext sender receiver caseError: self


in the non-inlined (we probably have a better term for this?) version of #caseOf:otherwise:. Would this be a code smell?

Best,
Christoph


Von: Squeak-dev <[hidden email]> im Auftrag von Thiede, Christoph
Gesendet: Dienstag, 18. Februar 2020 16:23:50
An: Squeak Dev
Betreff: Re: [squeak-dev] Problems with #caseError
 

(Hit send to early, see below ...)




Von: Thiede, Christoph
Gesendet: Dienstag, 18. Februar 2020 16:21 Uhr
An: Squeak Dev
Betreff: Problems with #caseError
 

Hi all,


there are two problems with #caseError.


First one:

Given the following example, you will receive a confusing error message:


#foo caseOf: {[#bar] -> [2]. [#baz] -> [3]} 
"--> Error: Case not found (nil), and no otherwise clause"

Either #caseError should be called on #foo, then this would be compiler bug.
Otherwise, I don't see any reason to print out the receiver. We could also name it #caseError: anObject and print out this object, but the current message is not helpful IMO ...

Second one:
I'm not sure whether it is expected behavior how #allLiterals works on the following example:

method := Compiler new
compiledMethodFor: '#bar caseOf: {[#bar] -> [2]}'
in: nil to: nil notifying: nil ifFail: nil.
method allLiterals.
"-->  #(#caseError #bar #caseOf:)"

This can be confusing when searching for all users of #caseError. Should we handle this edge case in #allLiterals?

Best,
Christoph



fix caseError.1.cs (5K) Download Attachment
Carpe Squeak!
Reply | Threaded
Open this post in threaded view
|

Re: Problems with #caseError

marcel.taeumel
Hi Christoph,

both issues can be addresed after the 5.3 release. Those a no new issues and not that severe to justify pushing back the first release candidate even further away.

1) Regressions compared to Squeak 5.2 (or maybe 5.1) are problematic.
2) Debuggers popping up for frequent programming tasks are problematic.

Otherwise: There are loads of bugs still luring inside the environment. Many of them rarly surface and can be worked around with little effort. ;-) Those get usually addressed in the "alpha" stage.

Best,
Marcel

Am 22.02.2020 18:37:56 schrieb Thiede, Christoph <[hidden email]>:

Here is a changeset that fixes the #caseError issue in a conservative way (no #caseError: with argument).

Unless we decide to parametrize #caseError, I think this one would be small enough for 5.3. Please review.


Please see also Question regarding compilation of #caseOf:[otherwise:] messages. I did not touch the relevant lines in this fix, but as I do not understand their function at the moment, I cannot rule out that they would also need to be adapted.


Best,

Christoph


Von: Squeak-dev <[hidden email]> im Auftrag von Thiede, Christoph
Gesendet: Samstag, 22. Februar 2020 17:35:11
An: Squeak Dev
Betreff: Re: [squeak-dev] Problems with #caseError
 

Otherwise, I don't see any reason to print out the receiver. We could also name it #caseError: anObject and print out this object


I have the feeling that this one would actually be more convenient because the error would be raised on the "failing" object. (Vividly speaking: The unrecognized object is not guilty of being unrecognized, but rather the selecting object is guilty of not recognizing the object.) But this might be a highly opinion-based discussion. On the other hand, #errorNonIntegerIndex belongs to the same category. Supporting the first argument again, #errorSubscriptBounds: is implemented on the calling receiver class ... Really not sure about this.


However, please note that a parametrized #caseError: would require something like


thisContext sender receiver caseError: self


in the non-inlined (we probably have a better term for this?) version of #caseOf:otherwise:. Would this be a code smell?

Best,
Christoph


Von: Squeak-dev <[hidden email]> im Auftrag von Thiede, Christoph
Gesendet: Dienstag, 18. Februar 2020 16:23:50
An: Squeak Dev
Betreff: Re: [squeak-dev] Problems with #caseError
 

(Hit send to early, see below ...)




Von: Thiede, Christoph
Gesendet: Dienstag, 18. Februar 2020 16:21 Uhr
An: Squeak Dev
Betreff: Problems with #caseError
 

Hi all,


there are two problems with #caseError.


First one:

Given the following example, you will receive a confusing error message:


#foo caseOf: {[#bar] -> [2]. [#baz] -> [3]} 
"--> Error: Case not found (nil), and no otherwise clause"

Either #caseError should be called on #foo, then this would be compiler bug.
Otherwise, I don't see any reason to print out the receiver. We could also name it #caseError: anObject and print out this object, but the current message is not helpful IMO ...

Second one:
I'm not sure whether it is expected behavior how #allLiterals works on the following example:

method := Compiler new
compiledMethodFor: '#bar caseOf: {[#bar] -> [2]}'
in: nil to: nil notifying: nil ifFail: nil.
method allLiterals.
"-->  #(#caseError #bar #caseOf:)"

This can be confusing when searching for all users of #caseError. Should we handle this edge case in #allLiterals?

Best,
Christoph


Reply | Threaded
Open this post in threaded view
|

Re: Problems with #caseError

Christoph Thiede

Hi Marcel,


thanks for the summary. Of course, you're right, if we want to fix all bugs, we will never release Squeak again :D


Best,

Christoph


Von: Squeak-dev <[hidden email]> im Auftrag von Taeumel, Marcel
Gesendet: Montag, 24. Februar 2020 15:11:41
An: John Pfersich via Squeak-dev
Betreff: Re: [squeak-dev] Problems with #caseError
 
Hi Christoph,

both issues can be addresed after the 5.3 release. Those a no new issues and not that severe to justify pushing back the first release candidate even further away.

1) Regressions compared to Squeak 5.2 (or maybe 5.1) are problematic.
2) Debuggers popping up for frequent programming tasks are problematic.

Otherwise: There are loads of bugs still luring inside the environment. Many of them rarly surface and can be worked around with little effort. ;-) Those get usually addressed in the "alpha" stage.

Best,
Marcel

Am 22.02.2020 18:37:56 schrieb Thiede, Christoph <[hidden email]>:

Here is a changeset that fixes the #caseError issue in a conservative way (no #caseError: with argument).

Unless we decide to parametrize #caseError, I think this one would be small enough for 5.3. Please review.


Please see also Question regarding compilation of #caseOf:[otherwise:] messages. I did not touch the relevant lines in this fix, but as I do not understand their function at the moment, I cannot rule out that they would also need to be adapted.


Best,

Christoph


Von: Squeak-dev <[hidden email]> im Auftrag von Thiede, Christoph
Gesendet: Samstag, 22. Februar 2020 17:35:11
An: Squeak Dev
Betreff: Re: [squeak-dev] Problems with #caseError
 

Otherwise, I don't see any reason to print out the receiver. We could also name it #caseError: anObject and print out this object


I have the feeling that this one would actually be more convenient because the error would be raised on the "failing" object. (Vividly speaking: The unrecognized object is not guilty of being unrecognized, but rather the selecting object is guilty of not recognizing the object.) But this might be a highly opinion-based discussion. On the other hand, #errorNonIntegerIndex belongs to the same category. Supporting the first argument again, #errorSubscriptBounds: is implemented on the calling receiver class ... Really not sure about this.


However, please note that a parametrized #caseError: would require something like


thisContext sender receiver caseError: self


in the non-inlined (we probably have a better term for this?) version of #caseOf:otherwise:. Would this be a code smell?

Best,
Christoph


Von: Squeak-dev <[hidden email]> im Auftrag von Thiede, Christoph
Gesendet: Dienstag, 18. Februar 2020 16:23:50
An: Squeak Dev
Betreff: Re: [squeak-dev] Problems with #caseError
 

(Hit send to early, see below ...)




Von: Thiede, Christoph
Gesendet: Dienstag, 18. Februar 2020 16:21 Uhr
An: Squeak Dev
Betreff: Problems with #caseError
 

Hi all,


there are two problems with #caseError.


First one:

Given the following example, you will receive a confusing error message:


#foo caseOf: {[#bar] -> [2]. [#baz] -> [3]} 
"--> Error: Case not found (nil), and no otherwise clause"

Either #caseError should be called on #foo, then this would be compiler bug.
Otherwise, I don't see any reason to print out the receiver. We could also name it #caseError: anObject and print out this object, but the current message is not helpful IMO ...

Second one:
I'm not sure whether it is expected behavior how #allLiterals works on the following example:

method := Compiler new
compiledMethodFor: '#bar caseOf: {[#bar] -> [2]}'
in: nil to: nil notifying: nil ifFail: nil.
method allLiterals.
"-->  #(#caseError #bar #caseOf:)"

This can be confusing when searching for all users of #caseError. Should we handle this edge case in #allLiterals?

Best,
Christoph


Carpe Squeak!
Reply | Threaded
Open this post in threaded view
|

Re: Problems with #caseError

Christoph Thiede
Hi all,

this is a small reminder, this changeset is still pending review. Would it
be possible to get this merged before the next release? :-)

Best,
Christoph



-----
Carpe Squeak!
--
Sent from: http://forum.world.st/Squeak-Dev-f45488.html

Carpe Squeak!
Reply | Threaded
Open this post in threaded view
|

Re: Problems with #caseError

David T. Lewis
Hi Christoph,

On Sun, Mar 28, 2021 at 06:51:24AM -0500, Christoph Thiede wrote:
> Hi all,
>
> this is a small reminder, this changeset is still pending review. Would it
> be possible to get this merged before the next release? :-)
>

Perhaps you can update the changes and maybe submit back to the inbox?
Looking at the change set, I see that your change to Object is already
in the image, and there have been more recent changes to MessageNode
that presumably would require merging. I expect that the remaining change
would be a small update to the Compiler package.

I should note that it took me quite some time to figure out the context
of this message. I think it is because of this:

> --
> Sent from: http://forum.world.st/Squeak-Dev-f45488.html
>

Apparently you send this as a reply from the forum, but the resulting
email here on the list provided no context for the thread you were
discussing. I was unable to find any prior emails with this subject
in the recent archives. I finally figured it out when I noticed the
"Sent from:" line at the bottom of your message.

Dave


Reply | Threaded
Open this post in threaded view
|

Re: Problems with #caseError

Christoph Thiede

Hi Dave,


I am sorry for the confusion. It took me some time to finally find the right thread, but apparently, Nicolas already has merged this changeset last May via Compiler-nice.430. (Thank you, Nicolas!) Because the thread when I mentioned it here ended with Marcel's advice to merge the changeset later, I had assumed that it would still be to do.

I could now, once again, point to the advantages of a GitHub-like issue and PR tracker where you could close and auto-reference threads from each other, but I guess I have already done this often enough. :-) Maybe it would be a good idea to (manually) send a short confirmation message directly to the thread where a changeset has been posted when someone finds the time to merge it. For inbox versions, this process is easier because they are arranged in Monticello with a clear identifier ...


Again, sorry for wasting your time! Nevertheless, your thoughts on #caseError: (with argument) and my #allLiterals observation are highly welcome. :-)


I should note that it took me quite some time to figure out the context of this message.


I have no clue why the Nabble link does not point to the exact conversation. Usually, it does as far as I know. I also think it would be an improvement if replies that are sent via Nabble would have attached the entire conversation. Are we in control of the Nabble sources to make such a change?


Best,

Christoph


Von: Squeak-dev <[hidden email]> im Auftrag von David T. Lewis <[hidden email]>
Gesendet: Sonntag, 28. März 2021 16:02:31
An: The general-purpose Squeak developers list
Betreff: Re: [squeak-dev] Problems with #caseError
 
Hi Christoph,

On Sun, Mar 28, 2021 at 06:51:24AM -0500, Christoph Thiede wrote:
> Hi all,
>
> this is a small reminder, this changeset is still pending review. Would it
> be possible to get this merged before the next release? :-)
>

Perhaps you can update the changes and maybe submit back to the inbox?
Looking at the change set, I see that your change to Object is already
in the image, and there have been more recent changes to MessageNode
that presumably would require merging. I expect that the remaining change
would be a small update to the Compiler package.

I should note that it took me quite some time to figure out the context
of this message. I think it is because of this:

> --
> Sent from: http://forum.world.st/Squeak-Dev-f45488.html
>

Apparently you send this as a reply from the forum, but the resulting
email here on the list provided no context for the thread you were
discussing. I was unable to find any prior emails with this subject
in the recent archives. I finally figured it out when I noticed the
"Sent from:" line at the bottom of your message.

Dave




Carpe Squeak!
Reply | Threaded
Open this post in threaded view
|

Re: Problems with #caseError

David T. Lewis
On Sun, Mar 28, 2021 at 03:05:03PM +0000, Thiede, Christoph wrote:
> Hi Dave,
>
>
> I am sorry for the confusion. It took me some time to finally find the right thread, but apparently, Nicolas already has merged this changeset last May via Compiler-nice.430. (Thank you, Nicolas!) Because the thread when I mentioned it here ended with Marcel's advice to merge the changeset later, I had assumed that it would still be to do.
>

It's no problem at all, I only wanted to explain it because it was
not obvious why the message context was lost, so I figured that if
it was confusing to me then it would probably be confusing to other
people too :-)

Dave