Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Local words break SORT/COMPARE #5265

Closed
dsunanda opened this issue Jan 5, 2023 · 5 comments
Closed

Local words break SORT/COMPARE #5265

dsunanda opened this issue Jan 5, 2023 · 5 comments
Assignees
Labels
status.built A change in codebase has been done to address the ticket. test.written A regression test has been written and tested for the ticket. type.bug Ticket describes an abnormal behavior, not conforming to the specs or expectation.

Comments

@dsunanda
Copy link

dsunanda commented Jan 5, 2023

Describe the bug
A SORT/COMPARE witt /LOCALs behaves badly

To reproduce

o1: make object! [x: 1 y: 90]
o2: make object! [x: 2 y: 1]
o3: make object! [x: 2 y: 2]

data: reduce [o1 o2 o3]

sort/compare data func [a b /local ra rb][
   ra: a/x + a/y
   print [ra a/x a/y]
   rb: b/x + b/y
   print [rb b/x b/y]
   return ra < rb
   ]
 probe data

Different results happen if either of the PRINTs are commented out. The results are not right, with or without either of the PRINTs. Repeated running can crash the console with no message.

Expected behavior
The data to be sorted.
It works just fine if the local words are global, or local to a function containing the SORT....
This works just fine:

my-func: func [/local o1 o2 o3 data ra rb][

o1: make object! [x: 1 y: 90]
o2: make object! [x: 2 y: 1]
o3: make object! [x: 2 y: 2]

data: reduce [o1 o2 o3]

sort/compare data func [a b][
   ra: a/x + a/y
   print [ra a/x a/y]
   rb: b/x + b/y
   print [rb b/x b/y]
   return ra < rb
   ]
 probe data
 
 ]
 
 my-func

Platform version

-----------RED & PLATFORM VERSION-----------

RED: [ branch: "master" tag: #v0.6.4 ahead: 4610 date: 7-Dec-2022/11:23:15 commit: #864cd973f53298228f242f5653843cbd61b7cbe0 ]
PLATFORM: [ name: "Windows 11" OS: 'Windows arch: 'x86-64 version: 10.0.0 build: 22621 ]

@hiiamboris
Copy link
Collaborator

It's clear some stack misuse is going on here. Related: #4854 #4543
I vaguely recall having similar problem with sort, one I couldn't reproduce reliably. Thanks for finding this @dsunanda :)

@hiiamboris hiiamboris added the type.bug Ticket describes an abnormal behavior, not conforming to the specs or expectation. label Jan 5, 2023
@hiiamboris
Copy link
Collaborator

Example outputs (commenting out print, removing return, etc):

ra a/x a/y 2 1
... 1 90
*** Script Error: cannot compare make vector! [] with [value make typeset! [datatype! unset! none! logic! block! paren! string! file!
*** Where: <
*** Near : return ra < rb
*** Stack:
ra a/x a/y 2 1
... 1 90
*** Script Error: ra has no value
*** Where: <
*** Near : ra < rb
*** Stack:

@dockimbel dockimbel self-assigned this Jan 7, 2023
qtxie added a commit that referenced this issue Jan 8, 2023
@qtxie qtxie self-assigned this Jan 8, 2023
@qtxie qtxie added status.built A change in codebase has been done to address the ticket. test.written A regression test has been written and tested for the ticket. labels Jan 8, 2023
@qtxie qtxie closed this as completed Jan 8, 2023
@dsunanda
Copy link
Author

dsunanda commented Jan 8, 2023

Hiiamboris says: the fix counts locals, so it misses refinements.

Here's an example with a refinement that exhibits the original (bad!) behaviour:

o1: make object! [x: 1 y: 90]
o2: make object! [x: 2 y: 1]
o3: make object! [x: 2 y: 2]
data: random reduce [o1 o2 o3 o1 o1 o2 o3 o2]

s-func: func [a b /x /local ra rb][
   ra: a/x + a/y
   print [ra a/x a/y]
   rb: b/x + b/y
  print [rb b/x b/y]
   return ra < rb
   ]
sort/compare data :s-func

*** Script Error: rb is missing its value argument
*** Where: rb
*** Near : return ra < rb

@dockimbel
Copy link
Member

The callback function spec should be validated before sorting occurs. It needs to test for 2 argument words, possible return: and no other refinement than an eventual /local.

We should do such user-provided callback spec validation systematically for all other similar cases (e.g. tracing callbacks).

@hiiamboris
Copy link
Collaborator

should this issue stay open then?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status.built A change in codebase has been done to address the ticket. test.written A regression test has been written and tested for the ticket. type.bug Ticket describes an abnormal behavior, not conforming to the specs or expectation.
Projects
None yet
Development

No branches or pull requests

4 participants