Windows build - fixing compile warnings before VS2010
While some effort has gone on to get the 32-bit build to compile without warnings (thanks for that!), 64-bit still has numerous warnings. Before I push forward on more of the VS2010 port, I'd like to have a clean 2008 build all around so we can more easily track what may have changed. In completing that effort, I'm using a guideline Martin set out in #9566 [0], and please let me know if there are any others to follow. I kind of doubt anyone is against this, but if you are, please speak up before I start pushing changes. While I have your attention, I'd like to throw two other things out there to follow up the above effort: 1. Is anyone opposed to moving up to Level 4 warnings? ...take a deep breath... 2. Is anyone opposed to enabling warnings as errors? [0] http://bugs.python.org/issue9566#msg113574
Zitat von Brian Curtin <brian@python.org>:
While some effort has gone on to get the 32-bit build to compile without warnings (thanks for that!), 64-bit still has numerous warnings. Before I push forward on more of the VS2010 port, I'd like to have a clean 2008 build all around so we can more easily track what may have changed.
Does that *really* have to be a prerequisite for porting to VS 2010? If yes, then my hopes that we can move to VS 2010 before 3.3 are falling...
While I have your attention, I'd like to throw two other things out there to follow up the above effort: 1. Is anyone opposed to moving up to Level 4 warnings?
Not sure what this means. What kind of warnings would this get us? MS says "This option should be used only to provide "lint" level warnings and is not recommended as your usual warning level setting." Usually, following MS recommendations is a good thing to do on Windows. But then, the documentation goes on saying "For a new project, it may be best to use /W4 in all compilations. This will ensure the fewest possible hard-to-find code defects."
...take a deep breath... 2. Is anyone opposed to enabling warnings as errors?
The immediate consequence would be that the Windows buildbots break when somebody makes a checkin on Unix, and they cannot easily figure out how to rewrite the code to make the compiler happy. So I guess I'm -1. Regards, Martin
On Tue, Feb 21, 2012 at 23:45, <martin@v.loewis.de> wrote:
Zitat von Brian Curtin <brian@python.org>:
While some effort has gone on to get the 32-bit build to compile without warnings (thanks for that!), 64-bit still has numerous warnings. Before I push forward on more of the VS2010 port, I'd like to have a clean 2008 build all around so we can more easily track what may have changed.
Does that *really* have to be a prerequisite for porting to VS 2010? If yes, then my hopes that we can move to VS 2010 before 3.3 are falling...
Is it a prerequisite? No. I guess with this question all I'm asking is "Can I fix a lot of these warnings without someone wanting to undo them for the sake of cleaner merges or neat hg history?" I'd prefer not to take 315 warnings into a compiler change, come out with 550, and not know what potentially went wrong. In a previous company, we changed from 2008 to 2010 by upping the warning level, fixing all warnings, then enabling warnings-as-errors (I'll address this later) - the port to 2010 went nicely and we experienced a very smooth transition. Much more smoothly than 2005 to 2008. I just cut out around 100 warnings last night in 45 minutes, so I don't plan on having this take several months or anything. If I get stuck, I'll just give it up.
While I have your attention, I'd like to throw two other things out there to follow up the above effort: 1. Is anyone opposed to moving up to Level 4 warnings?
Not sure what this means. What kind of warnings would this get us?
MS says "This option should be used only to provide "lint" level warnings and is not recommended as your usual warning level setting."
Usually, following MS recommendations is a good thing to do on Windows. But then, the documentation goes on saying
"For a new project, it may be best to use /W4 in all compilations. This will ensure the fewest possible hard-to-find code defects."
The last sentence (but applied to old projects) says it all. Like I mentioned above, my last company jacked everything up to the highest levels and stuck with it, and I think we wrote nicer code. That's really all I can say. No metrics, no strong support, no debate. You could just say "no" and I'll probably accept it.
...take a deep breath... 2. Is anyone opposed to enabling warnings as errors?
The immediate consequence would be that the Windows buildbots break when somebody makes a checkin on Unix, and they cannot easily figure out how to rewrite the code to make the compiler happy. So I guess I'm -1.
I didn't think about that, so yeah, I'm probably -1 here as well.
I just cut out around 100 warnings last night in 45 minutes, so I don't plan on having this take several months or anything. If I get stuck, I'll just give it up.
Would you mind posting a batch of these to the tracker? I'd like to review them, just to be sure we have the same understanding. Regards, Martin
The code below causes different behaviour for LOAD_GLOBAL and LOAD_NAME. Which is correct? Should exceptions raised in the equality test be converted to a NameError or just propogated? Cheers, Mark. ------------------------------------- import sys class S(str): pass def eq_except(self, other): if isinstance(other, str): raise TypeError("Cannot compare S and str") globals()[S("a")] = 0 S.__eq__ = eq_except def f(): print(a) try: f() except: print(sys.exc_info()[1]) try: print(a) except: print(sys.exc_info()[1]) ---------------------------------- Output: TypeError('Cannot compare S and str',) NameError("name 'a' is not defined",)
On Thu, Feb 23, 2012 at 8:12 PM, Mark Shannon <mark@hotpy.org> wrote:
Should exceptions raised in the equality test be converted to a NameError or just propogated?
Our general trend has been towards letting such exceptions escape the operation that caused them rather than implicitly suppressing them. In this case, the NameError message that results is also misleading (since "print(globals().keys())" will definitely show an 'a' entry). Given the effort you have to go to to trigger it, I'd consider fixing this low priority, but I agree that the conversion of the TypeError to NameError is a bug (likely resolved by adding a KeyError exception type check in the appropriate location). Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
Nick Coghlan wrote:
On Thu, Feb 23, 2012 at 8:12 PM, Mark Shannon <mark@hotpy.org> wrote:
Should exceptions raised in the equality test be converted to a NameError or just propogated?
Our general trend has been towards letting such exceptions escape the operation that caused them rather than implicitly suppressing them. In this case, the NameError message that results is also misleading (since "print(globals().keys())" will definitely show an 'a' entry).
Given the effort you have to go to to trigger it, I'd consider fixing this low priority, but I agree that the conversion of the TypeError to NameError is a bug (likely resolved by adding a KeyError exception type check in the appropriate location).
It is not a difficult fix. Just replacing calls to PyDict_GetItem with PyDict_GetItemWithError and raising NameError only if no Exception has occurred. Cheers, Mark
On Tue, 21 Feb 2012 21:32:23 -0600 Brian Curtin <brian@python.org> wrote:
While some effort has gone on to get the 32-bit build to compile without warnings (thanks for that!), 64-bit still has numerous warnings. Before I push forward on more of the VS2010 port, I'd like to have a clean 2008 build all around so we can more easily track what may have changed.
+1. Of course, it doesn't help that Microsoft implements POSIX APIs incorrectly (for example, Microsoft's read() take the length as an int, not a size_t). Regards Antoine.
On 22/02/2012 3:32am, Brian Curtin wrote:
1. Is anyone opposed to moving up to Level 4 warnings?
At that level I think it complains about common things like the "do {...} while (0)" idiom, and the unreferenced self parameter of builtin functions. Presumably you would have to disable those specific warnings and any other overly annoying ones? sbt
On Wed, Feb 22, 2012 at 10:04, shibturn <shibturn@gmail.com> wrote:
On 22/02/2012 3:32am, Brian Curtin wrote:
1. Is anyone opposed to moving up to Level 4 warnings?
At that level I think it complains about common things like the "do {...} while (0)" idiom, and the unreferenced self parameter of builtin functions.
Presumably you would have to disable those specific warnings and any other overly annoying ones?
What we did was fix what was reasonable, then disable warnings which were unreasonable. If that's reasonable, that's how I would do it. (just to say it one more time: reasonable)
participants (7)
-
"Martin v. Löwis" -
Antoine Pitrou -
Brian Curtin -
Mark Shannon -
martin＠v.loewis.de
-
Nick Coghlan -
shibturn