Author nnorwitz
Recipients
Date 2006-08-31.05:42:57
SpamBayes Score
Marked as misclassified
Message-id
In-reply-to
Content
Logged In: YES 
user_id=33168

import.c: comment has double word 'added'
ast.c: 
 * why is static commented out for ast_for_arguments?
 * in handle_keywordonly_args, doesn't the default case need
to return -1 after setting the error?
 * need to change the // comments to /* */
 * it looks like a bunch of lines had whitespace, but no
other changes.  this makes it hard to see the real changes.

compile.c:
 * i think there is a bug in the arglength calc if there are
more than 255 positional args and kw_default_count since
there is |= kw_default_count << 8 (2 places)
 * return should go on its own line like the rest of the
surrounding code.
 * why kwonlyargs and kw_args?  the underscore seems
inconsistent

in the compiler package, I didn't see a change to
MAKE_FUNCTION similar to the one in compile.c for the opcode
stack effect.

the change to regrtest.py doesn't seem necessary (just an
added blank line)

there should be a lot more tests added for both positive and
negative conditions (syntax errors).  Include variants with
func(**kwargs) func(*args), etc in calls.

it's not good to comment out the tests in test_frozen. 
otherwise, we will probably forget about the problems.

when you say it passes all tests, did you run with -u all?
the compiler doesn't do all the tests without it (and one or
two other tests too).

in the new test, i think you need to pass a name for the
filename param to compile.  I think there was some platform
(windows?) that crapped out with an empty filename.  (if you
copied from an existing test, then i must be remembering
something different.)

In testRaiseErrorWhenFuncall(), you can use
self.assertRaises, at least for most of the cases.

you need a test_main for this test to be run from regrtest.

History
Date User Action Args
2007-08-23 15:54:26adminlinkissue1549670 messages
2007-08-23 15:54:26admincreate