classification
Title: Document and slightly simplify lnotab tracing
Type: Stage: resolved
Components: Interpreter Core Versions: Python 3.2, Python 2.7
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: Nosy List: alexandre.vassalotti, collinwinter, georg.brandl, jyasskin, pitrou
Priority: normal Keywords: needs review, patch

Created on 2009-05-16 20:17 by jyasskin, last changed 2009-07-30 17:59 by alexandre.vassalotti. This issue is now closed.

Files
File name Uploaded Description Edit
lnotab.patch jyasskin, 2009-05-16 20:16
lnotab.patch jyasskin, 2009-05-21 22:23 Slightly simpler version with PyCode_CheckLineNumber() changes.
Messages (8)
msg87919 - (view) Author: Jeffrey Yasskin (jyasskin) * (Python committer) Date: 2009-05-16 20:16
lnotab-based tracing is very complicated and isn't documented very well.
There were at least 3 comment blocks purporting to document co_lnotab,
and none did a very good job. This patch unifies them into
Objects/lnotab_notes.txt which tries to completely capture the current
state of affairs.  I'm posting this here so that people can check that I
got it right. Thanks!

I also discovered that we've attached 2 layers of patches to the basic
tracing scheme. The first layer avoids jumping to instructions that
don't start a line, to avoid problems in if statements and while loops.
The second layer discovered that jumps backward do need to trace at
instructions that don't start a line, so it added extra lnotab entries
for 'while' and 'for' loops, and added a special case for backward jumps
within the same line. I replaced these patches by treating forward and
backward jumps differently.

I could simplify this slightly more by changing PyCode_CheckLineNumber.
 It doesn't appear to be used outside of the core, but since it doesn't
start with an _, I want to double-check that changing its interface is ok.
msg87975 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2009-05-17 09:24
Jeffrey, while you're at lnotab stuff, could you have a look at #1689458
as well?
msg88076 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-05-19 08:14
I can't say anything about the simplification since I never use tracing,
but +1 at least on the doc cleanup.
msg88171 - (view) Author: Jeffrey Yasskin (jyasskin) * (Python committer) Date: 2009-05-21 22:23
I've fixed #1689458. Here's a new version of the patch with the
simplifying tweaks to PyCode_CheckLineNumber() so y'all can see what
that looks like.

If I don't get any substantive comments by the weekend (thanks Antoine
for the docs +1 :), I'll assume I've looked at this code longer than
anyone in the last 7 years and commit it. :)
msg88196 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-05-22 16:09
I'm not sure why you changed PyCode_CheckLineNumber to
_PyCode_CheckLineNumber. Other than that, I suppose you know what you're
doing :)
msg88199 - (view) Author: Jeffrey Yasskin (jyasskin) * (Python committer) Date: 2009-05-22 16:26
I had two reasons to change PyCode_CheckLineNumber to
_PyCode_CheckLineNumber: First, its behavior is changing without its
signature changing. Without a name change, that could break users
silently (if there are any codesearch missed). Second, I think it's an
internal function which we should feel free to change again if it makes
the implementation cleaner. In order to let people change it at will, it
should start with "_Py".
msg88255 - (view) Author: Jeffrey Yasskin (jyasskin) * (Python committer) Date: 2009-05-23 23:24
Committed to trunk in r72879. I'll wait to merge it to 3.x until 3.1 has
been released, since we're approaching the release candidate there.
msg91102 - (view) Author: Alexandre Vassalotti (alexandre.vassalotti) * (Python committer) Date: 2009-07-30 17:59
The patch was merged to 3.x in r74132.
History
Date User Action Args
2009-07-30 17:59:48alexandre.vassalottisetstatus: open -> closed

nosy: + alexandre.vassalotti
messages: + msg91102

assignee: jyasskin ->
resolution: accepted
2009-05-23 23:24:20jyasskinsetassignee: jyasskin
2009-05-23 23:24:06jyasskinsetmessages: + msg88255
stage: patch review -> resolved
2009-05-22 16:26:04jyasskinsetmessages: + msg88199
2009-05-22 16:09:28pitrousetmessages: + msg88196
2009-05-21 22:23:27jyasskinsetfiles: + lnotab.patch

messages: + msg88171
2009-05-19 08:14:37pitrousetnosy: + pitrou
messages: + msg88076
2009-05-17 09:24:17georg.brandlsetnosy: + georg.brandl
messages: + msg87975
2009-05-17 05:07:47collinwintersetnosy: + collinwinter
2009-05-16 20:17:00jyasskincreate