-
Notifications
You must be signed in to change notification settings - Fork 19
chore: 104256: improve tlisp function docs (esp types) for primitives #150
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
base: master
Are you sure you want to change the base?
chore: 104256: improve tlisp function docs (esp types) for primitives #150
Conversation
Alchemy/CodeChain/DefPrimitives.h
Outdated
| "(< [x1 x2 ... xn]) -> True if x1 < x2 < ... < xn\n" | ||
| "Supports comparisons between additional types compared to ls", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bit misleading - fnEquality / fnEqualityNumerals both support the same types, but use different coercion rules which affect the treatment of Nils and implicit coercion of strings to numeric types.
fnEquality
- Treats Nil as a 0 in numeric comparisons i.e.
(eq Nil 0) -> True - Does not allow string to numeric comparisons. Internally this is returning -2 (incompatible)
fnEqualityNumerals
- Treats Nil as a special value which is always less than anything else
- Attempts to coerce strings to numeric types before comparison, if the coercion fails then we get the -2 (incompatible) result as above
The implicit conversion will mean that:
(= 2 "2") -> True
(= 2.0 "2") -> True
(= 2.0 "2.0") -> True
(= 2 "2.0") -> Nil
And does not support trailing spaces:
(= 2 " 2") -> True
(= 2 "2 ") -> Nil
(= 2.0 "2.0 ") -> Nil
Note - the internal result of -2 for incompatible types is not checked in CompareSucceeds so we get potentially confusing behavior:
(ls 2 "a") -> True
(ls "a" 2) -> True
(<= 2 "a") -> True
(<= "a" 2) -> True
(< 2 "a" 2) -> True
(< "a" 2 "a") -> True
While the greater than (gr, geq, >, >=) comparisons will always return False.
This is rather too much to document in the online help. You could try something like
- fnEquality - coerces Nil, does not coerce strings
- fnEqualityNumerals - coerces strings, does not coerce Nil
Or just put something along the lines of "old / new type coercion rules" and have a readme / wiki / ministry note to explain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A lot of this sounds like bugged implementations.. Does anything actually depend on the inconsistencies/edge cases seen in these functions?
This is so inconsistent I feel that we probably should fix this as a edge-case (using glitched behavior) breaking change for API55+.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does anything actually depend on the inconsistencies/edge cases seen in these functions?
I would not expect anything to rely on this (at least not in the core game).
I don't think the new ordering operators (>, < etc) are ever used much as you'd have to wrap them in a cdata block to have valid xml. They probably only get used for interactive testing in the console.
New core core does usually use the new (in)equality operators == / !=
This is so inconsistent I feel that we probably should fix this as a edge-case (using glitched behavior) breaking change for API55+.
I agree that the treatment of -2 (incompatible types) should probably be changed.
The implicit coercion of string to numeric in comparisons will be tricky as there will always be inconsistencies
(= 2.0 "2") -> True
(= 2.0 "2.0") -> True
; But we do need
(= "2" "2.0") -> Nil
Similarly with extra white-space. The string-string comparison should consider white space so " 2" != "2"
Maybe the automatic coercion should not allow any white-space (trailing or initial)?
The current integer to "string representation of a floating-point number" may be correct behavior. I don't know what would be typical behavior in Lisp, but for example in Python float("2"), float("2.0"), and int("2") are all valid, but int("2.0") is not.
Also see: https://ministry.kronosaur.com/record.hexm?id=72735
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think the new ordering operators (>, < etc) are ever used much as you'd have to wrap them in a cdata block to have valid xml. They probably only get used for interactive testing in the console.
You can replace them with < and > in XML. But why bother when you can use ls and gr instead? So yeah, probably not used much.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would (lt ...) / (gt ...) and (le ...) / (ge ...) be nice aliases to have to get the improved behavior without needing cdata blocks? I can add them in API55 (unfortunately not 2.0a3 as that is still intended to be an API54 release)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest a couple of fixes:
- Leading spaces should be treated the same as trailing spaces and trigger the incompatible types path.
- An incompatible type should be treated like nil--nil is always less than any real value.
The documentation can say something like:
fnEquality treats nil as 0 and doesn't coerce strings. Incompatible types are treated as 0.
fnEqualityNumerals treats nil as less than anything else and coerces strings (if they can be parsed exactly). Incompatible types are treated as less than anything else.
Is that accurate and consistent with the code + fixes above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- An incompatible type should be treated like nil
How would this work - would you treat the second value in a pair as the Nil? Effectively reversing the behaviour of < and > for incompatible arguments i.e.
- Incompatible type less than comparisons will always be Nil e.g.
(< 2 "a" 2) -> Nilis evaluating:- 2 <
"a"Nil -> Nil - "a" <
2Nil -> Nil
- 2 <
- Some incompatible type greater than comparisons may be True e.g.:
(> 2 "a" 2) -> Trueis evaluating:- 2 >
"a"Nil -> True - "a" >
2Nil -> True
- 2 >
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From what was shown last monday, it looks like gridwhale treats numbers as a higher priority type than strings, so a string that cannot be coerced into an int/double would be treated as incompatible.
Also in testing, I found that string comparisons are case insensitive, meaning that (= "a" "A") is true - this causes an issue for code that needs to work with case-sensitive strings like criteria. Also side note, trying to perform math operations on strings interestingly returns a 0 (int) for the old functions and 0.0 (double) for the new ones.
Also we will retarget this branch to the appropriate API version once its determined what changes will be made, and what API the changes will be going into.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right--we can't just treat an incompatible type like nil.
In GridWhale, I try to do a proper comparison for all types (e.g., string - number, etc.) but if the types are still incompatible, I convert both sides to strings and compare that. That should yield a consistent (commutative) comparison.
As for case-sensitive compares, we should introduce a new operator (=== "a" "A") is nil.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that there are edgecases where some things may appear identical that are definitely not (ex: an empty list and a string "()")
# Conflicts: # Alchemy/CodeChain/DefPrimitives.h
https://ministry.kronosaur.com/record.hexm?id=104256