Skip to content
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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Pre-feed the parser with the f-string expression location #85248

Closed
lysnikolaou opened this issue Jun 22, 2020 · 3 comments
Closed

Pre-feed the parser with the f-string expression location #85248

lysnikolaou opened this issue Jun 22, 2020 · 3 comments
Assignees
@lysnikolaou
Labels
3.9 only security fixes 3.10 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@lysnikolaou
Copy link
Contributor

BPO 41076
Nosy @gvanrossum, @ericvsmith, @lysnikolaou, @pablogsal
PRs
  • bpo-41076: Pre-feed the parser with the f-string expression location #21054
  • [3.9] bpo-41076: Pre-feed the parser with the f-string expression location (GH-21054) #21190
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/lysnikolaou'
closed_at = <Date 2020-06-28.00:17:18.995>
created_at = <Date 2020-06-22.12:54:04.263>
labels = ['interpreter-core', 'type-bug', '3.9', '3.10']
title = 'Pre-feed the parser with the f-string expression location'
updated_at = <Date 2020-06-28.00:17:18.994>
user = 'https://github.com/lysnikolaou'

    bugs.python.org fields:

    activity = <Date 2020-06-28.00:17:18.994>
actor = 'lys.nikolaou'
assignee = 'lys.nikolaou'
closed = True
closed_date = <Date 2020-06-28.00:17:18.995>
closer = 'lys.nikolaou'
components = ['Interpreter Core']
creation = <Date 2020-06-22.12:54:04.263>
creator = 'lys.nikolaou'
dependencies = []
files = []
hgrepos = []
issue_num = 41076
keywords = ['patch']
message_count = 3.0
messages = ['372086', '372481', '372482']
nosy_count = 4.0
nosy_names = ['gvanrossum', 'eric.smith', 'lys.nikolaou', 'pablogsal']
pr_nums = ['21054', '21190']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue41076'
versions = ['Python 3.9', 'Python 3.10']
    @lysnikolaou
    Copy link
    Contributor Author

    Inspired by bpo-41064, I sat down to try and find problems with f-string locations in the new parser. I was able to come up with a way to compute the locations of the f-string expressions that I think is more consistent and allows us to delete all the code that was fixing the expression locations after the actual parsing, which accounted for about 1/6 of string_parser.c.

    A high-level explanation of the change:

    Before this change we were pre-feeding the parser with the location of the f-string itself. The parser was then parsing the expression and was computing the locations of all the nodes based on the offset of the f-string. After the parsing was done, we were identifying the offset and the lineno of the expression within the fstring and were fixing the node locations accordingly. For example, for an f-string like a = 0; f'irrelevant {a}' we were doing the following:

    • Pre-feed the parser with lineno=0 and col_offset=7 (the offset of the f-string itself in the current line).
    • Parse the expression (adding 7 to the col_offset of each parsed node, lineno remains the same since it's 0).
    • Fix the node locations by shifting the Name node by 14, which is the number of characters in the f-string (counting the f and the opening quote) before the start of the expression.

    With this change we now pre-feed the parser with the exact lineno and offset of the expression itself, not the f-string. This allows us to completely skip the third step of shifting the node locations.

    @lysnikolaou lysnikolaou added 3.9 only security fixes 3.10 only security fixes labels Jun 22, 2020
    @lysnikolaou lysnikolaou self-assigned this Jun 22, 2020
    @lysnikolaou lysnikolaou added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error labels Jun 22, 2020
    @lysnikolaou lysnikolaou changed the title Pre-feed the parser with the f-string location Pre-feed the parser with the f-string expression location Jun 22, 2020
    @pablogsal
    Copy link
    Member

    New changeset 1f0f4ab by Lysandros Nikolaou in branch 'master':
    bpo-41076: Pre-feed the parser with the f-string expression location (GH-21054)
    1f0f4ab

    @pablogsal
    Copy link
    Member

    New changeset dab533d by Pablo Galindo in branch '3.9':
    [3.9] bpo-41076: Pre-feed the parser with the f-string expression location (GH-21054) (GH-21190)
    dab533d

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees

    @lysnikolaou lysnikolaou

    Labels
    3.9 only security fixes 3.10 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    2 participants
    @pablogsal @lysnikolaou