msg130342 - (view) |
Author: July Tikhonov (july) * |
Date: 2011-03-08 16:24 |
Normal:
>>> compile('1 = 1', '<string>', 'exec')
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "<string>", line 1
SyntaxError: can't assign to literal
SystemError is raised instead of SyntaxError:
>>> try: abcde
... except NameError:
... compile('1 = 1', '<string>', 'exec')
...
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
NameError: name 'abcde' is not defined
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "<stdin>", line 3, in <module>
SystemError: Objects/tupleobject.c:126: bad argument to internal function
Error can be discovered by calling dis.dis('1 = 1').
|
msg130343 - (view) |
Author: Daniel Urban (daniel.urban) * |
Date: 2011-03-08 17:31 |
Apparently ast_error_finish calls PyTuple_GetItem with a value that is not a tuple, but a SyntaxError instance (in Python/ast.c line 112). It seems that ast_error_finish expects that PyErr_Fetch will return the exception value as a tuple, and in some cases this seems correct (for example when not in an except clause), but not in this case. I don't know much about Python exception handling in C, but it seems to me, that it shouldn't expect always a tuple (see also http://docs.python.org/dev/py3k/c-api/exceptions.html#PyErr_NormalizeException).
|
msg130346 - (view) |
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * |
Date: 2011-03-08 18:04 |
Right. In most cases, "PyErr_SetObject(PyExc_SyntaxError, tuple);" will store the untouched tuple in tstate->curexc_value, *except* when "Implicit exception chaining" occurs, in which case the exception is normalized.
ast_error_finish() should not expect a tuple in all cases, and should probably normalize the exception as well.
|
msg130374 - (view) |
Author: Daniel Urban (daniel.urban) * |
Date: 2011-03-08 21:07 |
Here is a patch. I wasn't sure, where to put the test, so I put it in test_ast.
|
msg130378 - (view) |
Author: Benjamin Peterson (benjamin.peterson) * |
Date: 2011-03-08 22:07 |
You can put the test in test_compile.
|
msg130380 - (view) |
Author: Daniel Urban (daniel.urban) * |
Date: 2011-03-08 22:25 |
Okay, here is a new patch with the test in the correct place (I hope).
|
msg130386 - (view) |
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * |
Date: 2011-03-08 23:37 |
Why is the exception normalized at the end? I suppose it's because when value is an exception instance, it's replaced by a tuple, but the original value has to be recreated at the end. So in some cases, the SyntaxError object is created twice...
If PyErr_NormalizeException() can't be avoided, I suggest to call it at the start, just after PyErr_Fetch, and use the PySyntaxErrorObject* structure directly to get the file name and line numbers.
|
msg130431 - (view) |
Author: Daniel Urban (daniel.urban) * |
Date: 2011-03-09 07:34 |
> Why is the exception normalized at the end? I suppose it's because
> when value is an exception instance, it's replaced by a tuple, but the
> original value has to be recreated at the end. So in some cases, the
> SyntaxError object is created twice...
>
> If PyErr_NormalizeException() can't be avoided, I suggest to call it
> at the start, just after PyErr_Fetch, and use the PySyntaxErrorObject*
> structure directly to get the file name and line numbers.
Yeah, it is because ast_error_finish creates a new tuple to use as the exception value. (It creates a new (errstr, (filename, lineno, offset, loc)) tuple from the original (errstr, lineno, offset) tuple). And yes, in some cases the SyntaxError instance is created twice. I wasn't sure if it's okay to simply replace the args field of a PyBaseExceptionObject. I don't know, if PyErr_NormalizeException() can be avoided, you wrote, that it "should probably normalize the exception as well".
Would it be better, if we, when got an exception instance, create the new tuple from the info, and replace the args field of the instance with it? (But it also seems to me, that the SyntaxError objects have other fields as well, so probably we should modify them also. That's why I thought that calling PyErr_NormalizeException with the new tuple is the simplest thing to do, becuase I guess that'll take care of all fields automatically.)
|
msg130434 - (view) |
Author: July Tikhonov (july) * |
Date: 2011-03-09 09:22 |
There is an XXX just before the definition of ast_error. Wouldn't it be
useful?
The idea is to merge ast_error() and ast_error_finish().
This requires redefinition of most functions in ast.c, adding "const char
*filename" to their parameters.
|
msg130436 - (view) |
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * |
Date: 2011-03-09 09:44 |
> That's why I thought that calling PyErr_NormalizeException with the new
> tuple is the simplest thing to do, becuase I guess that'll take care of
> all fields automatically.
You could also call PyErr_NormalizeException at the beginning, and update the fields directly in the PySyntaxErrorObject structure. No need to deal with any tuple.
|
msg130453 - (view) |
Author: Daniel Urban (daniel.urban) * |
Date: 2011-03-09 12:50 |
> You could also call PyErr_NormalizeException at the beginning, and
> update the fields directly in the PySyntaxErrorObject structure. No
> need to deal with any tuple.
Sorry, but I don't really understand. If I call PyErr_NormalizeException at the beginning, the SyntaxError instance will be initialized with the wrong 3-tuple: (errstr, lineno, offset). If after that I update the msg, filename, ... fields, they will be correct, but I think the args field still will be the wrong 3-tuple. So I'll have to still create the new tuple, and replace the args field, right?
|
msg130454 - (view) |
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * |
Date: 2011-03-09 12:54 |
hmm, you are right, of course. I forgot that e.args is part of the SyntaxError members.
|
msg130515 - (view) |
Author: Daniel Urban (daniel.urban) * |
Date: 2011-03-10 17:13 |
So, I see four possible solutions:
1. If we get a tuple, create the new tuple, normalize the exception, and store it. If we get a SyntaxError instance, use its args, create the new tuple, normalize, and store. (In this case a SyntaxError instance will be created twice.)
2. If we get a tuple, create the new tuple and store it without normalization. If we get a SyntaxError instance use its args to create the new tuple and store it without normalization. (I think, that later it's still possible that a new SynaxError will be created, but we don't create it here.)
3. If we get a tuple, create the new tuple, and store it without normalization. If we get a SyntaxError, take its args, create the new tuple, and call SyntaxError.__init__ with it. I think this will set all fields properly.
4. Like 3., but if we got a tuple, store the new tuple with normalization.
My patch currently does 1.
My patch, without the PyErr_NormalizeException() call would be 2.
I think maybe 3. would be the best solution, or 4., if normalization is desired in all cases.
I can write a new patch, if the experts tell me what is the best solution from the four (or some other I didn't think of).
|
msg130517 - (view) |
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * |
Date: 2011-03-10 17:23 |
I'd choose solution 3, but instead of calling SyntaxError.__init__, call PyErr_NormalizeException().
|
msg130518 - (view) |
Author: Daniel Urban (daniel.urban) * |
Date: 2011-03-10 17:57 |
Err... sorry, I don't understand again:
If we get a tuple, create a new, store it without normalization. That's okay, I understand.
If we get a SyntaxError instance, then take its args field, create the new tuple. Then call PyErr_NormalizeException(), with:
a) the new tuple? But this will create a new SyntaxError instance...
b) the old SyntaxError instance? But this won't correct the wrong fields of the instance. (I think SyntaxError.__init__ would correct them without creating a new instance.)
c) or replace the args of the SyntaxError instance with the new tuple, then call PyErr_NormalizeException() on the instance? But I think that still won't correct the other fields of the instance...
Sorry for all these questions... I'd just like to help.
|
msg179704 - (view) |
Author: Brett Cannon (brett.cannon) * |
Date: 2013-01-11 17:06 |
This no longer seems to be a problem in Python 3.2, 3.3, or 3.4.
|
|
Date |
User |
Action |
Args |
2022-04-11 14:57:14 | admin | set | github: 55650 |
2013-01-11 17:06:19 | brett.cannon | set | status: open -> closed resolution: out of date messages:
+ msg179704
|
2011-03-10 17:57:44 | daniel.urban | set | nosy:
brett.cannon, georg.brandl, amaury.forgeotdarc, ncoghlan, benjamin.peterson, july, daniel.urban messages:
+ msg130518 |
2011-03-10 17:23:00 | amaury.forgeotdarc | set | nosy:
brett.cannon, georg.brandl, amaury.forgeotdarc, ncoghlan, benjamin.peterson, july, daniel.urban messages:
+ msg130517 |
2011-03-10 17:13:55 | daniel.urban | set | nosy:
brett.cannon, georg.brandl, amaury.forgeotdarc, ncoghlan, benjamin.peterson, july, daniel.urban messages:
+ msg130515 |
2011-03-09 12:54:55 | amaury.forgeotdarc | set | nosy:
brett.cannon, georg.brandl, amaury.forgeotdarc, ncoghlan, benjamin.peterson, july, daniel.urban messages:
+ msg130454 |
2011-03-09 12:50:05 | daniel.urban | set | nosy:
brett.cannon, georg.brandl, amaury.forgeotdarc, ncoghlan, benjamin.peterson, july, daniel.urban messages:
+ msg130453 |
2011-03-09 11:08:26 | july | set | files:
- unnamed nosy:
brett.cannon, georg.brandl, amaury.forgeotdarc, ncoghlan, benjamin.peterson, july, daniel.urban |
2011-03-09 09:44:11 | amaury.forgeotdarc | set | nosy:
brett.cannon, georg.brandl, amaury.forgeotdarc, ncoghlan, benjamin.peterson, july, daniel.urban messages:
+ msg130436 |
2011-03-09 09:22:37 | july | set | files:
+ unnamed
messages:
+ msg130434 nosy:
brett.cannon, georg.brandl, amaury.forgeotdarc, ncoghlan, benjamin.peterson, july, daniel.urban |
2011-03-09 07:34:56 | daniel.urban | set | nosy:
brett.cannon, georg.brandl, amaury.forgeotdarc, ncoghlan, benjamin.peterson, july, daniel.urban messages:
+ msg130431 |
2011-03-08 23:37:54 | amaury.forgeotdarc | set | nosy:
brett.cannon, georg.brandl, amaury.forgeotdarc, ncoghlan, benjamin.peterson, july, daniel.urban messages:
+ msg130386 |
2011-03-08 22:25:42 | daniel.urban | set | files:
+ issue11441_2.patch nosy:
brett.cannon, georg.brandl, amaury.forgeotdarc, ncoghlan, benjamin.peterson, july, daniel.urban messages:
+ msg130380
|
2011-03-08 22:07:34 | benjamin.peterson | set | nosy:
brett.cannon, georg.brandl, amaury.forgeotdarc, ncoghlan, benjamin.peterson, july, daniel.urban messages:
+ msg130378 |
2011-03-08 21:44:16 | terry.reedy | set | nosy:
+ benjamin.peterson, georg.brandl, brett.cannon, ncoghlan
|
2011-03-08 21:07:18 | daniel.urban | set | files:
+ issue11441.patch
messages:
+ msg130374 keywords:
+ patch |
2011-03-08 18:04:17 | amaury.forgeotdarc | set | nosy:
+ amaury.forgeotdarc messages:
+ msg130346
|
2011-03-08 17:31:41 | daniel.urban | set | nosy:
+ daniel.urban
messages:
+ msg130343 versions:
+ Python 3.2 |
2011-03-08 16:24:43 | july | create | |