Added support for negative constant indexing in tuples #1534

anutosh491 · 2023-02-19T17:34:16Z

This pr implements negative constant indexing in tuples .

(lf) anutosh491@spbhat68:~/lpython/lpython$ cat examples/expr2.py 
def main0():
    t: tuple[i32, i32, i32] = (1, 2, 3)
    print(t[-1], t[-2], t[-3])

main0()

# On main

(lf) anutosh491@spbhat68:~/lpython/lpython$ ./src/bin/lpython examples/expr2.py 
Internal Compiler Error: Unhandled exception
Traceback (most recent call last):
  Binary file "/home/anutosh491/lpython/lpython/src/bin/lpython", in _start()
  Binary file "/lib/x86_64-linux-gnu/libc.so.6", in __libc_start_main()
  File "/home/anutosh491/lpython/lpython/src/bin/lpython.cpp", line 1682, in ??
    err = compile_python_to_object_file(arg_file, tmp_o, runtime_library_dir,
  File "/home/anutosh491/lpython/lpython/src/bin/lpython.cpp", line 680, in ??
    !(arg_c && compiler_options.disable_main), infile);
  File "/home/anutosh491/lpython/lpython/src/lpython/semantics/python_ast_to_asr.cpp", line 6193, in LCompilers::LPython::python_ast_to_asr(Allocator&, LCompilers::LocationManager&, LCompilers::LPython::AST::ast_t&, LCompilers::diag::Diagnostics&, LCompilers::CompilerOptions&, bool, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, bool)
    ast_overload, allow_implicit_casting);
  File "/home/anutosh491/lpython/lpython/src/lpython/semantics/python_ast_to_asr.cpp", line 6145, in LCompilers::LPython::body_visitor(Allocator&, LCompilers::LocationManager&, LCompilers::LPython::AST::Module_t const&, LCompilers::diag::Diagnostics&, LCompilers::ASR::asr_t*, bool, std::map<int, LCompilers::ASR::symbol_t*, std::less<int>, std::allocator<std::pair<int const, LCompilers::ASR::symbol_t*> > >&, bool)
    b.visit_Module(ast);
  File "/home/anutosh491/lpython/lpython/src/lpython/semantics/python_ast_to_asr.cpp", line 3870, in LCompilers::LPython::BodyVisitor::visit_Module(LCompilers::LPython::AST::Module_t const&)
    visit_stmt(*x.m_body[i]);
  File "/home/anutosh491/lpython/lpython/src/lpython/python_ast.h", line 1882, in LCompilers::LPython::AST::BaseVisitor<LCompilers::LPython::BodyVisitor>::visit_stmt(LCompilers::LPython::AST::stmt_t const&)
    void visit_stmt(const stmt_t &b) { visit_stmt_t(b, self()); }
  File "/home/anutosh491/lpython/lpython/src/lpython/python_ast.h", line 1750, in ??
    case stmtType::FunctionDef: { v.visit_FunctionDef((const FunctionDef_t &)x); return; }
  File "/home/anutosh491/lpython/lpython/src/lpython/semantics/python_ast_to_asr.cpp", line 3930, in LCompilers::LPython::BodyVisitor::visit_FunctionDef(LCompilers::LPython::AST::FunctionDef_t const&)
    handle_fn(x, *f);
  File "/home/anutosh491/lpython/lpython/src/lpython/semantics/python_ast_to_asr.cpp", line 3905, in LCompilers::LPython::BodyVisitor::handle_fn(LCompilers::LPython::AST::FunctionDef_t const&, LCompilers::ASR::Function_t&)
    transform_stmts(body, x.n_body, x.m_body);
  File "/home/anutosh491/lpython/lpython/src/lpython/semantics/python_ast_to_asr.cpp", line 3830, in LCompilers::LPython::BodyVisitor::transform_stmts(LCompilers::Vec<LCompilers::ASR::stmt_t*>&, unsigned long, LCompilers::LPython::AST::stmt_t**)
    this->visit_stmt(*m_body[i]);
  File "/home/anutosh491/lpython/lpython/src/lpython/python_ast.h", line 1882, in LCompilers::LPython::AST::BaseVisitor<LCompilers::LPython::BodyVisitor>::visit_stmt(LCompilers::LPython::AST::stmt_t const&)
    void visit_stmt(const stmt_t &b) { visit_stmt_t(b, self()); }
  File "/home/anutosh491/lpython/lpython/src/lpython/python_ast.h", line 1772, in ??
    case stmtType::Expr: { v.visit_Expr((const Expr_t &)x); return; }
  File "/home/anutosh491/lpython/lpython/src/lpython/semantics/python_ast_to_asr.cpp", line 5408, in LCompilers::LPython::BodyVisitor::visit_Expr(LCompilers::LPython::AST::Expr_t const&)
    visit_expr_list(c->m_args, c->n_args, args);
  File "/home/anutosh491/lpython/lpython/src/lpython/semantics/python_ast_to_asr.cpp", line 752, in LCompilers::LPython::CommonVisitor<LCompilers::LPython::BodyVisitor>::visit_expr_list(LCompilers::LPython::AST::expr_t**, unsigned long, LCompilers::Vec<LCompilers::ASR::call_arg_t>&)
    this->visit_expr(*exprs[i]);
  File "/home/anutosh491/lpython/lpython/src/lpython/python_ast.h", line 1909, in LCompilers::LPython::AST::BaseVisitor<LCompilers::LPython::BodyVisitor>::visit_expr(LCompilers::LPython::AST::expr_t const&)
    void visit_expr(const expr_t &b) { visit_expr_t(b, self()); }
  File "/home/anutosh491/lpython/lpython/src/lpython/python_ast.h", line 1811, in ??
    case exprType::Subscript: { v.visit_Subscript((const Subscript_t &)x); return; }
  File "/home/anutosh491/lpython/lpython/src/lpython/semantics/python_ast_to_asr.cpp", line 3173, in LCompilers::LPython::CommonVisitor<LCompilers::LPython::BodyVisitor>::visit_Subscript(LCompilers::LPython::AST::Subscript_t const&)
    if( !visit_SubscriptIndices(x.m_slice, args, value, type,
  File "/home/anutosh491/lpython/lpython/src/lpython/semantics/python_ast_to_asr.cpp", line 3129, in LCompilers::LPython::CommonVisitor<LCompilers::LPython::BodyVisitor>::visit_SubscriptIndices(LCompilers::LPython::AST::expr_t*, LCompilers::Vec<LCompilers::ASR::array_index_t>&, LCompilers::ASR::expr_t*, LCompilers::ASR::ttype_t*, bool&, LCompilers::Location const&)
    int i = ASR::down_cast<ASR::IntegerConstant_t>(ASRUtils::EXPR(tmp))->m_n;
AssertFailed: is_a<T>(*f)

# On branch
(lf) anutosh491@spbhat68:~/lpython/lpython$ ./src/bin/lpython examples/expr2.py 
3 2 1

Other out of bound cases being ( similar result can be seen for positive indices that'll run out of bound)

(lf) anutosh491@spbhat68:~/lpython/lpython$ cat examples/expr2.py 
def main0():
    t: tuple[i32, i32, i32] = (1, 2, 3)
    print(t[-4])

main0()

(lf) anutosh491@spbhat68:~/lpython/lpython$ ./src/bin/lpython examples/expr2.py
semantic error: TupleIndex out of range
 --> examples/expr2.py:3:11
  |
3 |     print(t[-4])
  |           ^^^^^ 


Note: if any of the above error or warning messages are not clear or are lacking
context please report it to us (we consider that a bug that needs to be fixed).

anutosh491 · 2023-02-19T17:35:12Z

if this would need a test , i'll do that soon !

certik · 2023-02-20T03:24:37Z

This looks good, it implements compile time negative indexing, which should cover most of the use cases.

Can you add an integration test? Test the value of the element for t[-1], etc. Enable cpython and llvm.

src/lpython/semantics/python_ast_to_asr.cpp

src/libasr/runtime/lfortran_intrinsics.c

Thirumalai-Shaktivel

LGTM

czgdp1807 · 2023-02-21T17:21:06Z

src/lpython/semantics/python_ast_to_asr.cpp

                index = ASRUtils::EXPR(tmp);
-                int i = ASR::down_cast<ASR::IntegerConstant_t>(ASRUtils::EXPR(tmp))->m_n;
+                ASR::expr_t* val = ASRUtils::expr_value(index);
+                if (val && ASR::is_a<ASR::IntegerConstant_t>(*val)) {


If val is nullptr then it should throw an error I guess. Runtime indexing isn't possible with tuples because then the type of TupleItem cannot be figured out at compile time so generating the node itself won't be possible.

Yeah as llvm backend does not yet support variable index for tuples.... for eg these programs

def main0(): t: tuple[i32, i32, i32] = (1, 2, 3) x: i32 = -1 print(t[x]) main0()

So for these cases , we can see that val would turn out to be nullptr . Hence we could surely raise something here like Runtime Indexing with Tuples is not yet supported .

After the latest commit we would get the following error raised for the program .

(lf) anutosh491@spbhat68:~/lpython/lpython$ ./src/bin/lpython examples/expr2.py semantic error: Runtime Indexing with Tuples is not yet supported --> examples/expr2.py:4:11 | 4 | print(t[x]) | ^^^^

IMO, runtimes indexing with tuples can never be supported in a strictly typed compiler like LPython. For example C++ (a statically typed language) also doesn't support runtime indices with tuples. The reason is fairly simple if the index is runtime then type of the indexed item becomes runtime, which implies the destination variable where the indexed item will be stored has a runtime type but that's contradictory because the compiler is strictly typed i.e., you should know the type of each variable while writing the code.

What should we do? - An error message like, "Runtime Indexing with " + ASRUtils::type_to_str_python(whatever_the_type_of_tuple_is) + "is not possible". That way user will know that it isn't possible to pass runtime indices to tuples like, tuple[i32, str, ...].

Supporting tuple[any, any, any] will be a separate case which I think is different from what we are dealing with here.

Code

#include <tuple> #include <iostream> #include <string> #include <stdexcept> std::tuple<double, char, std::string> get_student(int id) { switch (id) { case 0: return {3.8, 'A', "Lisa Simpson"}; case 1: return {2.9, 'C', "Milhouse Van Houten"}; case 2: return {1.7, 'D', "Ralph Wiggum"}; case 3: return {0.6, 'F', "Bart Simpson"}; } throw std::invalid_argument("id"); } int main() { int i, j, k; const auto student0 = get_student(0); std::cin >> i >> j >> k; std::cout << std::get<i>(student0) << ", " << std::get<j>(student0) << ", " << std::get<k>(student0) << '\n'; }

Shell Output

(lp) 20:07:50:~/lpython_project/lpython % clang++ /Users/czgdp1807/lpython_project/debug.cpp -"std=c++17" /Users/czgdp1807/lpython_project/debug.cpp:25:18: error: no matching function for call to 'get' std::cout << std::get<i>(student0) ^~~~~~~~~~~ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__tuple:226:1: note: candidate template ignored: invalid explicitly-specified argument for template parameter '_Ip' get(array<_Tp, _Size>&) _NOEXCEPT; ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__tuple:231:1: note: candidate template ignored: invalid explicitly-specified argument for template parameter '_Ip' get(const array<_Tp, _Size>&) _NOEXCEPT; ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__tuple:237:1: note: candidate template ignored: invalid explicitly-specified argument for template parameter '_Ip' get(array<_Tp, _Size>&&) _NOEXCEPT; ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__tuple:242:1: note: candidate template ignored: invalid explicitly-specified argument for template parameter '_Ip' get(const array<_Tp, _Size>&&) _NOEXCEPT; ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__utility/pair.h:491:1: note: candidate template ignored: invalid explicitly-specified argument for template parameter '_Ip' get(pair<_T1, _T2>& __p) _NOEXCEPT ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__utility/pair.h:499:1: note: candidate template ignored: invalid explicitly-specified argument for template parameter '_Ip' get(const pair<_T1, _T2>& __p) _NOEXCEPT ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__utility/pair.h:508:1: note: candidate template ignored: invalid explicitly-specified argument for template parameter '_Ip' get(pair<_T1, _T2>&& __p) _NOEXCEPT ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__utility/pair.h:516:1: note: candidate template ignored: invalid explicitly-specified argument for template parameter '_Ip' get(const pair<_T1, _T2>&& __p) _NOEXCEPT ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__utility/pair.h:525:17: note: candidate template ignored: invalid explicitly-specified argument for template parameter '_T1' constexpr _T1 & get(pair<_T1, _T2>& __p) _NOEXCEPT ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__utility/pair.h:532:23: note: candidate template ignored: invalid explicitly-specified argument for template parameter '_T1' constexpr _T1 const & get(pair<_T1, _T2> const& __p) _NOEXCEPT ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__utility/pair.h:539:18: note: candidate template ignored: invalid explicitly-specified argument for template parameter '_T1' constexpr _T1 && get(pair<_T1, _T2>&& __p) _NOEXCEPT ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__utility/pair.h:546:24: note: candidate template ignored: invalid explicitly-specified argument for template parameter '_T1' constexpr _T1 const && get(pair<_T1, _T2> const&& __p) _NOEXCEPT ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__utility/pair.h:553:17: note: candidate template ignored: invalid explicitly-specified argument for template parameter '_T1' constexpr _T1 & get(pair<_T2, _T1>& __p) _NOEXCEPT ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__utility/pair.h:560:23: note: candidate template ignored: invalid explicitly-specified argument for template parameter '_T1' constexpr _T1 const & get(pair<_T2, _T1> const& __p) _NOEXCEPT ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__utility/pair.h:567:18: note: candidate template ignored: invalid explicitly-specified argument for template parameter '_T1' constexpr _T1 && get(pair<_T2, _T1>&& __p) _NOEXCEPT ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__utility/pair.h:574:24: note: candidate template ignored: invalid explicitly-specified argument for template parameter '_T1' constexpr _T1 const && get(pair<_T2, _T1> const&& __p) _NOEXCEPT ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/tuple:1129:1: note: candidate template ignored: invalid explicitly-specified argument for template parameter '_Ip' get(tuple<_Tp...>& __t) _NOEXCEPT ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/tuple:1138:1: note: candidate template ignored: invalid explicitly-specified argument for template parameter '_Ip' get(const tuple<_Tp...>& __t) _NOEXCEPT ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/tuple:1147:1: note: candidate template ignored: invalid explicitly-specified argument for template parameter '_Ip' get(tuple<_Tp...>&& __t) _NOEXCEPT ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/tuple:1157:1: note: candidate template ignored: invalid explicitly-specified argument for template parameter '_Ip' get(const tuple<_Tp...>&& __t) _NOEXCEPT ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/tuple:1206:16: note: candidate template ignored: invalid explicitly-specified argument for template parameter '_T1' constexpr _T1& get(tuple<_Args...>& __tup) noexcept ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/tuple:1213:22: note: candidate template ignored: invalid explicitly-specified argument for template parameter '_T1' constexpr _T1 const& get(tuple<_Args...> const& __tup) noexcept ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/tuple:1220:17: note: candidate template ignored: invalid explicitly-specified argument for template parameter '_T1' constexpr _T1&& get(tuple<_Args...>&& __tup) noexcept ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/tuple:1227:23: note: candidate template ignored: invalid explicitly-specified argument for template parameter '_T1' constexpr _T1 const&& get(tuple<_Args...> const&& __tup) noexcept ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/variant:1487:59: note: candidate template ignored: invalid explicitly-specified argument for template parameter '_Ip' constexpr variant_alternative_t<_Ip, variant<_Types...>>& get( ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/variant:1497:60: note: candidate template ignored: invalid explicitly-specified argument for template parameter '_Ip' constexpr variant_alternative_t<_Ip, variant<_Types...>>&& get( ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/variant:1507:65: note: candidate template ignored: invalid explicitly-specified argument for template parameter '_Ip' constexpr const variant_alternative_t<_Ip, variant<_Types...>>& get( ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/variant:1517:66: note: candidate template ignored: invalid explicitly-specified argument for template parameter '_Ip' constexpr const variant_alternative_t<_Ip, variant<_Types...>>&& get( ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/variant:1527:16: note: candidate template ignored: invalid explicitly-specified argument for template parameter '_Tp' constexpr _Tp& get(variant<_Types...>& __v) { ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/variant:1535:17: note: candidate template ignored: invalid explicitly-specified argument for template parameter '_Tp' constexpr _Tp&& get(variant<_Types...>&& __v) { ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/variant:1544:22: note: candidate template ignored: invalid explicitly-specified argument for template parameter '_Tp' constexpr const _Tp& get(const variant<_Types...>& __v) { ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/variant:1552:23: note: candidate template ignored: invalid explicitly-specified argument for template parameter '_Tp' constexpr const _Tp&& get(const variant<_Types...>&& __v) {

Thanks a lot for your explanation. I will go through it and make neccessary changes !

Supporting tuple[any, any, any] will be a separate case which I think is different from what we are dealing with here.

Correct

czgdp1807

LGTM other than the suggested change.

src/lpython/semantics/python_ast_to_asr.cpp

anutosh491 · 2023-02-23T05:37:05Z

I've addressed all requested changes . Thank you !

czgdp1807

LGTM.

src/lpython/semantics/python_ast_to_asr.cpp

certik · 2023-02-25T04:43:15Z

Otherwise this looks good to me, thanks!

anutosh491 · 2023-02-26T01:51:33Z

Thanks for the reviews on this @Thirumalai-Shaktivel @czgdp1807 @certik

Added support for negative indexing in tuples

1d9168b

faze-geek reviewed Feb 20, 2023

View reviewed changes

src/lpython/semantics/python_ast_to_asr.cpp Outdated Show resolved Hide resolved

anutosh491 added 2 commits February 20, 2023 13:11

Added integration test

2a9cd42

Updated error messages

36562bf

faze-geek reviewed Feb 20, 2023

View reviewed changes

src/libasr/runtime/lfortran_intrinsics.c Outdated Show resolved Hide resolved

Thirumalai-Shaktivel approved these changes Feb 21, 2023

View reviewed changes

czgdp1807 requested changes Feb 21, 2023

View reviewed changes

anutosh491 and others added 2 commits February 22, 2023 09:40

Merge branch 'main' into branch05

7d99710

Added semanticerror when val is nullptr

3c620f2

anutosh491 requested a review from czgdp1807 February 22, 2023 06:50

czgdp1807 marked this pull request as draft February 22, 2023 14:46

updated error message

3ca6367

anutosh491 marked this pull request as ready for review February 22, 2023 17:13

czgdp1807 approved these changes Feb 23, 2023

View reviewed changes

src/lpython/semantics/python_ast_to_asr.cpp Show resolved Hide resolved

czgdp1807 requested changes Feb 23, 2023

View reviewed changes

src/lpython/semantics/python_ast_to_asr.cpp Show resolved Hide resolved

src/lpython/semantics/python_ast_to_asr.cpp Show resolved Hide resolved

Added tests to check for errors being thrown

57b27d4

anutosh491 requested a review from czgdp1807 February 23, 2023 06:58

czgdp1807 approved these changes Feb 24, 2023

View reviewed changes

certik reviewed Feb 25, 2023

View reviewed changes

src/lpython/semantics/python_ast_to_asr.cpp Show resolved Hide resolved

added else block when type is not IntegerConstant

bfd353f

certik merged commit d66b71f into lcompilers:main Feb 25, 2023

anutosh491 deleted the branch05 branch February 26, 2023 01:51

czgdp1807 mentioned this pull request Feb 28, 2023

Negative variable indexing do not work for lists #1533

Open

Added support for negative constant indexing in tuples #1534

Added support for negative constant indexing in tuples #1534

Uh oh!

Conversation

anutosh491 commented Feb 19, 2023

Uh oh!

anutosh491 commented Feb 19, 2023

Uh oh!

certik commented Feb 20, 2023

Uh oh!

Uh oh!

Uh oh!

Thirumalai-Shaktivel left a comment

Choose a reason for hiding this comment

Uh oh!

czgdp1807 Feb 21, 2023

Choose a reason for hiding this comment

Uh oh!

anutosh491 Feb 22, 2023

Choose a reason for hiding this comment

Uh oh!

anutosh491 Feb 22, 2023

Choose a reason for hiding this comment

Uh oh!

czgdp1807 Feb 22, 2023

Choose a reason for hiding this comment

Uh oh!

anutosh491 Feb 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

czgdp1807 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

anutosh491 commented Feb 23, 2023

Uh oh!

czgdp1807 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

certik commented Feb 25, 2023

Uh oh!

anutosh491 commented Feb 26, 2023

Uh oh!

Uh oh!

anutosh491 Feb 22, 2023 •

edited

Loading