Skip to content
This repository was archived by the owner on Apr 14, 2022. It is now read-only.

Do not evaluate library functions with various arguments - #1047

Merged
Mikhail Arkhipov (MikhailArkhipov) merged 42 commits into
microsoft:masterfrom
MikhailArkhipov:skipbody
May 8, 2019
Merged

Mikhail Arkhipov (MikhailArkhipov) merged 42 commits into
microsoft:masterfrom
MikhailArkhipov:skipbody

Conversation

@MikhailArkhipov

Copy link
Copy Markdown

We do evaluate function body once and if we are able to figure out return type, we have it. Walking function body with assorted arguments, even with caching takes quite a bit of time for little bang for the buck.

This does not apply to user code.

MikhailArkhipov added 30 commits March 1, 2019 09:51
This reverts commit 7ffc9db.
// Try and evaluate with specific arguments. Note that it does not
// make sense to evaluate stubs since they already should be annotated.
if (fn.DeclaringModule is IDocument doc && fd?.Ast == doc.GetAnyAst()) {
if (EvaluateFunctionBody(fn) && fn.DeclaringModule is IDocument doc && fd?.Ast == doc.GetAnyAst()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We can call EvaluateFunctionBody after checking fn.DeclaringModule is IDocument doc && fd?.Ast == doc.GetAnyAst()

@MikhailArkhipov
Mikhail Arkhipov (MikhailArkhipov) merged commit 6fb450b into microsoft:master May 8, 2019
Jake Bailey (jakebailey) pushed a commit to jakebailey/python-language-server that referenced this pull request Nov 1, 2019
)

* Fix microsoft#668 (partial)

* Tests

* Revert "Tests"

This reverts commit 7ffc9db.

* Exp

* Limit concurrency

* Concurrency limit

* Drop cache after analysis

* Fix regression

* Fix test

* Limit body analysis in libraries

* More skipping

* Fix test

* Walk library functions once

* Add comment
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants