Skip to content
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

Changes in imports and one minor fix for globals. #8

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

sifislag
Copy link

@sifislag sifislag commented Feb 8, 2019

Hello,

I made some changes in imports, in order to model the 'from A import B' instructions correctly.

Overview of what changed:

from package.sub1.sub2 import a

would be translated to(in pseudo code):

temp = import package
temp2 = temp (get) sub1
temp3 = temp2 (get) sub2
a = temp3 (get) a

Now the same snippet will be translated to:

temp = import package.sub1.sub2
a = temp (special get) a

Note that 'a' could be either a module, a sub-package or something defined in the global scope of the package (at the global scope of the __init__ file).

I created a new instruction PythonImportFromGetInstruction which extends SSAGetInstruction so it shouldn't break anything on your end.

The need to differentiate between the two comes from the fact that an attribute access instruction (modeled as a normal field get instruction) should not be able to read the sub-packages or the modules of a package object if they are not available at the global scope of the packages' __init__ file. When using the "from.. import..." instructions however you also need to be able to access modules and sub-packages not available in the global scope of the __init__ file. For this we added the new instruction.

For the same reason we removed all the additional get instructions and changed the 'from' part to be just like normal imports (contain the whole path to the package/module). This may break something on your part. Another reason for this change is that explicit relative imports where very ugly with multiple get instructions.

It also includes a minor fix for globals in analyzing multiple files.

Thanks,
Sifis.

Copy link
Collaborator

@khatchad khatchad left a comment

Choose a reason for hiding this comment

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

Please revert formatting changes so that conflicts could be more easily resolved.

@@ -596,7 +631,8 @@ protected boolean doVisit(CAstNode n, WalkContext context, CAstVisitor<WalkConte
context.setValue(n, result);
return true;

} else if(n.getKind() == CAstNode.GLOBAL_DECL){
}
else if(n.getKind() == CAstNode.GLOBAL_DECL){
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we revert these formatting changes so that conflicts could be more easily resolved?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants