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.

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

PEP 263 Implementation #36217

Closed
loewis mannequin opened this issue Mar 7, 2002 · 9 comments
Closed

PEP 263 Implementation #36217

loewis mannequin opened this issue Mar 7, 2002 · 9 comments
Assignees
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs)

Comments

@loewis
Copy link
Mannequin

loewis mannequin commented Mar 7, 2002

BPO 526840
Nosy @malemburg, @gvanrossum, @loewis
Files
  • codings.diff: Version 2
  • 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/malemburg'
    closed_at = <Date 2002-08-04.16:59:01.000>
    created_at = <Date 2002-03-07.08:55:40.000>
    labels = ['interpreter-core']
    title = 'PEP 263 Implementation'
    updated_at = <Date 2002-08-04.16:59:01.000>
    user = 'https://github.com/loewis'

    bugs.python.org fields:

    activity = <Date 2002-08-04.16:59:01.000>
    actor = 'loewis'
    assignee = 'lemburg'
    closed = True
    closed_date = None
    closer = None
    components = ['Interpreter Core']
    creation = <Date 2002-03-07.08:55:40.000>
    creator = 'loewis'
    dependencies = []
    files = ['4034']
    hgrepos = []
    issue_num = 526840
    keywords = ['patch']
    message_count = 9.0
    messages = ['39158', '39159', '39160', '39161', '39162', '39163', '39164', '39165', '39166']
    nosy_count = 3.0
    nosy_names = ['lemburg', 'gvanrossum', 'loewis']
    pr_nums = []
    priority = 'high'
    resolution = 'out of date'
    stage = None
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue526840'
    versions = ['Python 2.3']

    @loewis
    Copy link
    Mannequin Author

    loewis mannequin commented Mar 7, 2002

    The attached patch implements PEP-263. The following
    differences to the PEP (rev. 1.8) are known:

    • The implementation interprets "ASCII compatible" as
      meaning "bytes below 128 always denote ASCII
      characters", although this property is only used for
      ",', and \. There have been other readings of "ASCII
      compatible", so this should probably be elaborated in
      the PEP.

    • The check whether all bytes follow the declared or
      system encoding (including comments and string
      literals) is only performed if the encoding is "ascii".

    @loewis loewis mannequin closed this as completed Mar 7, 2002
    @loewis loewis mannequin assigned malemburg Mar 7, 2002
    @loewis loewis mannequin added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Mar 7, 2002
    @loewis loewis mannequin closed this as completed Mar 7, 2002
    @loewis loewis mannequin assigned malemburg Mar 7, 2002
    @loewis loewis mannequin added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Mar 7, 2002
    @loewis
    Copy link
    Mannequin Author

    loewis mannequin commented Mar 7, 2002

    Logged In: YES
    user_id=21627

    A note on the implementation strategy: it turned out that
    communicating the encoding into the abstract syntax was the
    biggest challenge.

    To solve this, I introduced encoding_decl pseudo node: it is
    an unused non-terminal whose STR() is the encoding, and
    whose only child is the true root of the syntax tree. As
    such, it is the only non-terminal which has a STR value.

    @malemburg
    Copy link
    Member

    Logged In: YES
    user_id=38388

    Thank you !

    I'll add a note to the PEP about the way the first two lines
    are processed (removing the ASCII mention...).

    @gvanrossum
    Copy link
    Member

    Logged In: YES
    user_id=6380

    I've set the group to Python 2.3 so the priority has some
    context (I'd rather you move the priority down to 5 but I
    understand this is your personal priority).

    I haven't accepted the PEP yet (although I expect I will),
    so please don't check this in yet (if you feel it needs to
    be saved in CVS, use a branch).

    @malemburg
    Copy link
    Member

    Logged In: YES
    user_id=38388

    Ok, I've had a look at the patch.

    It looks good except for the overly
    complicated implementation of the
    unicode-escape codec.

    Even though there's a bit of code duplication,
    I'd prefer to have two separate functions here:
    one for the standard char* pointer type and
    another one for Py_UNICODE*, ie.

    PyUnicode_DecodeUnicodeEscape(char*...)
    and
    PyUnicode_DecodeUnicodeEscapeFromUnicode(Py_UNICODE*...)

    This is easier to support and gives better
    performance since the compiler can optimize
    the two functions making different
    assumptions.

    You'll also need to include a name mangling
    at the top of the header for the new API.

    @loewis
    Copy link
    Mannequin Author

    loewis mannequin commented Mar 7, 2002

    Logged In: YES
    user_id=21627

    Changing the decoding functions will not result in one
    additional function, but in two of them: you'll also get
    PyUnicode_DecodeRawUnicodeEscapeFromUnicode.

    That seems quite unmaintainable to me: any change now needs
    to propagate into four functions. OTOH, I don't think that
    the code that allows parsing a variable-sized strings is
    overly complicated.

    @loewis
    Copy link
    Mannequin Author

    loewis mannequin commented Mar 21, 2002

    Logged In: YES
    user_id=21627

    Version 2 of this patch implements revision 1.11 of the PEP
    (phase 1). The check of the complete source file for
    compliance with the declared encoding is implemented by
    decoding the input line-by-line; I believe that for all
    supported encodings, this is not different compared to
    decoding the entire source file at once.

    @malemburg
    Copy link
    Member

    Logged In: YES
    user_id=38388

    Apart from the codec changes, the patch looks ok.

    I would still like two APIs for the two different codec tasks,
    though. I don't expect anything much to change in the
    codecs, so maintenance is not an issue.

    @loewis
    Copy link
    Mannequin Author

    loewis mannequin commented Aug 4, 2002

    Logged In: YES
    user_id=21627

    This patch has been superceded by 534304.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 9, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    interpreter-core (Objects, Python, Grammar, and Parser dirs)
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants