Skip to content

Implemented ASR checks for function dependencies #2167

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
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
049fbc6
fix: check dependencies are from same symbol table
arteevraina Jul 16, 2023
fc86d98
fix: remove print statement
arteevraina Jul 16, 2023
42d5146
fix: update verify logic
arteevraina Jul 17, 2023
4805b69
fix: update logic for verification of dependencies
arteevraina Jul 18, 2023
71bee66
refactor: remove cout
arteevraina Jul 18, 2023
70f2f9c
fix: get the parent symtab of x
arteevraina Jul 18, 2023
a2e802b
fix: skip the symbols which are nullptr or of type External Symbol
arteevraina Jul 18, 2023
8d8bbc7
fix: update cmakelists
arteevraina Jul 19, 2023
c1116e3
fix: check while adding dependencies to make sure they are from same …
arteevraina Jul 26, 2023
b531009
fix: asr verify passes for callback_01 test
arteevraina Aug 11, 2023
2c436ba
fix: tests relating to external symbols and function dependencies
arteevraina Aug 12, 2023
6fc8a6d
fix: condition for checking intrinsic function
arteevraina Aug 15, 2023
f1d77ae
Allow global scope dependencies as well
certik Aug 18, 2023
f94840f
Merge main
czgdp1807 Aug 25, 2023
47d0f5d
Robust check for making sure that dependency isn't defined inside the…
czgdp1807 Aug 25, 2023
456071b
fix: add robust checking using resolve symbol when symbol is nested
arteevraina Aug 26, 2023
ca26122
fix: improved check by checking only in parent scopes and not in curr…
arteevraina Aug 26, 2023
dd2d3ab
fix: improved checks for dependencies in asr verify
arteevraina Aug 27, 2023
681514f
Check if external symbol is not nested inside the current_scope
czgdp1807 Aug 28, 2023
433d944
fix: applied correct checks in ast to asr
arteevraina Aug 28, 2023
14632b4
refactor: added function to check for current scope in asr utils
arteevraina Aug 28, 2023
0491339
refactor: remove unused function
arteevraina Aug 28, 2023
d943c5a
updated references
arteevraina Aug 28, 2023
f97063f
Merge branch 'main' into checks-for-same-symtab-dependencies
arteevraina Aug 28, 2023
b1d00d4
remove nofast
arteevraina Aug 28, 2023
c1c8bd2
Merge branch 'main' into checks-for-same-symtab-dependencies
arteevraina Aug 29, 2023
f1e1c2f
refactor: function call & subroutine call dependencies
arteevraina Sep 2, 2023
261c187
feat: sync libasr
arteevraina Oct 4, 2023
6f7c8fa
fix: update branch
arteevraina Oct 4, 2023
ccaa3ec
dev: udpate branch
arteevraina Oct 4, 2023
f68f71a
fix: updated all references
arteevraina Oct 4, 2023
0c7e8b5
dev: sync libasr with lfortran
arteevraina Oct 18, 2023
74e4251
test: update references
arteevraina Oct 18, 2023
1801bb5
tests: update references
arteevraina Oct 18, 2023
895f411
fix: set asr_owner when visiting For
arteevraina Oct 24, 2023
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
16 changes: 12 additions & 4 deletions src/libasr/asr_utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -616,7 +616,9 @@ bool use_overloaded(ASR::expr_t* left, ASR::expr_t* right,
}
}
}
current_function_dependencies.push_back(al, s2c(al, matched_func_name));
if (ASRUtils::symbol_parent_symtab(a_name) == curr_scope->parent) {
current_function_dependencies.push_back(al, s2c(al, matched_func_name));
}
ASRUtils::insert_module_dependency(a_name, al, current_module_dependencies);
ASRUtils::set_absent_optional_arguments_to_null(a_args, func, al);
asr = ASRUtils::make_FunctionCall_t_util(al, loc, a_name, sym,
Expand Down Expand Up @@ -699,7 +701,9 @@ void process_overloaded_unary_minus_function(ASR::symbol_t* proc, ASR::expr_t* o
}
}
}
current_function_dependencies.push_back(al, s2c(al, matched_func_name));
if (ASRUtils::symbol_parent_symtab(a_name) == curr_scope->parent) {
current_function_dependencies.push_back(al, s2c(al, matched_func_name));
}
ASRUtils::insert_module_dependency(a_name, al, current_module_dependencies);
ASRUtils::set_absent_optional_arguments_to_null(a_args, func, al);
asr = ASRUtils::make_FunctionCall_t_util(al, loc, a_name, proc,
Expand Down Expand Up @@ -870,7 +874,9 @@ void process_overloaded_assignment_function(ASR::symbol_t* proc, ASR::expr_t* ta
if( a_name == nullptr ) {
err("Unable to resolve matched subroutine for assignment overloading, " + matched_subrout_name, loc);
}
current_function_dependencies.push_back(al, s2c(al, matched_subrout_name));
if (ASRUtils::symbol_parent_symtab(a_name) == curr_scope->parent) {
current_function_dependencies.push_back(al, s2c(al, matched_subrout_name));
}
ASRUtils::insert_module_dependency(a_name, al, current_module_dependencies);
ASRUtils::set_absent_optional_arguments_to_null(a_args, subrout, al);
asr = ASRUtils::make_SubroutineCall_t_util(al, loc, a_name, sym,
Expand Down Expand Up @@ -1010,7 +1016,9 @@ bool use_overloaded(ASR::expr_t* left, ASR::expr_t* right,
} else {
return_type = ASRUtils::expr_type(func->m_return_var);
}
current_function_dependencies.push_back(al, s2c(al, matched_func_name));
if (ASRUtils::symbol_parent_symtab(a_name) == curr_scope->parent) {
current_function_dependencies.push_back(al, s2c(al, matched_func_name));
}
ASRUtils::insert_module_dependency(a_name, al, current_module_dependencies);
ASRUtils::set_absent_optional_arguments_to_null(a_args, func, al);
asr = ASRUtils::make_FunctionCall_t_util(al, loc, a_name, sym,
Expand Down
15 changes: 14 additions & 1 deletion src/libasr/asr_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,17 @@ static inline std::pair<char**, size_t> symbol_dependencies(const ASR::symbol_t
}
}

static inline bool is_present_in_current_scope(ASR::ExternalSymbol_t* external_symbol, SymbolTable* current_scope) {
SymbolTable* scope = external_symbol->m_parent_symtab;
while (scope != nullptr) {
if (scope->get_counter() == current_scope->get_counter()) {
return true;
}
scope = scope->parent;
}
return false;
}

static inline SymbolTable *symbol_parent_symtab(const ASR::symbol_t *f)
{
switch (f->type) {
Expand Down Expand Up @@ -3023,7 +3034,9 @@ class ReplaceArgVisitor: public ASR::BaseExprReplacer<ReplaceArgVisitor> {
default:
break;
}
current_function_dependencies.push_back(al, ASRUtils::symbol_name(new_es));
if (ASRUtils::symbol_parent_symtab(new_es) == current_scope->parent) {
current_function_dependencies.push_back(al, ASRUtils::symbol_name(new_es));
}
ASRUtils::insert_module_dependency(new_es, al, current_module_dependencies);
x->m_name = new_es;
}
Expand Down
41 changes: 39 additions & 2 deletions src/libasr/asr_verify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,32 @@ class VerifyVisitor : public BaseWalkVisitor<VerifyVisitor>
verify_unique_dependencies(x.m_dependencies, x.n_dependencies,
x.m_name, x.base.base.loc);

// Get the x parent symtab.
SymbolTable *sym = x.m_symtab->parent;

// Dependencies of the function should be from function's parent symbol table.
for( size_t i = 0; i < x.n_dependencies; i++ ) {
std::string found_dep = x.m_dependencies[i];

// Get the symbol of the found_dep.
ASR::symbol_t* dep_sym = sym->get_symbol(found_dep);

if (dep_sym == nullptr) {
// The symbol `dep_sym` is not in the parent symbol table. For
// now we allow one exception: it can also be in the global scope.
ASR::symbol_t* dep_sym_global = sym->resolve_symbol(found_dep);

if (dep_sym_global != nullptr && ASRUtils::symbol_parent_symtab(dep_sym_global)->parent == nullptr) {
// This is a global scope symbol, which we allow for now
} else {
// This is not a global scope symbol and not in the parent symbol table,
// Return an error:
require(dep_sym != nullptr,
"Dependency " + found_dep + " was not found in the parent symbol table " + std::string(x.m_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 am trying to understand the logic here.

Are we ensuring that all dependencies are from the same parent symbol table?

It seems this logic is testing that all the symbols (except ExternalSymbol) are NOT from the parent symbol table. Tests pass, so I guess this is the correct logic, but I don't understand it. Once I understand it, let's document this as a comment.


// Check if there are unnecessary dependencies
// present in the dependency list of the function
for( size_t i = 0; i < x.n_dependencies; i++ ) {
Expand Down Expand Up @@ -868,7 +894,12 @@ class VerifyVisitor : public BaseWalkVisitor<VerifyVisitor>
}
}

function_dependencies.push_back(std::string(ASRUtils::symbol_name(x.m_name)));
if ((current_symtab->get_counter() != ASRUtils::symbol_parent_symtab(x.m_name)->get_counter()) ||
(ASRUtils::symbol_parent_symtab(x.m_name)->parent == nullptr)) {
// add to dependencies
function_dependencies.push_back(std::string(ASRUtils::symbol_name(x.m_name)));
}

if( ASR::is_a<ASR::ExternalSymbol_t>(*x.m_name) ) {
ASR::ExternalSymbol_t* x_m_name = ASR::down_cast<ASR::ExternalSymbol_t>(x.m_name);
if( x_m_name->m_external && ASR::is_a<ASR::Module_t>(*ASRUtils::get_asr_owner(x_m_name->m_external)) ) {
Expand Down Expand Up @@ -1001,7 +1032,13 @@ class VerifyVisitor : public BaseWalkVisitor<VerifyVisitor>
void visit_FunctionCall(const FunctionCall_t &x) {
require(x.m_name,
"FunctionCall::m_name must be present");
function_dependencies.push_back(std::string(ASRUtils::symbol_name(x.m_name)));
// Check x.m_name is from parent sym tab.
if ((ASRUtils::symbol_parent_symtab(x.m_name)->get_counter() != current_symtab->get_counter()) ||
(ASRUtils::symbol_parent_symtab(x.m_name)->parent == nullptr)) {
// add to dependencies
function_dependencies.push_back(std::string(ASRUtils::symbol_name(x.m_name)));
}

if( ASR::is_a<ASR::ExternalSymbol_t>(*x.m_name) ) {
ASR::ExternalSymbol_t* x_m_name = ASR::down_cast<ASR::ExternalSymbol_t>(x.m_name);
if( x_m_name->m_external && ASR::is_a<ASR::Module_t>(*ASRUtils::get_asr_owner(x_m_name->m_external)) ) {
Expand Down
8 changes: 6 additions & 2 deletions src/libasr/pass/instantiate_template.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,9 @@ class SymbolInstantiator : public ASR::BaseExprStmtDuplicator<SymbolInstantiator
ASR::down_cast<ASR::Function_t>(name2));
context_map[ASRUtils::symbol_name(name2)] = ASRUtils::symbol_name(name);
}
dependencies.push_back(al, ASRUtils::symbol_name(name));
if (ASRUtils::symbol_parent_symtab(name) == template_scope->parent) {
dependencies.push_back(al, ASRUtils::symbol_name(name));
}
return ASRUtils::make_FunctionCall_t_util(al, x->base.base.loc, name, x->m_original_name,
args.p, args.size(), type, value, dt);
}
Expand All @@ -370,7 +372,9 @@ class SymbolInstantiator : public ASR::BaseExprStmtDuplicator<SymbolInstantiator
name = nested_t.instantiate_symbol(name2);
context_map[ASRUtils::symbol_name(name2)] = ASRUtils::symbol_name(name);
}
dependencies.push_back(al, ASRUtils::symbol_name(name));
if (ASRUtils::symbol_parent_symtab(name) == template_scope->parent) {
dependencies.push_back(al, ASRUtils::symbol_name(name));
}
return ASRUtils::make_SubroutineCall_t_util(al, x->base.base.loc, name /* change this */,
x->m_original_name, args.p, args.size(), dt, nullptr, false);
}
Expand Down
75 changes: 71 additions & 4 deletions src/libasr/pass/pass_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ namespace LCompilers {
bool fill_function_dependencies;
bool fill_module_dependencies;
bool fill_variable_dependencies;
SymbolTable* current_scope;

public:

Expand All @@ -260,10 +261,13 @@ namespace LCompilers {
function_dependencies.n = 0;
module_dependencies.n = 0;
variable_dependencies.n = 0;
current_scope = nullptr;
}

void visit_Function(const ASR::Function_t& x) {
ASR::Function_t& xx = const_cast<ASR::Function_t&>(x);
SymbolTable* current_scope_copy = current_scope;
current_scope = xx.m_symtab;
SetChar function_dependencies_copy;
function_dependencies_copy.from_pointer_n_copy(al, function_dependencies.p, function_dependencies.size());
function_dependencies.n = 0;
Expand All @@ -278,6 +282,7 @@ namespace LCompilers {
function_dependencies_copy.p,
function_dependencies_copy.size()
);
current_scope = current_scope_copy;
}

void visit_Module(const ASR::Module_t& x) {
Expand Down Expand Up @@ -314,9 +319,38 @@ namespace LCompilers {
}

void visit_FunctionCall(const ASR::FunctionCall_t& x) {
if( fill_function_dependencies ) {
function_dependencies.push_back(al, ASRUtils::symbol_name(x.m_name));
if( fill_function_dependencies &&
ASRUtils::symbol_symtab(x.m_name) != nullptr &&
ASRUtils::symbol_parent_symtab(x.m_name)->get_counter() !=
current_scope->get_counter()) {
if (ASR::is_a<ASR::Function_t>(*x.m_name)) {
ASR::Function_t *f = ASR::down_cast2<ASR::Function_t>(current_scope->asr_owner);

// Check is x.m_name is not an argument.
bool is_arg = false;

for (size_t i=0; i<f->n_args; i++) {
ASR::Var_t *arg = ASR::down_cast<ASR::Var_t>(f->m_args[i]);

if (arg->m_v == x.m_name) {
is_arg = true;
break;
}
}

if (!is_arg) {
function_dependencies.push_back(al, ASRUtils::symbol_name(x.m_name));
}
}
}

if (fill_function_dependencies && ASR::is_a<ASR::ExternalSymbol_t>(*x.m_name)) {
ASR::ExternalSymbol_t* external_symbol = ASR::down_cast<ASR::ExternalSymbol_t>(x.m_name);
if (!ASRUtils::is_present_in_current_scope(external_symbol, current_scope)) {
function_dependencies.push_back(al, ASRUtils::symbol_name(x.m_name));
}
}

if( ASR::is_a<ASR::ExternalSymbol_t>(*x.m_name) &&
fill_module_dependencies ) {
ASR::ExternalSymbol_t* x_m_name = ASR::down_cast<ASR::ExternalSymbol_t>(x.m_name);
Expand All @@ -328,9 +362,42 @@ namespace LCompilers {
}

void visit_SubroutineCall(const ASR::SubroutineCall_t& x) {
if( fill_function_dependencies ) {
function_dependencies.push_back(al, ASRUtils::symbol_name(x.m_name));
if( fill_function_dependencies &&
ASRUtils::symbol_symtab(x.m_name) != nullptr &&
ASRUtils::symbol_parent_symtab(x.m_name)->get_counter() !=
current_scope->get_counter()) {
if (ASR::is_a<ASR::Function_t>(*x.m_name)) {
// Get the function type from x.
ASR::Function_t *f = ASR::down_cast2<ASR::Function_t>(current_scope->asr_owner);

// Check is x.m_name is not an argument.
bool is_arg = false;

for (size_t i=0; i<f->n_args; i++) {
// Cast f->m_args[i] to Var.
ASR::Var_t *arg = ASR::down_cast<ASR::Var_t>(f->m_args[i]);

// Check if x.m_name is an argument.
if (arg->m_v == x.m_name) {
is_arg = true;
break;
}
}

if (!is_arg) {
function_dependencies.push_back(al, ASRUtils::symbol_name(x.m_name));
}
}
}

if (fill_function_dependencies && ASR::is_a<ASR::ExternalSymbol_t>(*x.m_name)) {
ASR::ExternalSymbol_t* external_symbol = ASR::down_cast<ASR::ExternalSymbol_t>(x.m_name);

if (!ASRUtils::is_present_in_current_scope(external_symbol, current_scope)) {
function_dependencies.push_back(al, ASRUtils::symbol_name(x.m_name));
}
}

if( ASR::is_a<ASR::ExternalSymbol_t>(*x.m_name) &&
fill_module_dependencies ) {
ASR::ExternalSymbol_t* x_m_name = ASR::down_cast<ASR::ExternalSymbol_t>(x.m_name);
Expand Down
27 changes: 24 additions & 3 deletions src/lpython/semantics/python_ast_to_asr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1325,15 +1325,34 @@ class CommonVisitor : public AST::BaseVisitor<Struct> {
args_new.reserve(al, func->n_args);
visit_expr_list_with_cast(func->m_args, func->n_args, args_new, args,
!ASRUtils::is_intrinsic_function2(func));
dependencies.push_back(al, ASRUtils::symbol_name(stemp));

// Check if it is inside the parent symbol scope.
if(ASRUtils::symbol_parent_symtab(stemp) == func->m_symtab->parent) {
dependencies.push_back(al, ASRUtils::symbol_name(stemp));
} else {
// Check if it is not an Intrinsic Function.
if( !ASRUtils::is_intrinsic_function2(func) ) {
// Check if it is an External Symbol.
if( ASR::is_a<ASR::ExternalSymbol_t>(*stemp) ) {
ASR::ExternalSymbol_t* es = ASR::down_cast<ASR::ExternalSymbol_t>(stemp);
// Only add dependency if both belong to same parent symbol table.
if(ASRUtils::symbol_parent_symtab(es->m_external) == func->m_symtab->parent) {
dependencies.push_back(al, ASRUtils::symbol_name(stemp));
}
}
}
}
return ASRUtils::make_FunctionCall_t_util(al, loc, stemp,
s_generic, args_new.p, args_new.size(),
a_type, value, nullptr);
} else {
Vec<ASR::call_arg_t> args_new;
args_new.reserve(al, func->n_args);
visit_expr_list_with_cast(func->m_args, func->n_args, args_new, args);
dependencies.push_back(al, ASRUtils::symbol_name(stemp));

if(ASRUtils::symbol_parent_symtab(stemp)->get_counter() != current_scope->get_counter()) {
dependencies.push_back(al, ASRUtils::symbol_name(stemp));
}
return ASRUtils::make_SubroutineCall_t_util(al, loc, stemp,
s_generic, args_new.p, args_new.size(), nullptr, nullptr, false);
}
Expand Down Expand Up @@ -1595,7 +1614,9 @@ class CommonVisitor : public AST::BaseVisitor<Struct> {
target_scope, target_scope, new_f, f);
}
dependencies.erase(s2c(al, func_name));
dependencies.push_back(al, s2c(al, new_func_name));
if(ASRUtils::symbol_parent_symtab(sym) == current_scope->parent) {
dependencies.push_back(al, s2c(al, new_func_name));
}
return t;
}

Expand Down
2 changes: 1 addition & 1 deletion tests/reference/asr-callback_01-df40fd5.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"outfile": null,
"outfile_hash": null,
"stdout": "asr-callback_01-df40fd5.stdout",
"stdout_hash": "5be73c5b09034604701853c55fffbdca38993aa3f92782e89a50c91e",
"stdout_hash": "d8283ff4af45372de119998a592b0995335f8d6ada869664e3306a22",
"stderr": null,
"stderr_hash": null,
"returncode": 0
Expand Down
2 changes: 1 addition & 1 deletion tests/reference/asr-callback_01-df40fd5.stdout
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,7 @@
[]
.false.
)
[func]
[]
[(Var 6 func)
(Var 6 arg)]
[(=
Expand Down
2 changes: 1 addition & 1 deletion tests/reference/asr-cast-435c233.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"outfile": null,
"outfile_hash": null,
"stdout": "asr-cast-435c233.stdout",
"stdout_hash": "601d0fda929bc1ee47ec591680476f69f32af980624d2fd504094091",
"stdout_hash": "a93c5a7141a3d175723f95e3422250bc69fa23fb1da39a6a132668dc",
"stderr": null,
"stderr_hash": null,
"returncode": 0
Expand Down
2 changes: 1 addition & 1 deletion tests/reference/asr-cast-435c233.stdout
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@
[]
.false.
)
[list]
[]
[]
[(=
(Var 3 s)
Expand Down
2 changes: 1 addition & 1 deletion tests/reference/asr-complex1-f26c460.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"outfile": null,
"outfile_hash": null,
"stdout": "asr-complex1-f26c460.stdout",
"stdout_hash": "f9bf5d60924825871eda0b6077447ff9650625d0d1f15c94f16ec0e6",
"stdout_hash": "79c039ef8f2b16c8518e689aaaf252a4dd5048a6a08df68a4453e7a8",
"stderr": null,
"stderr_hash": null,
"returncode": 0
Expand Down
7 changes: 1 addition & 6 deletions tests/reference/asr-complex1-f26c460.stdout
Original file line number Diff line number Diff line change
Expand Up @@ -368,12 +368,7 @@
[]
.false.
)
[complex@__lpython_overloaded_0__complex
complex@__lpython_overloaded_1__complex
complex@__lpython_overloaded_5__complex
complex@__lpython_overloaded_2__complex
complex@__lpython_overloaded_9__complex
complex@__lpython_overloaded_13__complex]
[]
[]
[(=
(Var 3 c)
Expand Down
Loading