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 — gh-98831: Modernize FORMAT_VALUE by gvanrossum · Pull Request #101628 · python/cpython

Conversation

@gvanrossum

@gvanrossum gvanrossum commented Feb 7, 2023 •
edited by bedevere-bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

This was the simplest one I've done in a while, except that I had to fix the DSL parser to support balanced parentheses and brackets in conditions (and array sizes, as a side benefit).

  • Issue: Generate the interpreter #98831

Sorry, something went wrong.

Uh oh!

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

All reactions
@gvanrossum gvanrossum added the skip news label Feb 7, 2023
@gvanrossum
gvanrossum requested a review from iritkatriel February 7, 2023 01:48
@bedevere-bot bedevere-bot added the awaiting core review label Feb 7, 2023
@bedevere-bot bedevere-bot mentioned this pull request Feb 7, 2023
Generate the interpreter #98831
Closed
@gvanrossum
gvanrossum marked this pull request as draft February 7, 2023 05:21
gvanrossum added 4 commits February 7, 2023 08:30
@gvanrossum
Skip balanced ()/[] blocks in expressions
ff87ca4
@gvanrossum
Update test
43beb67
@gvanrossum
Modernize FORMAT_VALUE
575b810
@gvanrossum
Fix test crash
1e48f9d
@gvanrossum
gvanrossum force-pushed the format-value branch from d4ba909 to 1e48f9d Compare February 7, 2023 18:23
@gvanrossum

gvanrossum commented Feb 7, 2023

Copy link
Copy Markdown
Member Author

This wasn't simple after all. A test crashed because there was an error path with a manual goto error when the inputs had already been decref'ed. This caused a reference deficiency for the inputs on the stack leading to a crash that had me stumped until I picked up paper and pencil and reasoned through the generated code.

While I think I've fixed this now, this points out a general weakness in our bytecode definitions. Sometimes we feel forced to leave a manual goto error in the code (though note that in this case it was an oversight, for which I take full responsibility), but maintaining reference count correctness in all cases is hard!

All reactions

Sorry, something went wrong.

Uh oh!

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

@gvanrossum
gvanrossum marked this pull request as ready for review February 7, 2023 18:37
@gvanrossum gvanrossum added the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Feb 7, 2023
@bedevere-bot

bedevere-bot commented Feb 7, 2023

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @gvanrossum for commit 1e48f9d 🤖

If you want to schedule another build, you need to add the :hammer: test-with-refleak-buildbots label again.

All reactions

Sorry, something went wrong.

Uh oh!

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

@bedevere-bot bedevere-bot removed the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Feb 7, 2023
@gvanrossum

gvanrossum commented Feb 7, 2023

Copy link
Copy Markdown
Member Author

Ready for review.

All reactions

Sorry, something went wrong.

Uh oh!

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

iritkatriel
iritkatriel approved these changes Feb 7, 2023
View reviewed changes
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels Feb 7, 2023
@gvanrossum

gvanrossum commented Feb 7, 2023 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member Author

This is blocked until I can figure out the refleak failures. (EDIT: But first I'm going to re-run that particular build, in case it's a flake.)

All reactions

Sorry, something went wrong.

Uh oh!

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

@gvanrossum

gvanrossum commented Feb 7, 2023

Copy link
Copy Markdown
Member Author

Hm, aarch64 RHEL8 Refleaks PR seems to have been failing a lot, historically. Ignoring.

All reactions

Sorry, something went wrong.

Uh oh!

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

@gvanrossum
gvanrossum merged commit b2b85b5 into python:main Feb 8, 2023
@bedevere-bot bedevere-bot removed the awaiting merge label Feb 8, 2023
@gvanrossum
gvanrossum deleted the format-value branch February 8, 2023 01:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iritkatriel iritkatriel iritkatriel approved these changes

Uh oh!

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

Assignees

No one assigned

Labels

skip news

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.

3 participants

@gvanrossum @bedevere-bot @iritkatriel
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.