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

Any.== should check for Meta.is_same_object as the first step #6065

Closed
Akirathan opened this issue Mar 23, 2023 · 2 comments · Fixed by #6301
Closed

Any.== should check for Meta.is_same_object as the first step #6065

Akirathan opened this issue Mar 23, 2023 · 2 comments · Fixed by #6301
Assignees
Labels
--bug Type: bug -compiler -libs Libraries: New libraries to be implemented

Comments

@Akirathan
Copy link
Member

Mentioned in #5742 (comment), Meta.is_same_object obj1 obj2 should imply obj1 == obj2. So far, the reason why it is not checked as the first step is because of performance - explained in #5709 (comment).

When applying this patch:

diff --git a/distribution/lib/Standard/Base/0.0.0-dev/src/Data/Time/Day_Of_Week.enso b/distribution/lib/Standard/Base/0.0.0-dev/src/Data/Time/Day_Of_Week.enso
index 4e3dd7400..5aca27d98 100644
--- a/distribution/lib/Standard/Base/0.0.0-dev/src/Data/Time/Day_Of_Week.enso
+++ b/distribution/lib/Standard/Base/0.0.0-dev/src/Data/Time/Day_Of_Week.enso
@@ -1,4 +1,5 @@
 import project.Data.Numbers.Integer
+from project.Data.Ordering import all
 from project.Data.Boolean import Boolean, True, False
 
 polyglot java import java.time.DayOfWeek
@@ -49,3 +50,14 @@ type Day_Of_Week
         Day_Of_Week.Thursday -> DayOfWeek.THURSDAY
         Day_Of_Week.Friday -> DayOfWeek.FRIDAY
         Day_Of_Week.Saturday -> DayOfWeek.SATURDAY
+
+## PRIVATE
+type Day_Of_Week_Comparator
+    compare x y =
+        hash_x = Day_Of_Week_Comparator.hash x
+        hash_y = Day_Of_Week_Comparator.hash y
+        Comparable.from hash_x . compare hash_x hash_y
+
+    hash x = x.to_integer
+
+Comparable.from (_:Day_Of_Week) = Day_Of_Week_Comparator

Day_Of_Week.Monday < Day_Of_Week.Friday will cause StackOverflow because of that.

@Akirathan Akirathan added --bug Type: bug -compiler -libs Libraries: New libraries to be implemented labels Mar 23, 2023
@Akirathan Akirathan self-assigned this Mar 23, 2023
@Akirathan Akirathan moved this from ❓New to 📤 Backlog in Issues Board Mar 23, 2023
@Akirathan
Copy link
Member Author

Blocked by #5709

@jdunkerley jdunkerley added this to the Design Partners milestone Mar 28, 2023
@jdunkerley jdunkerley moved this from 📤 Backlog to ❓New in Issues Board Mar 28, 2023
@jdunkerley jdunkerley moved this from ❓New to 📤 Backlog in Issues Board Mar 28, 2023
@Akirathan Akirathan linked a pull request Apr 17, 2023 that will close this issue
5 tasks
@jdunkerley jdunkerley moved this from 📤 Backlog to ❓New in Issues Board Apr 18, 2023
@jdunkerley jdunkerley moved this from ❓New to 📤 Backlog in Issues Board Apr 18, 2023
@JaroslavTulach
Copy link
Member

Pavel is already working on a fix.

@mergify mergify bot closed this as completed in #6301 Apr 20, 2023
@github-project-automation github-project-automation bot moved this from 📤 Backlog to 🟢 Accepted in Issues Board Apr 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
--bug Type: bug -compiler -libs Libraries: New libraries to be implemented
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants