-
Notifications
You must be signed in to change notification settings - Fork 164
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
fix: check if external symbol before checking from same scope. #2434
base: main
Are you sure you want to change the base?
Conversation
I thought the ExternalSymbol should still be part of the current symbol table. I will need to investigate this. Also let's add a test that now works. |
5b3b78c
to
56b964d
Compare
// If the symbol is an ExternalSymbol, then omit the check. | ||
if (ASR::is_a<ASR::ExternalSymbol_t>(*symbol)) { | ||
return; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this change is not correct --- I think the ASR is incorrect if the ExternalSymbol is not present in the current scope. So there is a bug in the way we construct the ASR that we need to fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The symbol is there in the current_symtab
imported as ExternalSymbol
. But, x.m_derived_type
points to actual definition of the class which is inside another module, that is why symtab_in_scope
fails. I think ASR is correct as the External Symbol is there in the current scope.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The proper fix is described at lfortran/lfortran#2957.
ExternalSymbol
, it should omit the same scope check in ASR Verify.