Skip to content

Make the fine-grained mergechecker work again - #8313

Merged
msullivan merged 3 commits into
masterfrom
fg-consistency
Jan 23, 2020
Merged

msullivan merged 3 commits into
masterfrom
fg-consistency

Conversation

@msullivan

Copy link
Copy Markdown
Collaborator

This fixes some stuff in the mergechecker that breaks with current
mypy's and also some bugs that the mergechecker caught. The entire
fine-grained test suite now passes with the merge checker on, which I
don't think has ever been true before.

This means that we could turn it on by default, but it doubles the
runtime of the fine-grained tests (from 90s CPU time to 180s CPU time
on my laptop), so I've left it off for now.

The motivation here is that I knew intersect_callable's creation of
types during typechecking used to run afoul of the consistency checker
and so I was nervous that #8305 would cause more problems by adding
more logic of that kind. It no longer does, probably as a result of
the semantic analyzer rewrite, so I think we are in the clear on that.

This fixes some stuff in the mergechecker that breaks with current
mypy's and also some bugs that the mergechecker caught. The entire
fine-grained test suite now passes with the merge checker on, which I
don't think has ever been true before.

This means that we could turn it on by default, but it doubles the
runtime of the fine-grained tests (from 90s CPU time to 180s CPU time
on my laptop), so I've left it off for now.

The motivation here is that I knew intersect_callable's creation of
types during typechecking used to run afoul of the consistency checker
and so I was nervous that #8305 would cause more problems by adding
more logic of that kind. It no longer does, probably as a result of
the semantic analyzer rewrite, so I think we are in the clear on that.

@ilevkivskyi ilevkivskyi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great! I just have one suggestion.

Comment thread mypy/semanal_main.py Outdated
# Clear out some stale data to avoid memory leaks and astmerge
# validity check confusion
analyzer.statement = None
delattr(analyzer, 'cur_mod_node')

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why not del analyzer.cur_mod_node?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Because I forgot how to write python apparently

@msullivan
msullivan merged commit 9c2b8b3 into master Jan 23, 2020
@msullivan
msullivan deleted the fg-consistency branch January 23, 2020 21:46
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