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

Update USUAL ! operator to attempt type cast #1606

Merged

Conversation

joshua-azyra
Copy link
Contributor

Background

It is possible to implicitly cast a USUAL to a LOGIC value but not possible to apply the not (!) operator.
This seems counter intuitive as in one case the USUAL can be treated as though it is a LOGIC but not in the other case.

I think it would feel more consistent to attempt to implicitly cast the

Example

image

image

Both giving
image

Details

The change to the runtime should make no impact on performance for the happy path (unchanged so there should be no overhead with casting)
It will allow USUAL of type [] to be treated a LOGIC
It will change the exception for the other USUAL types

image

I'm happy for this PR to be closed without merging if you think the current behaviour is preferable.

The happy path is unchanged so there should be no overhead with casting
(where there wasn't previously)
@RobertvanderHulst RobertvanderHulst added this to the 2.21 milestone Oct 19, 2024
@RobertvanderHulst RobertvanderHulst merged commit 5138852 into X-Sharp:main Oct 19, 2024
@RobertvanderHulst RobertvanderHulst self-requested a review October 19, 2024 14:20
@RobertvanderHulst RobertvanderHulst self-assigned this Oct 19, 2024
@cpyrgas
Copy link

cpyrgas commented Oct 19, 2024

Guys, this is incompatible to VO though. Actually VO does not allow casting a usual holding an it to a logic at all. All attempts below to make such an assignment, result to a data type runtime error in VO. Not sure why we did allow the USUAL(INT)->LOGIC conversion in X# in the first place, but does not look correct. Maybe it was for compatibility with vulcan?

LOCAL u AS USUAL
LOCAL l AS LOGIC
LOCAL cb AS CODEBLOCK
cb := {||999}
IF FALSE//Eval(cb) // runtime error in VO
   ? "aaaa"
ENDIF
u := 1
l := u  // runtime error
l := Eval(cb)  // runtime error

@cpyrgas
Copy link

cpyrgas commented Oct 19, 2024

Only way to assign an INT (or an INT in a USUAL) to LOGIC in VO, is with a strong cast. A soft conversion also fails:

LOCAL u AS USUAL
LOCAL l AS LOGIC
LOCAL n AS INT

n := 123
l := LOGIC(_CAST,n) // works in VO
? l // TRUE
u := 1
l := LOGIC(_CAST,u) // works in VO
? l // TRUE
l := LOGIC(u) // error
? l

So I think it's better to handle this in the explicit conversion operator from usual to logic instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants