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

Jump to bottom

Py3K warn against assigning to True/False #46602

Closed
brettcannon opened this issue Mar 17, 2008 · 28 comments
Closed

Py3K warn against assigning to True/False #46602

brettcannon opened this issue Mar 17, 2008 · 28 comments
Assignees
@birkenfeld
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs)

Comments

@brettcannon
Copy link
Member

BPO 2349
Nosy @brettcannon, @birkenfeld, @rhettinger, @abalkin, @benjaminp
Dependencies
  • bpo-2720: make compiling struct be passed around to all ast helpers
    		•
    Files
  • bool_assign.patch
  • bool_assign2.patch: rephrase the warnings
  • bool_assign3.patch: caught more places
  • bool_assign4.patch: added a test
  • bool_assign5.patch
  • bool_assign6.patch
  • bool_assign7.patch
    		•

    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/birkenfeld'
closed_at = <Date 2008-06-08.23:10:45.447>
created_at = <Date 2008-03-17.19:19:17.918>
labels = ['interpreter-core']
title = 'Py3K warn against assigning to True/False'
updated_at = <Date 2008-06-08.23:10:45.423>
user = 'https://github.com/brettcannon'

    bugs.python.org fields:

    activity = <Date 2008-06-08.23:10:45.423>
actor = 'benjamin.peterson'
assignee = 'georg.brandl'
closed = True
closed_date = <Date 2008-06-08.23:10:45.447>
closer = 'benjamin.peterson'
components = ['Interpreter Core']
creation = <Date 2008-03-17.19:19:17.918>
creator = 'brett.cannon'
dependencies = ['2720']
files = ['9703', '9706', '9772', '9773', '10181', '10359', '10551']
hgrepos = []
issue_num = 2349
keywords = ['patch', '26backport']
message_count = 28.0
messages = ['63717', '63765', '63768', '63777', '63779', '63782', '63784', '63796', '63797', '63799', '63802', '64015', '64075', '64081', '64083', '64086', '64089', '64318', '64981', '65953', '65985', '66157', '67014', '67819', '67824', '67840', '67846', '67849']
nosy_count = 5.0
nosy_names = ['brett.cannon', 'georg.brandl', 'rhettinger', 'belopolsky', 'benjamin.peterson']
pr_nums = []
priority = 'critical'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue2349'
versions = ['Python 2.6']
    @brettcannon
    Copy link
    Member Author

    Assigning to True of False should raise at least a Py3K warning (maybe
    something more severe?).

    @brettcannon brettcannon added release-blocker interpreter-core (Objects, Python, Grammar, and Parser dirs) and removed release-blocker labels Mar 17, 2008
    @rhettinger
    Copy link
    Contributor

    I would like to review the patch on this one.

    I think it should limit itself to the True and False in builtin. It
    would be *very* expensive to check for every assignment in every
    possible namespace.

    @brettcannon
    Copy link
    Member Author

    On Mon, Mar 17, 2008 at 3:41 PM, Raymond Hettinger
    <report@bugs.python.org> wrote:

    Raymond Hettinger <rhettinger@users.sourceforge.net> added the comment:

    I would like to review the patch on this one.

    I think it should limit itself to the True and False in builtin. It
    would be *very* expensive to check for every assignment in every
    possible namespace.

    It shouldn't be expensive when the parser does the check (just like
    with SyntaxError for None).

    @rhettinger
    Copy link
    Contributor

    The parser approach should be fine.

    @benjaminp
    Copy link
    Contributor

    I'm working on it.

    @benjaminp
    Copy link
    Contributor

    This patch alters the parser to warn for assignment to True and False.
    Enjoy!

    @rhettinger
    Copy link
    Contributor

    Looks fine. Please apply.

    @benjaminp
    Copy link
    Contributor

    Sorry, I don't permission.

    @rhettinger
    Copy link
    Contributor

    I'll apply when I get a chance.

    @rhettinger rhettinger self-assigned this Mar 17, 2008
    @abalkin
    Copy link
    Member

    abalkin commented Mar 17, 2008

    This is a minor concern, but existing -3 warnings refer to python 3.0
    and above as "3.x", not 'Py3K'. It would be nice to preserve consistency.

    @benjaminp
    Copy link
    Contributor

    "A Foolish Consistency is the Hobgoblin of Little Minds"
    This update makes the warnings say 3.x.

    @rhettinger
    Copy link
    Contributor

    Back to Brett for application.

    @rhettinger rhettinger assigned brettcannon and unassigned rhettinger Mar 18, 2008
    @brettcannon
    Copy link
    Member Author

    Actually, the patch is incomplete. You can still do assignment through
    things such as def True(): pass. If you go through ast.c and find
    the various places None is protected (search for "None" and note the
    places where an ast_error() call is made) you should put the warnings there.

    @benjaminp
    Copy link
    Contributor

    Wow! I never realized how many ways you could possibly assign to
    something. I found all the None assignments and put the True/False ones
    under it.

    @benjaminp
    Copy link
    Contributor

    I just added on a test.

    @rhettinger
    Copy link
    Contributor

    FWIW, I don't think it's important to catch every possible assignment.
    Add warnings for the common cases and declare victory.

    @benjaminp
    Copy link
    Contributor

    Do you think we should remove some?

    @rhettinger
    Copy link
    Contributor

    Keep what you've got but don't lose sleep if some offbeat case is not
    covered.

    @rhettinger rhettinger assigned brettcannon and unassigned rhettinger Mar 22, 2008
    @benjaminp
    Copy link
    Contributor

    Brett, shall I apply?

    @brettcannon
    Copy link
    Member Author

    If Raymond says it's fine, then it's also fine by me.

    @brettcannon brettcannon assigned benjaminp and unassigned brettcannon Apr 29, 2008
    @benjaminp
    Copy link
    Contributor

    As I looked over the code again, I realized that it doesn't help to just
    do a normal warning while compiling because the line number isn't
    supplied. You have to use PyWarn_Explicit for that (see the warning
    about backquotes). Since the filename (in the compiling struct) isn't
    passed around to all ast helpers, you can't warn in every function.
    Therefore, I have bpo-2720.

    @benjaminp
    Copy link
    Contributor

    Okay, now that bpo-2720 was dealt with, here's another (close to final)
    patch. I added an ast_warn help function. When -Werror is used, the
    warnings are converted to SyntaxErrors. Raymond or Brett, if you could
    take a look, that would be great. Thanks!

    @benjaminp
    Copy link
    Contributor

    I'm attaching a new patch with changes made from the Georg's comments.

    @benjaminp benjaminp assigned brettcannon and unassigned benjaminp May 18, 2008
    @benjaminp
    Copy link
    Contributor

    Georg, can I apply?

    @benjaminp benjaminp assigned birkenfeld and unassigned brettcannon Jun 8, 2008
    @birkenfeld
    Copy link
    Member

    Hmm, I'd even go a step further and factor out the whole checking for
    invalid/warnable names, like in Py3k's forbidden_name.

    Also the warning text shouldn't start with uppercase and end in a full stop.

    @benjaminp
    Copy link
    Contributor

    Here's a much better patch that delegates checking to a helper.

    @birkenfeld
    Copy link
    Member

    The macro at the top of the patch should be removed, then this can be
    checked in.

    @benjaminp
    Copy link
    Contributor

    Done in r64044.

    @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

    @birkenfeld birkenfeld

    Labels
    interpreter-core (Objects, Python, Grammar, and Parser dirs)
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    5 participants
    @brettcannon @birkenfeld @benjaminp @abalkin @rhettinger