classification
Title: Py3K warn against assigning to True/False
Type: Stage:
Components: Interpreter Core Versions: Python 2.6
process
Status: closed Resolution: fixed
Dependencies: 2720 Superseder:
Assigned To: georg.brandl Nosy List: belopolsky, benjamin.peterson, brett.cannon, georg.brandl, rhettinger
Priority: critical Keywords: 26backport, patch

Created on 2008-03-17 19:19 by brett.cannon, last changed 2008-06-08 23:10 by benjamin.peterson. This issue is now closed.

Files
File name Uploaded Description Edit
bool_assign.patch benjamin.peterson, 2008-03-17 21:23
bool_assign2.patch benjamin.peterson, 2008-03-17 22:59 rephrase the warnings
bool_assign3.patch benjamin.peterson, 2008-03-19 18:33 caught more places
bool_assign4.patch benjamin.peterson, 2008-03-19 18:50 added a test
bool_assign5.patch benjamin.peterson, 2008-05-03 16:59
bool_assign6.patch benjamin.peterson, 2008-05-17 20:03
bool_assign7.patch benjamin.peterson, 2008-06-08 17:07
Messages (28)
msg63717 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2008-03-17 19:19
Assigning to True of False should raise at least a Py3K warning (maybe
something more severe?).
msg63765 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2008-03-17 20:41
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.
msg63768 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2008-03-17 20:45
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).
msg63777 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2008-03-17 21:03
The parser approach should be fine.
msg63779 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-03-17 21:06
I'm working on it.
msg63782 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-03-17 21:23
This patch alters the parser to warn for assignment to True and False.
Enjoy!
msg63784 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2008-03-17 21:27
Looks fine.  Please apply.
msg63796 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-03-17 22:37
Sorry, I don't permission.
msg63797 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2008-03-17 22:39
I'll apply when I get a chance.
msg63799 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2008-03-17 22:53
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.
msg63802 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-03-17 22:59
"A Foolish Consistency is the Hobgoblin of Little Minds"
This update makes the warnings say 3.x.
msg64015 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2008-03-18 23:35
Back to Brett for application.
msg64075 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2008-03-19 17:34
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.
msg64081 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-03-19 18:33
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.
msg64083 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-03-19 18:50
I just added on a test.
msg64086 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2008-03-19 19:09
FWIW, I don't think it's important to catch every possible assignment. 
Add warnings for the common cases and declare victory.
msg64089 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-03-19 19:33
Do you think we should remove some?
msg64318 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2008-03-22 11:23
Keep what you've got but don't lose sleep if some offbeat case is not 
covered.
msg64981 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-04-05 15:18
Brett, shall I apply?
msg65953 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2008-04-29 02:17
If Raymond says it's fine, then it's also fine by me.
msg65985 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-04-29 21:48
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 #2720.
msg66157 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-05-03 16:59
Okay, now that #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!
msg67014 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-05-17 20:03
I'm attaching a new patch with changes made from the Georg's comments.
msg67819 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-06-08 01:43
Georg, can I apply?
msg67824 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2008-06-08 08:23
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.
msg67840 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-06-08 17:07
Here's a much better patch that delegates checking to a helper.
msg67846 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2008-06-08 18:06
The macro at the top of the patch should be removed, then this can be
checked in.
msg67849 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-06-08 23:10
Done in r64044.
History
Date User Action Args
2008-06-08 23:10:45benjamin.petersonsetstatus: open -> closed
resolution: accepted -> fixed
messages: + msg67849
2008-06-08 18:06:21georg.brandlsetresolution: accepted
messages: + msg67846
2008-06-08 17:07:08benjamin.petersonsetfiles: + bool_assign7.patch
messages: + msg67840
2008-06-08 08:23:18georg.brandlsetmessages: + msg67824
2008-06-08 01:43:58benjamin.petersonsetassignee: brett.cannon -> georg.brandl
messages: + msg67819
nosy: + georg.brandl
2008-05-18 21:09:17benjamin.petersonsetassignee: benjamin.peterson -> brett.cannon
2008-05-17 20:04:14benjamin.petersonsetfiles: + bool_assign6.patch
messages: + msg67014
2008-05-03 16:59:21benjamin.petersonsetfiles: + bool_assign5.patch
messages: + msg66157
2008-05-02 21:15:33benjamin.petersonsetdependencies: + make compiling struct be passed around to all ast helpers
2008-04-29 21:48:54benjamin.petersonsetmessages: + msg65985
2008-04-29 02:17:59brett.cannonsetassignee: brett.cannon -> benjamin.peterson
messages: + msg65953
2008-04-05 15:18:51benjamin.petersonsetmessages: + msg64981
2008-03-22 11:23:15rhettingersetassignee: rhettinger -> brett.cannon
messages: + msg64318
2008-03-19 21:06:20brett.cannonsetassignee: brett.cannon -> rhettinger
2008-03-19 19:33:21benjamin.petersonsetmessages: + msg64089
2008-03-19 19:09:50rhettingersetmessages: + msg64086
2008-03-19 18:50:17benjamin.petersonsetfiles: + bool_assign4.patch
messages: + msg64083
2008-03-19 18:33:55benjamin.petersonsetfiles: + bool_assign3.patch
messages: + msg64081
2008-03-19 17:34:01brett.cannonsetresolution: accepted -> (no value)
messages: + msg64075
2008-03-18 23:35:14rhettingersetassignee: rhettinger -> brett.cannon
messages: + msg64015
2008-03-17 22:59:07benjamin.petersonsetfiles: + bool_assign2.patch
messages: + msg63802
2008-03-17 22:53:09belopolskysetnosy: + belopolsky
messages: + msg63799
2008-03-17 22:39:41rhettingersetassignee: rhettinger
messages: + msg63797
2008-03-17 22:37:29benjamin.petersonsetmessages: + msg63796
2008-03-17 21:27:46rhettingersetresolution: accepted
messages: + msg63784
2008-03-17 21:23:47benjamin.petersonsetfiles: + bool_assign.patch
keywords: + patch
messages: + msg63782
2008-03-17 21:06:22benjamin.petersonsetnosy: + benjamin.peterson
messages: + msg63779
2008-03-17 21:03:53rhettingersetmessages: + msg63777
2008-03-17 20:45:28brett.cannonsetmessages: + msg63768
2008-03-17 20:41:01rhettingersetnosy: + rhettinger
messages: + msg63765
2008-03-17 20:14:12brett.cannonsetpriority: release blocker -> critical
2008-03-17 19:19:17brett.cannoncreate