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

feat: Implement int #3922

Closed
wants to merge 9 commits into from
Closed

Conversation

HarshitaKalani
Copy link
Contributor

@HarshitaKalani HarshitaKalani commented Apr 23, 2024

Towards: #492 and #3248

@HarshitaKalani
Copy link
Contributor Author

HarshitaKalani commented Apr 23, 2024

I don't think we should be adding intrinsics for simple casts like this one. Cannot we simply insert a regular cast node here? -- #3691 (comment)

I implemented this using cast node but this has a problem as it does not handle arrays as elemental Intrinsics do. I think we will need to implement this as an intrinsic only, what say?

program main
    real(4) :: harvest(2) = [1, 2]
    print*, int(harvest)
end
$ lfortran a.f90
semantic error: Unexpected args, Int expects (int), (real) or (complex) as arguments
 --> a.f90:3:13
  |
3 |     print*, int(harvest)
  |             ^^^^^^^^^^^^ 


Note: Please report unclear, confusing or incorrect messages as bugs at
https://github.com/lfortran/lfortran/issues.
$ gfortran a.f90 && ./a.out
           1           2

@Pranavchiku
Copy link
Contributor

I do not think we need IntegerConstructor, simply you can do Cast( <Type>toInt ) and this shall work.

program main
        real :: harvest
        harvest = 13.0
        print *, int(harvest)
end program

The changes in current PR do:

(IntegerConstructor
    (Cast
        (Var 2 harvest)
        RealToInteger
        (Integer 4)
        ()
    )
    (Integer 4)
    ()
)

We need to simply do:

(Cast
    (Var 2 harvest)
    RealToInteger
    (Integer 4)
    ()
)

This can be intrinsic_functions, just add a simple cast node and things will work.

@HarshitaKalani
Copy link
Contributor Author

I think this is not the correct way, hence closing this. Continued in #3953

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

Successfully merging this pull request may close these issues.

None yet

2 participants