lcompilers / lpython

Python compiler
https://lpython.org/
Other
1.5k stars 158 forks source link

Const i32 expressions do not support integer div/mod operations #2349

Closed dylon closed 11 months ago

dylon commented 1 year ago

I have the following source:

# path: /var/tmp/lp/qux.py
from lpython import Const, i32
foo: Const[i32] = 4
bar: Const[i32] = foo // 2

running it through lpython breaks with the following error:

$ ~/Workspace/lpython/src/bin/lpython /var/tmp/lp/qux.py
semantic error: Arguments do not match for any generic procedure, _lpython_floordiv
 --> /var/tmp/lp/qux.py:4:19
  |
4 | bar: Const[i32] = foo // 2
  |                   ^^^^^^^^

Note: Please report unclear or confusing messages as bugs at
https://github.com/lcompilers/lpython/issues.

This should be supported, especially since lpython does not complain about multiplication:

# path: /var/tmp/lp/qux.py
from lpython import Const, i32
foo: Const[i32] = 4
bar: Const[i32] = foo * 2

def main() -> None:
    print(foo)
    print(bar)

main()
$ ~/Workspace/lpython/src/bin/lpython /var/tmp/lp/qux.py
4
8

The following does work, but is ugly:

# path: /var/tmp/lp/qux.py
from lpython import Const, i32
foo: Const[i32] = 4
bar: Const[i32] = i32(foo / 2)

def main() -> None:
    print(foo)
    print(bar)

main()
$ ~/Workspace/lpython/src/bin/lpython /var/tmp/lp/qux.py
4
2

Note that mod (%) also does not work:

$ cat /var/tmp/lp/qux.py
# path: /var/tmp/lp/qux.py
from lpython import Const, i32
foo: Const[i32] = 4
bar: Const[i32] = foo % 2
$ ~/Workspace/lpython/src/bin/lpython /var/tmp/lp/qux.py
semantic error: Arguments do not match for any generic procedure, _mod
 --> /var/tmp/lp/qux.py:4:19
  |
4 | bar: Const[i32] = foo % 2
  |                   ^^^^^^^

Note: Please report unclear or confusing messages as bugs at
https://github.com/lcompilers/lpython/issues.
Thirumalai-Shaktivel commented 1 year ago

I think, the Const[i32] is not handled in _lpython_floordiv and _mod

abhi-glitchhg commented 1 year ago

Can I try this one?

Though I would need some guidance for this.

On Fri, 29 Sept 2023, 09:22 Thirumalai Shaktivel, @.***> wrote:

I think, the Const[i32] is not handled in _lpython_floordiv and _mod

— Reply to this email directly, view it on GitHub https://github.com/lcompilers/lpython/issues/2349#issuecomment-1740270950, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARLRQF7ZKKAFJ7UBPOEY2PLX4ZAYNANCNFSM6AAAAAA5LKA6IU . You are receiving this because you are subscribed to this thread.Message ID: @.***>

-- The information contained in this electronic communication is intended solely for the individual(s) or entity to which it is addressed. It may contain proprietary, confidential and/or legally privileged information. Any review, retransmission, dissemination, printing, copying or other use of, or taking any action in reliance on the contents of this information by person(s) or entities other than the intended recipient is strictly prohibited and may be unlawful. If you have received this communication in error, please notify us by responding to this email or telephone and immediately and permanently delete all copies of this message and any attachments from your system(s). The contents of this message do not necessarily represent the views or policies of BITS Pilani.

Thirumalai-Shaktivel commented 1 year ago

Sure, go ahead and submit a PR!

AnandRaj-7 commented 1 year ago

If i am right then this is becasue in C and C++ comments start with "//" so it is unable to recognize it as a floor division operator and thinks of it as a comment.

certik commented 1 year ago

No, the error comes from LPython not being able to find an implementation for this case. Rather, we should be routing this operation via an explicit ASR node, or via the IntrinsicFunction node.

Shaikh-Ubaid commented 12 months ago

I think we can implement floordiv operation similar to the mod operation using IntrinsicFunction node as in https://github.com/lfortran/lfortran/pull/2519.

abhi-glitchhg commented 12 months ago

I was thinking of overloading the _lpython_floordiv function with inputs dtypes const[i32]. is this approach not good/preferred?

certik commented 12 months ago

You can try it, and if the patch is simple, we can merge it as a short-term workaround. Floor belongs into ASR itself as IntrinsicFunction, so it's always better to invest into the long-term solution.

certik commented 11 months ago

The following now works:

$ cat qux.py 
from lpython import Const, i32
foo: Const[i32] = 4
bar: Const[i32] = foo // 2
print(foo, bar)
$ lpython qux.py
4 2

Thanks @Shaikh-Ubaid !