Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

gh-119786: improve internal docs onco_linetable#123198

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

@picnixz
Copy link
Member

@picnixzpicnixz commentedAug 21, 2024
edited by bedevere-appbot
Loading

In#123168, I needed to play with artificial instructions wheresome positions are not available. It's trivial to have full positions information or no positions information, but it's hard to createco_linetable values where only some instructions have positions.

This PR aims to improve the internal docs so that we can easily remember what to do (and how to do it with an example).

@picnixzpicnixz added docsDocumentation in the Doc dir skip issue skip news interpreter-core(Objects, Python, Grammar, and Parser dirs) labelsAug 21, 2024
Copy link
Member

@markshannonmarkshannon left a comment
edited
Loading

Choose a reason for hiding this comment

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

Up until the section "Artificial constructions" this seems fine.

I don't think we want to be encouraging anyone to create position tables.

The reason for explaining the layout in detail is for anyone maintaining the code.
Even for out of process tools, like pyspy, we provide C functions for them to use, so they don't need to parse the tables.

To create tables for testingdis and such, I'd recommend parsing Python code, modifying the locations on the AST, then generating the code object withcompile.
Like this:

importastimportdiscode="""def foo():    a = 1    b = 2    c = 3"""tree=ast.parse(code)func=tree.body[0]b=func.body[1].targets[0]b.lineno=5b.col_offset=17b.end_lineno=6b.end_col_offset=18foo=compile(tree,"test","exec")dis.dis(foo.co_consts[0],show_positions=True)
2:0-2:0              RESUME                   03:8-3:9              LOAD_CONST               1 (1)3:4-3:5              STORE_FAST               0 (a)4:8-4:9              LOAD_CONST               2 (2)5:17-6:18            STORE_FAST               1 (b)5:8-5:9              LOAD_CONST               3 (3)5:4-5:5              STORE_FAST               2 (c)5:4-5:5              RETURN_CONST             0 (None)

@picnixz
Copy link
MemberAuthor

To create tables for testing dis and such, I'd recommend parsing Python code, modifying the locations on the AST, then generating the code object with compile.

Oh, that's what I was searching for in#123168. Should I amend this PR and use your approach instead?

@iritkatriel
Copy link
Member

Yes, make a new PR though as that one was merged.

picnixz reacted with thumbs up emoji

@picnixz
Copy link
MemberAuthor

Actually, modifying the AST does not help because I can't remove some attributes that are expected to exist on the nodes. In particular, I cannot cancel positions (but I can easily change the values of the positions). So I think we need to stick with theco_linetable approach (unless there is something I'm missing?)

@markshannon
Copy link
Member

I see. You can't set an expression to have no location, because the compiler will just give it one.
FTR, you could test the no columns case by settingnode.end_lineno = node.end_col_offset = -1, and leaving the line number.

I think the test you wrote is fine. There is need to rewrite it.

@picnixz
Copy link
MemberAuthor

no columns case by setting node.end_lineno = node.end_col_offset = -1,

Yes, that's what I saw. But I'm not sure whether this is actually a bug or a feature of the compiler (namely, whether the -1 is really meant to indicate "no location").

@picnixz
Copy link
MemberAuthor

So I managed to use AST tricks only though I'm not really sure that setting the columns to -1 means that we are deleting the information. I'll remove the artificial constructions in these docs anyway.

@picnixzpicnixz changed the titleImprove internal docs onco_linetablegh-119786: improve internal docs onco_linetableAug 24, 2024
@picnixz
Copy link
MemberAuthor

picnixz commentedNov 27, 2024
edited
Loading

Friendly ping@iritkatriel (I'll fix the merge conflicts once I'm back, sorry missed that one)

@iritkatriel
Copy link
Member

There's a merge conflict now.

@picnixz
Copy link
MemberAuthor

Since I'm on mobile I cannot resolve it but I'll take care of it on Friday (or maybe you can if you have time). Sorry for bothering you!

iritkatriel reacted with thumbs up emoji

@picnixz
Copy link
MemberAuthor

@iritkatriel Ok, actually thelocations.md file was gone! So I fixed the conflicts. By the way, I put everything under the "Source code locations" section (semantically speaking I think the way we encode integers, the meaning of locations and the short forms are all part of this topic).

@iritkatrieliritkatriel merged commit49f15d8 intopython:mainNov 30, 2024
23 checks passed
@iritkatriel
Copy link
Member

Thank you!

picnixz reacted with heart emoji

@picnixzpicnixz deleted the improve-internal-docs-on-co-linetable branchNovember 30, 2024 00:40
picnixz added a commit to picnixz/cpython that referenced this pull requestDec 2, 2024
srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this pull requestJan 8, 2025
ebonnal pushed a commit to ebonnal/cpython that referenced this pull requestJan 12, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@markshannonmarkshannonmarkshannon left review comments

@iritkatrieliritkatrieliritkatriel approved these changes

Assignees

No one assigned

Labels

docsDocumentation in the Doc dirinterpreter-core(Objects, Python, Grammar, and Parser dirs)skip news

Projects

Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@picnixz@iritkatriel@markshannon

[8]ページ先頭

©2009-2025 Movatter.jp