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

Conversion can overflow doesn't recognize direct Enum cast to INTEGER #929

Open
movedoa opened this issue Dec 2, 2021 · 9 comments
Open
Assignees

Comments

@movedoa
Copy link

movedoa commented Dec 2, 2021

2.14.5

This works:

liFoo = INTEGER(loEnum:GetValue()).

This works:

liFoo = loEnum:GetValue().

This doesn't work:

liFoo = INTEGER(loEnum).
@gquerret
Copy link
Contributor

gquerret commented Dec 6, 2021

Reproduced. In order to confirm, this is this code ?

def var rslt as integer.
def var val as Progress.Reflect.AccessMode.

val = Progress.Reflect.AccessMode:private.
rslt = integer(val:GetValue()).
rslt = val:GetValue().
rslt = integer(val). // Not reported 

@gquerret gquerret self-assigned this Dec 6, 2021
@gquerret
Copy link
Contributor

It might be better to report usage of INTEGER(objectReference), whether it is an enum or not. When it's not an enum, the returned value doesn't have any meaning outside of the AVM (so no reason not to use INT64). When it's an enum, it's better (IMO) to use :getValue().

Opinons ?

@movedoa
Copy link
Author

movedoa commented Jan 1, 2022

I fully agree

@PeterJudgeZA
Copy link

Reasons to use INTEGER(val) are that there's no "NPE" when val is an unknown value; the unknown value is simply returned.

@PeterJudgeZA
Copy link

The above is also true for STRING() and ToString()

@gquerret
Copy link
Contributor

gquerret commented Jan 4, 2022

Reasons to use INTEGER(val) are that there's no "NPE" when val is an unknown value; the unknown value is simply returned.

So until the enumRef?:getValue() syntax is available, it's better to use INT64(enumRef) ?
Is there any use case for using INTEGER(objectRef) (so not on enums) ?

@PeterJudgeZA
Copy link

Reasons to use INTEGER(val) are that there's no "NPE" when val is an unknown value; the unknown value is simply returned.

So until the enumRef?:getValue() syntax is available, it's better to use INT64(enumRef) ?

It's shorter, for sure. You could argue that it's not as clear as if valid-object(val) then val:GetVlaue() else ?; FWIW we use both approaches here.

Is there any use case for using INTEGER(objectRef) (so not on enums) ?

Yes, for weak references (ie non-GC-holding) or for logging actual the objects used (eg similar to what the PLO GetString() method does).

@gquerret
Copy link
Contributor

gquerret commented Jan 4, 2022

Rule will report INTEGER(enumRef), but not INT64(enumRef). That will be re-evaluated when the ?: notation is available. @PeterJudge-PSC Is that planned for 12.5 ? Or later ?
I think reporting INTEGER(objectRef) might be useful. The cases you mentioned can be handled by the annotations.

@PeterJudgeZA
Copy link

To the best of my knowledge, 12.5 {std/disclaimer.i}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants