Re: [pypy-svn] r8078 - pypy/trunk/src/pypy/interpreter
On Wed, Jan 05, 2005 at 17:35 +0100, tismer@codespeak.net wrote:
Author: tismer Date: Wed Jan 5 17:35:47 2005 New Revision: 8078
Modified: pypy/trunk/src/pypy/interpreter/pyopcode.py Log: changed LOAD_GLOBAL to support the flow space better. In case of a NameError, where the space does not define NameError, we raise a real NameError with a message.
Modified: pypy/trunk/src/pypy/interpreter/pyopcode.py ============================================================================== --- pypy/trunk/src/pypy/interpreter/pyopcode.py (original) +++ pypy/trunk/src/pypy/interpreter/pyopcode.py Wed Jan 5 17:35:47 2005 @@ -525,8 +525,13 @@ if not e.match(f.space, f.space.w_KeyError): raise message = "global name '%s' is not defined" % varname - w_exc_type = f.space.w_NameError - w_exc_value = f.space.wrap(message) + try: + w_exc_type = f.space.w_NameError + w_exc_value = f.space.wrap(message) + except AttributeError: + # object space does not support it, so crash really + raise NameError, (message + + ", but %s has no NameError!" % f.space) raise OperationError(w_exc_type, w_exc_value) f.valuestack.push(w_value)
I don't understand this change. Why is raising a NameError for not finding space.w_NameError better than just an AttributeError? This seems to me like mixing different levels (application level and interpreter level). It seems the flow object space could easily provide its own w_NameError and then you can check the usual way, catching an OperationError and then for e.match(space, space.w_NameError) without changing the interpreter. cheers (and happy new year to you, christian!), holger
Hi, On Fri, Jan 07, 2005 at 12:39:41PM +0100, holger krekel wrote:
Log: changed LOAD_GLOBAL to support the flow space better. In case of a NameError, where the space does not define NameError, we raise a real NameError with a message.
(...). It seems the flow object space could easily provide its own w_NameError and then you can check the usual way, catching an OperationError and then for e.match(space, space.w_NameError) without changing the interpreter.
I guess the goal is to have the problem crash the program with a meaningful error message. If the flow objspace provided a w_NameError, then it would become a normal exception within the flow graph, and you would end up compiling a program that contains an explicit "raise NameError". In this case it's better just to crash the flow objspace clearly. Maybe a nicer hack than Christian's can be found for this purpose, e.g. setting w_NameError to a special value that would trigger an (interp-level) AssertionError in the constructor of OperationError -- the advantage is that the AssertionError can provide the meaningful bit of information: the intended message of the faulty w_NameError. A bientot, Armin
On Fri, Jan 07, 2005 at 18:09 +0000, Armin Rigo wrote:
On Fri, Jan 07, 2005 at 12:39:41PM +0100, holger krekel wrote:
Log: changed LOAD_GLOBAL to support the flow space better. In case of a NameError, where the space does not define NameError, we raise a real NameError with a message.
(...). It seems the flow object space could easily provide its own w_NameError and then you can check the usual way, catching an OperationError and then for e.match(space, space.w_NameError) without changing the interpreter.
I guess the goal is to have the problem crash the program with a meaningful error message. If the flow objspace provided a w_NameError, then it would become a normal exception within the flow graph, and you would end up compiling a program that contains an explicit "raise NameError". In this case it's better just to crash the flow objspace clearly.
Probably, it depends on the exact context. But forgetting about this detail, I don't yet see the point of throwing a NameError instead of an AttributeError here with a slightly modified message. I mean if you see the lines like ... w_exc_type = f.space.w_NameError AtttributeError: 'FlowObjSpace' object has no attribute 'w_NameError' you'll now see (approximately): ... raise NameError, (message + ", but %s has no NameError!" % f.space) NameError: global name w_NameError is not defined, but FlowObjSpace has no NameError! why is the latter more helpful?
Maybe a nicer hack than Christian's can be found for this purpose, e.g. setting w_NameError to a special value that would trigger an (interp-level) AssertionError in the constructor of OperationError -- the advantage is that the AssertionError can provide the meaningful bit of information: the intended message of the faulty w_NameError.
Makes sense. cheers, holger
Hi Holger, On Fri, Jan 07, 2005 at 07:40:09PM +0100, holger krekel wrote:
you'll now see (approximately): NameError: global name w_NameError is not defined, but FlowObjSpace has no NameError!
No, the point is that you'll now see: NameError: global name xyzzy is not defined, but FlowObjSpace has no NameError! where 'xyzzy' is the name of the particular variable in the user source code that is not defined. Armin
Hi Armin, On Fri, Jan 07, 2005 at 19:10 +0000, Armin Rigo wrote:
On Fri, Jan 07, 2005 at 07:40:09PM +0100, holger krekel wrote:
you'll now see (approximately): NameError: global name w_NameError is not defined, but FlowObjSpace has no NameError!
No, the point is that you'll now see:
NameError: global name xyzzy is not defined, but FlowObjSpace has no NameError!
where 'xyzzy' is the name of the particular variable in the user source code that is not defined.
ah ok, right. But your suggestion of providing a specific w_NameError (maybe just None) and an appropriate assertion in OperationError.__init__ still sounds nicer and more generic. holger
Armin Rigo wrote:
Hi,
On Fri, Jan 07, 2005 at 12:39:41PM +0100, holger krekel wrote:
Log: changed LOAD_GLOBAL to support the flow space better. In case of a NameError, where the space does not define NameError, we raise a real NameError with a message.
(...). It seems the flow object space could easily provide its own w_NameError and then you can check the usual way, catching an OperationError and then for e.match(space, space.w_NameError) without changing the interpreter.
No, I tried that in the last sprint, but Armin hit my fingers and said "no no, we *want* to crash here!", which is correct. So I thought to crash with a useful message, instead.
I guess the goal is to have the problem crash the program with a meaningful error message. If the flow objspace provided a w_NameError, then it would become a normal exception within the flow graph, and you would end up compiling a program that contains an explicit "raise NameError". In this case it's better just to crash the flow objspace clearly.
Exactly. An errorof this kind is usually a bug in the source code, and actually I found one in _formatting.
Maybe a nicer hack than Christian's can be found for this purpose, e.g. setting w_NameError to a special value that would trigger an (interp-level) AssertionError in the constructor of OperationError -- the advantage is that the AssertionError can provide the meaningful bit of information: the intended message of the faulty w_NameError.
Sure, there are better ways. For me, having the real name error instead of a simple crash was extremely helpful, because I finally wanted to get that translation out of the door. :-) But don't allow a NameError, because that is a violation of the flow space's contract: Global names are constants and must exist. ciao - chris -- Christian Tismer :^) <mailto:tismer@stackless.com> tismerysoft GmbH : Have a break! Take a ride on Python's Johannes-Niemeyer-Weg 9A : *Starship* http://starship.python.net/ 14109 Berlin : PGP key -> http://wwwkeys.pgp.net/ work +49 30 802 86 56 mobile +49 173 24 18 776 fax +49 30 80 90 57 05 PGP 0x57F3BF04 9064 F4E1 D754 C2FF 1619 305B C09C 5A3B 57F3 BF04 whom do you want to sponsor today? http://www.stackless.com/
participants (4)
-
Armin Rigo -
Christian Tismer -
holger krekel -
hpk＠trillke.net