GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-43950: Implement fine grained error locations for interactive mode by thatbirdguythatuknownot · Pull Request #92827 · python/cpython

Conversation

@thatbirdguythatuknownot

@thatbirdguythatuknownot thatbirdguythatuknownot commented May 15, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Original issue number: #88116
Most of the implementation comes from #27117. Changes include new fields in the compiler struct (.c_source) and the PyCodeObject object (.co_source) to overcome the problem in the old PR by storing the source locally instead of globally. A new source argument was also added to _PyAST_Compile() to pass the source from the parser to the compiler.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
thatbirdguythatuknownot added 12 commits May 16, 2022 07:13
@thatbirdguythatuknownot
bpo-43950: Implement fine grained error locations for interactive mode
dedba1f
@thatbirdguythatuknownot
bpo-43950: Implement fine grained error locations for interactive mode
494a3a5
@thatbirdguythatuknownot
bpo-43950: Implement fine grained error locations for interactive mode
1865ac8
@thatbirdguythatuknownot
bpo-43950: Implement fine grained error locations for interactive mode
affb018
@thatbirdguythatuknownot
bpo-43950: Implement fine grained error locations for interactive mode
f15aa44
@thatbirdguythatuknownot
bpo-43950: Implement fine grained error locations for interactive mode
18f79a7
@thatbirdguythatuknownot
bpo-43950: Implement fine grained error locations for interactive mode
ed98ae3
@thatbirdguythatuknownot
bpo-43950: Implement fine grained error locations for interactive mode
c99b286
@thatbirdguythatuknownot
bpo-43950: Implement fine grained error locations for interactive mode
551256b
@thatbirdguythatuknownot
bpo-43950: Implement fine grained error locations for interactive mode
165d763
@thatbirdguythatuknownot
bpo-43950: Implement fine grained error locations for interactive mode
3e79c5c
@thatbirdguythatuknownot
bpo-43950: Implement fine grained error locations for interactive mode
dd4f5bb
@thatbirdguythatuknownot
thatbirdguythatuknownot requested review from iritkatriel, lysnikolaou, markshannon and pablogsal as code owners May 15, 2022 23:59
@bedevere-bot bedevere-bot added the awaiting review label May 15, 2022
blurb-it Bot and others added 3 commits May 16, 2022 00:00
@blurb-it
📜🤖 Added by blurb_it.
0ce3fef
@thatbirdguythatuknownot
bpo-43950: Implement fine grained error locations for interactive mode
00d8452
@thatbirdguythatuknownot
bpo-43950: Implement fine grained error locations for interactive mode
c6c5d9d
@thatbirdguythatuknownot thatbirdguythatuknownot closed this May 16, 2022
thatbirdguythatuknownot added 3 commits May 16, 2022 08:38
@thatbirdguythatuknownot
bpo-43950: Implement fine grained error locations for interactive mode
801b237
@thatbirdguythatuknownot
bpo-43950: Implement fine grained error locations for interactive mode
867b182
@thatbirdguythatuknownot
bpo-43950: Implement fine grained error locations for interactive mode
2f04d08
@thatbirdguythatuknownot thatbirdguythatuknownot reopened this May 16, 2022
@thatbirdguythatuknownot thatbirdguythatuknownot closed this May 16, 2022
thatbirdguythatuknownot added 2 commits May 16, 2022 10:19
@thatbirdguythatuknownot
bpo-43950: Implement fine grained error locations for interactive mode
47b8756
@thatbirdguythatuknownot
bpo-43950: Implement fine grained error locations for interactive mode
13b9a52
@thatbirdguythatuknownot thatbirdguythatuknownot reopened this May 16, 2022
@thatbirdguythatuknownot
bpo-43950: Implement fine grained error locations for interactive mode
0403cbb
@thatbirdguythatuknownot
bpo-43950: Implement fine grained error locations for interactive mode
2f3d44d
@pablogsal

