Skip to content

Conversation

@carljm
Copy link
Member

@carljm carljm commented Nov 21, 2022

facebook-github-bot pushed a commit to facebookincubator/cinder that referenced this pull request Nov 21, 2022
Summary: Backport python/cpython#99654 to fix `inspect.getsource` handling of decorator calls with nested parens. Reviewed By: itamaro Differential Revision: D41438553 fbshipit-source-id: 420a90d
@JelleZijlstra
Copy link
Member

I think there's a simpler solution: remove the special cases for parentheses completely. That code is unnecessary because we don't emit NEWLINE tokens for newlines within parentheses. Instead we emit NL:

In [46]: def t(s): ...: return list(tokenize.tokenize(io.BytesIO(s.encode()).readline)) In [48]: t("""(x ...: )""") Out[48]: [TokenInfo(type=63 (ENCODING), string='utf-8', start=(0, 0), end=(0, 0), line=''), TokenInfo(type=54 (OP), string='(', start=(1, 0), end=(1, 1), line='(x\n'), TokenInfo(type=1 (NAME), string='x', start=(1, 1), end=(1, 2), line='(x\n'), TokenInfo(type=62 (NL), string='\n', start=(1, 2), end=(1, 3), line='(x\n'), TokenInfo(type=54 (OP), string=')', start=(2, 0), end=(2, 1), line=')'), TokenInfo(type=4 (NEWLINE), string='', start=(2, 1), end=(2, 2), line=''), TokenInfo(type=0 (ENDMARKER), string='', start=(3, 0), end=(3, 0), line='')] 

If the parens-handling code was necessary, we would see a similar bug with [] but we don't.

@carljm
Copy link
Member Author

carljm commented Dec 1, 2022

I think there's a simpler solution

Good call! I wondered about that, but decided to just stick with the less-invasive fix. Thanks for the nudge, I'll try just removing the paren-handling instead.

* main: (112 commits) pythongh-99894: Ensure the local names don't collide with the test file in traceback suggestion error checking (python#99895) pythongh-99612: Fix PyUnicode_DecodeUTF8Stateful() for ASCII-only data (pythonGH-99613) Doc: Add summary line to isolation_level & autocommit sqlite3.connect params (python#99917) pythonGH-98906 ```re``` module: ```search() vs. match()``` section should mention ```fullmatch()``` (pythonGH-98916) pythongh-89189: More compact range iterator (pythonGH-27986) bpo-47220: Document the optional callback parameter of weakref.WeakMethod (pythonGH-25491) pythonGH-99905: Fix output of misses in summarize_stats.py execution counts (pythonGH-99906) pythongh-99845: PEP 670: Convert PyObject macros to functions (python#99850) pythongh-99845: Use size_t type in __sizeof__() methods (python#99846) pythonGH-99877) Fix typo in exception message in `multiprocessing.pool` (python#99900) pythongh-87092: move all localsplus preparation into separate function called from assembler stage (pythonGH-99869) pythongh-99891: Fix infinite recursion in the tokenizer when showing warnings (pythonGH-99893) pythongh-99824: Document that sqlite3.connect implicitly open a transaction if autocommit=False (python#99825) pythonGH-81057: remove static state from suggestions.c (python#99411) Improve zip64 limit error message (python#95892) pythongh-98253: Break potential reference cycles in external code worsened by typing.py lru_cache (python#98591) pythongh-99127: Allow some features of syslog to the main interpreter only (pythongh-99128) pythongh-82836: fix private network check (python#97733) Docs: improve accuracy of socketserver reference (python#24767) ...
@carljm
Copy link
Member Author

carljm commented Dec 1, 2022

Updated.

I also wondered looking at this code whether we couldn't just use the AST lineno information for all objects (as is already done for classes, since #35113) and eliminate the need for BlockFinder altogether. But that's definitely a more involved change.

Copy link
Member

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

Thanks, looks good!

I feel this should be safe to backport to the bugfix branches, do you agree?

@carljm
Copy link
Member Author

carljm commented Dec 7, 2022

@JelleZijlstra Yes, I think this should be safe to backport.

@JelleZijlstra JelleZijlstra added needs backport to 3.10 only security fixes needs backport to 3.11 only security fixes labels Dec 7, 2022
@JelleZijlstra JelleZijlstra merged commit 68e4129 into python:main Dec 7, 2022
@miss-islington
Copy link
Contributor

Thanks @carljm for the PR, and @JelleZijlstra for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Sorry @carljm and @JelleZijlstra, I had trouble checking out the 3.11 backport branch.
Please retry by removing and re-adding the "needs backport to 3.11" label.
Alternatively, you can backport using cherry_picker on the command line.
cherry_picker 68e41295b8611a990de68f15c89f1eb3dea51867 3.11

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 7, 2022
…rce (pythonGH-99654) (cherry picked from commit 68e4129) Co-authored-by: Carl Meyer <carl@oddbird.net>
@bedevere-bot
Copy link

GH-100080 is a backport of this pull request to the 3.10 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Dec 7, 2022
@AlexWaygood AlexWaygood added needs backport to 3.11 only security fixes and removed needs backport to 3.11 only security fixes labels Dec 7, 2022
@miss-islington
Copy link
Contributor

Thanks @carljm for the PR, and @JelleZijlstra for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

@JelleZijlstra JelleZijlstra removed the needs backport to 3.11 only security fixes label Dec 7, 2022
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 7, 2022
…rce (pythonGH-99654) (cherry picked from commit 68e4129) Co-authored-by: Carl Meyer <carl@oddbird.net>
@bedevere-bot
Copy link

GH-100081 is a backport of this pull request to the 3.11 branch.

@JelleZijlstra JelleZijlstra added the needs backport to 3.11 only security fixes label Dec 7, 2022
@miss-islington
Copy link
Contributor

Thanks @carljm for the PR, and @JelleZijlstra for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 7, 2022
…rce (pythonGH-99654) (cherry picked from commit 68e4129) Co-authored-by: Carl Meyer <carl@oddbird.net>
@JelleZijlstra
Copy link
Member

oops now we'll have two backports

@carljm carljm deleted the fixinspect branch December 7, 2022 16:57
@AlexWaygood
Copy link
Member

AlexWaygood commented Dec 7, 2022

oops now we'll have two backports

No, she force-pushes to the existing backport branch if this happens (idk if it's an undocumented feature or a bug, it's a bit strange)

@AlexWaygood AlexWaygood removed the needs backport to 3.11 only security fixes label Dec 7, 2022
miss-islington added a commit that referenced this pull request Dec 7, 2022
…H-99654) (cherry picked from commit 68e4129) Co-authored-by: Carl Meyer <carl@oddbird.net>
miss-islington added a commit that referenced this pull request Dec 7, 2022
…H-99654) (cherry picked from commit 68e4129) Co-authored-by: Carl Meyer <carl@oddbird.net>
facebook-github-bot pushed a commit to facebookincubator/cinder that referenced this pull request Dec 8, 2022
…source (#99654) Summary: Backport the upstream fix from python/cpython#99654 Reviewed By: carljm Differential Revision: D41824891 fbshipit-source-id: 903f676
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

5 participants