Skip to content

New semantic analyzer: abstract classes #6423

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 10 commits into from
Feb 18, 2019
Merged

New semantic analyzer: abstract classes #6423

merged 10 commits into from
Feb 18, 2019

Conversation

JukkaL
Copy link
Collaborator

@JukkaL JukkaL commented Feb 18, 2019

This fixes abstract classes by moving determination of abstract
attributes to a new pass that happens between the main semantic
analysis pass and type checking. The new pass is added mostly
to simplify the main semantic analysis pass.

This also adds simple type inference for decorated functions from
the old semantic analysis pass 3. This fixes a test case where
we need access to the signature of an abstract method when type
checking the module top level. Since module top levels can't be
deferred during type checking, the simple type inference is needed
to remain compatible with the old semantic analyzer.

Fixes #6322.

@JukkaL JukkaL requested a review from ilevkivskyi February 18, 2019 11:30
Copy link
Member

@ilevkivskyi ilevkivskyi left a comment

Choose a reason for hiding this comment

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

Looks good! I didn't check the implementations carefully since IIUC you basically copied them from old third pass.

@@ -56,7 +59,8 @@ def semantic_analysis_for_scc(graph: 'Graph', scc: List[str]) -> None:
# before functions. This limitation is unlikely to go away soon.
process_top_levels(graph, scc)
process_functions(graph, scc)
check_type_arguments(graph, scc)
check_type_arguments(graph, scc, errors)
process_abstract(graph, scc, errors)
Copy link
Member

Choose a reason for hiding this comment

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

Bikeshedding: I would give it a longer name, like process_abstract_status.

return None


def find_fixed_callable_return(expr: Expression) -> Optional[CallableType]:
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add a docstring while you are at it?

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.

New semantic analyzer: abstractness calculations
2 participants