pablogsal commented May 16, 2022

Copy link
Copy Markdown
Member

I'm confused. The original PR #27117 is still open, even if it is a draft and the original design questions already remain. Why have you opened another PR? Have you contacted @isidentical first to take his code/approach?

We still need to decide if this is a good idea because this has drawbacks, which is one of the reasons we didn't pursue this originally.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@thatbirdguythatuknownot
Update peg_extension.c
72f9079
@thatbirdguythatuknownot

thatbirdguythatuknownot commented May 16, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

I'm confused. The original PR #27117 is still open, even if it is a draft and the original design questions already remain. Why have you opened another PR? Have you contacted @isidentical first to take his code/approach?

We still need to decide if this is a good idea because this has drawbacks, which is one of the reasons we didn't pursue this originally.

Alright, so I have to close this one?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@pablogsal

pablogsal commented May 16, 2022

Copy link
Copy Markdown
Member

Alright, so I have to close this one?

Well, you don't have to but the aspects and questions I have mentioned still stand. Particularly and most importantly, if you have taken code directly from #27117 then you need first to have the author's permission, otherwise, we won't be able to merge this even if we wanted.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@isidentical

isidentical commented May 16, 2022

Copy link
Copy Markdown
Member

My PR can be used as a baseline, no worries on that end (though I have some reservations about the implementation, especially the new code field).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@pablogsal

pablogsal commented May 16, 2022

Copy link
Copy Markdown
Member

I have some reservations about the implementation, especially the new code field).

Yeah, that's what I dislike the most and I think this may make it a no-go. At the end of the day the reason we left it there was because the extra overhead and complexity was not worth the advantages as other reprls like Ipython will not even benefit from it.

isidentical reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

thatbirdguythatuknownot added 10 commits May 17, 2022 07:41
@thatbirdguythatuknownot
Don't try to get the source of the file
847e77f
@thatbirdguythatuknownot
Don't get the source of the file
f12ac1b
@thatbirdguythatuknownot
Handle the case of <string>
b1fb696
@thatbirdguythatuknownot
Use Py_XINCREF instead of Py_INCREF
bc5b734
@thatbirdguythatuknownot
Skip file-related operations when file is either <stdin> or <string>
d401e86
@thatbirdguythatuknownot
Py_INCREF the source line to prevent negative reference count
a910c19
@thatbirdguythatuknownot
Get proper line from possible multilined statement
5dab69e
@thatbirdguythatuknownot
Update traceback.c
10a0796
@thatbirdguythatuknownot
Get proper length of the unicode string
29141b8
@thatbirdguythatuknownot
Merge branch 'main' into patch-22
5345744
@pablogsal

pablogsal commented Jun 4, 2022

Copy link
Copy Markdown
Member

After thinking about this I am -1 as the complexity introduced by this is quite high to just cover interactive mode. Unless @isidentical think otherwise, we should close this PR.

Thanks @thatbirdguythatuknownot for the prototype and for persuing the idea. Unfortunately this is one of the cases where the complexity introduced unfortunately doesn't balance well with the feature.

I think we could probably do it better in the future implementing interactive mode over temporary files

isidentical reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@thatbirdguythatuknownot

thatbirdguythatuknownot commented Jun 4, 2022

Copy link
Copy Markdown
Contributor Author

Alright, I'll close this PR.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@thatbirdguythatuknownot thatbirdguythatuknownot closed this Jun 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pablogsal pablogsal Awaiting requested review from pablogsal pablogsal is a code owner
@lysnikolaou lysnikolaou Awaiting requested review from lysnikolaou lysnikolaou is a code owner
@iritkatriel iritkatriel Awaiting requested review from iritkatriel iritkatriel is a code owner
@markshannon markshannon Awaiting requested review from markshannon markshannon is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

awaiting review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@thatbirdguythatuknownot @pablogsal @isidentical @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.