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

Avoid expensive list/tuple multiplication operations #2228

Merged
merged 5 commits into from
Jun 30, 2023

Conversation

jacobtylerwalls
Copy link
Member

Type of Changes

Type
βœ“ πŸ› Bug fix

Description

Closes pylint-dev/pylint#8748

Similar approach to #1610, instead of adding this guard to safe_infer, since it seems better to catch this as close to the source as possible instead of all of the places that call infer().

@codecov
Copy link

codecov bot commented Jun 27, 2023

Codecov Report

Merging #2228 (3a873c6) into main (8d57ce2) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2228   +/-   ##
=======================================
  Coverage   92.93%   92.93%           
=======================================
  Files          95       95           
  Lines       10921    10924    +3     
=======================================
+ Hits        10149    10152    +3     
  Misses        772      772           
Flag Coverage Ξ”
linux 92.74% <100.00%> (+<0.01%) ⬆️
pypy 91.07% <100.00%> (+<0.01%) ⬆️
windows 92.50% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Ξ”
astroid/protocols.py 90.68% <100.00%> (+0.06%) ⬆️

astroid/protocols.py Outdated Show resolved Hide resolved
@@ -167,6 +167,9 @@ def _multiply_seq_by_int(
context: InferenceContext,
) -> _TupleListNodeT:
node = self.__class__(parent=opnode)
if isinstance(other.value, int) and other.value > 1e8:
node.elts = [nodes.Const(NotImplemented)]
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be Uninferable instead of NotImplemented ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I suppose that's better than implying there's one element in the list.

DanielNoord
DanielNoord previously approved these changes Jun 28, 2023
astroid/protocols.py Outdated Show resolved Hide resolved
@Pierre-Sassoulas Pierre-Sassoulas merged commit 1a318a0 into pylint-dev:main Jun 30, 2023
github-actions bot pushed a commit that referenced this pull request Jun 30, 2023
@jacobtylerwalls jacobtylerwalls deleted the memory-error branch June 30, 2023 20:45
jacobtylerwalls added a commit that referenced this pull request Jul 1, 2023
(cherry picked from commit 1a318a0)

Co-authored-by: Jacob Walls <jacobtylerwalls@gmail.com>
@Pierre-Sassoulas Pierre-Sassoulas added backported Assigned once the backport is done and removed backport maintenance/2.15.x labels Feb 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported Assigned once the backport is done Bug πŸͺ³ topic-performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Long arguments in a function cause a fatal error
3 participants