Skip to content

Implement goto in LPython #1163

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

Merged
merged 8 commits into from
Sep 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions integration_tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,7 @@ RUN(NAME generics_list_01 LABELS cpython llvm)
RUN(NAME test_statistics LABELS cpython llvm)
RUN(NAME test_str_attributes LABELS cpython llvm)
RUN(NAME kwargs_01 LABELS cpython llvm)
RUN(NAME test_01_goto LABELS cpython llvm c)

RUN(NAME func_inline_01 LABELS llvm wasm)
RUN(NAME func_static_01 LABELS cpython llvm c wasm)
Expand Down
35 changes: 35 additions & 0 deletions integration_tests/test_01_goto.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
from ltypes import with_goto, i32

@with_goto
def f() -> i32:
i:i32
for i in range(10):
if i == 5:
goto .end

label .end
assert i == 5
return i

@with_goto
def g(size: i32) -> i32:
i:i32

i = 0
label .loop
if i >= size:
goto .end
i += 1
goto .loop

label .end
return i

def test_goto():
print(f())
print(g(10))
print(g(20))
assert g(30) == 30
assert g(40) == 40

test_goto()
4 changes: 2 additions & 2 deletions src/libasr/ASR.asdl
Original file line number Diff line number Diff line change
Expand Up @@ -173,10 +173,10 @@ stmt
-- GoTo points to a GoToTarget with the corresponding target_id within
-- the same procedure. We currently use `int` IDs to link GoTo with
-- GoToTarget to avoid issues with serialization.
| GoTo(int target_id)
| GoTo(int target_id, identifier name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think GoTo should just reference using an ID. The name should go into GoToTarget.

Suggested change
| GoTo(int target_id, identifier name)
| GoTo(int target_id)

Or am I missing something?

Copy link
Collaborator

@czgdp1807 czgdp1807 Sep 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well in C backend we need the name to produce, label name_of_the_label or goto name_of_the_label. So we need the name to be stored in both GoTo and GoToTarget.

-- An empty statement, a target of zero or more GoTo statements
-- the `id` is only unique within a procedure
| GoToTarget(int id)
| GoToTarget(int id, identifier name)
| If(expr test, stmt* body, stmt* orelse)
| IfArithmetic(expr test, int lt_label, int eq_label, int gt_label)
| Print(expr? fmt, expr* values, expr? separator, expr? end)
Expand Down
3 changes: 2 additions & 1 deletion src/libasr/asr_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -1746,7 +1746,8 @@ class ReplaceReturnWithGotoVisitor: public ASR::BaseStmtReplacer<ReplaceReturnWi
}

void replace_Return(ASR::Return_t* x) {
*current_stmt = ASRUtils::STMT(ASR::make_GoTo_t(al, x->base.base.loc, goto_label));
*current_stmt = ASRUtils::STMT(ASR::make_GoTo_t(al, x->base.base.loc, goto_label,
s2c(al, "__" + std::to_string(goto_label))));
has_replacement_happened = true;
}

Expand Down
8 changes: 8 additions & 0 deletions src/libasr/codegen/asr_to_c.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1126,6 +1126,14 @@ R"(
src = "strlen(" + src + ")";
}

void visit_GoTo(const ASR::GoTo_t &x) {
std::string indent(indentation_level*indentation_spaces, ' ');
src = indent + "goto " + std::string(x.m_name) + ";\n";
}

void visit_GoToTarget(const ASR::GoToTarget_t &x) {
src = std::string(x.m_name) + ":\n";
}
};

Result<std::string> asr_to_c(Allocator &al, ASR::TranslationUnit_t &asr,
Expand Down
57 changes: 53 additions & 4 deletions src/lpython/semantics/python_ast_to_asr.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include <fstream>
#include <iostream>
#include <map>
#include <set>
#include <memory>
#include <string>
#include <cmath>
Expand Down Expand Up @@ -2843,7 +2844,6 @@ class SymbolTableVisitor : public CommonVisitor<SymbolTableVisitor> {
bool current_procedure_interface = false;
bool overload = false;
bool vectorize = false, is_inline = false, is_static = false;

Vec<ASR::ttype_t*> tps;
tps.reserve(al, x.m_args.n_args);
bool is_restriction = false;
Expand All @@ -2866,6 +2866,8 @@ class SymbolTableVisitor : public CommonVisitor<SymbolTableVisitor> {
vectorize = true;
} else if (name == "restriction") {
is_restriction = true;
} else if (name == "with_goto") {
// TODO: Use goto attribute in function?
} else if (name == "inline") {
is_inline = true;
} else if (name == "static") {
Expand Down Expand Up @@ -3211,11 +3213,14 @@ class BodyVisitor : public CommonVisitor<BodyVisitor> {

public:
ASR::asr_t *asr;
std::map<std::string, std::tuple<int64_t, bool, Location>> goto_name2id;
int64_t gotoids;


BodyVisitor(Allocator &al, ASR::asr_t *unit, diag::Diagnostics &diagnostics,
bool main_module, std::map<int, ASR::symbol_t*> &ast_overload)
: CommonVisitor(al, nullptr, diagnostics, main_module, ast_overload, ""), asr{unit}
: CommonVisitor(al, nullptr, diagnostics, main_module, ast_overload, ""), asr{unit},
gotoids{0}
{}

// Transforms statements to a list of ASR statements
Expand Down Expand Up @@ -3300,6 +3305,8 @@ class BodyVisitor : public CommonVisitor<BodyVisitor> {
}

void visit_FunctionDef(const AST::FunctionDef_t &x) {
goto_name2id.clear();
gotoids = 0;
SymbolTable *old_scope = current_scope;
ASR::symbol_t *t = current_scope->get_symbol(x.m_name);
if (ASR::is_a<ASR::Function_t>(*t)) {
Expand All @@ -3320,6 +3327,13 @@ class BodyVisitor : public CommonVisitor<BodyVisitor> {
}
current_scope = old_scope;
tmp = nullptr;

for( auto itr: goto_name2id ) {
if( !std::get<1>(itr.second) ) {
throw SemanticError("Label '" + itr.first + "' is not defined in '"
+ std::string(x.m_name) + "'", std::get<2>(itr.second));
}
}
}

void visit_Import(const AST::Import_t &/*x*/) {
Expand Down Expand Up @@ -3969,7 +3983,36 @@ class BodyVisitor : public CommonVisitor<BodyVisitor> {
void visit_Attribute(const AST::Attribute_t &x) {
if (AST::is_a<AST::Name_t>(*x.m_value)) {
std::string value = AST::down_cast<AST::Name_t>(x.m_value)->m_id;
if( value == "label" ) {
std::string labelname = x.m_attr;
if( goto_name2id.find(labelname) == goto_name2id.end() ) {
goto_name2id[labelname] = std::make_tuple(gotoids, true, x.base.base.loc);
gotoids += 1;
} else if( !std::get<1>(goto_name2id[labelname]) ) {
goto_name2id[labelname] = std::make_tuple(
std::get<0>(goto_name2id[labelname]),
true,
std::get<2>(goto_name2id[labelname])
);
}
int id = std::get<0>(goto_name2id[labelname]);
tmp = ASR::make_GoToTarget_t(al, x.base.base.loc, id, x.m_attr);
return ;
}

if (value == "goto"){
std::string labelname = std::string(x.m_attr);
if( goto_name2id.find(labelname) == goto_name2id.end() ) {
goto_name2id[labelname] = std::make_tuple(gotoids, false, x.base.base.loc);
gotoids += 1;
}
int id = std::get<0>(goto_name2id[labelname]);
tmp = ASR::make_GoTo_t(al, x.base.base.loc, id, x.m_attr);
return ;
}

ASR::symbol_t *t = current_scope->resolve_symbol(value);

if (!t) {
throw SemanticError("'" + value + "' is not defined in the scope",
x.base.base.loc);
Expand Down Expand Up @@ -4543,8 +4586,14 @@ class BodyVisitor : public CommonVisitor<BodyVisitor> {
return;
}
this->visit_expr(*x.m_value);
ASRUtils::EXPR(tmp);
tmp = nullptr;

// If tmp is a statement and not an expression
// never cast into expression using ASRUtils::EXPR
// Just ignore and exit the function naturally.
if( !ASR::is_a<ASR::stmt_t>(*tmp) ) {
LFORTRAN_ASSERT(ASR::is_a<ASR::expr_t>(*tmp));
tmp = nullptr;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the purpose of these lines, both the original and the new ones?

Are we checking that the result is an expression? If so, you can use an assert for ASR::is_a (or perhaps down_cast, since is_a might require a particular class).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the scene is that goto.end is represented as [(Expr (Attribute (Name goto Load) end Load))] at AST level. So, now when we will complete visiting Attribute, tmp will either point to a ASR expression or a ASR statement (because GoTo in ASR is a statement). So in case we get something like, goto.label then tmp will point to a statement in which case we should exit the function naturally and the transform_stmts before this node will include that GoTo statement. However if tmp is not pointing to a statement then I replaced ASRUtils::EXPR(tmp) with an LFORTRAN_ASSERT.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If so, you can use an assert for ASR::is_a (or perhaps down_cast, since is_a might require a particular class).

Assert with ASR::is_a works fine.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please document this as a comment? Next person to see that code will have the same question, including me tomorrow.

}


Expand Down
Loading