Hi Guenther,

Please use statically-defined strings in a header, e.g.

extern NSString * kLKInvalidSymbolError;

With a .m file defining them, so that implementations can do simple  
pointer comparison.

Please don't replace the NotImplementedException in LKAST with an  
error, since it's an error in the compiler, not in the code.  That  
should never be reached, it's just there so that I can easily add AST  
nodes and get kicked if I forget to add code for compiling them.   
Changing it to subclassResponsiblity: is fine.

Please change LKErrorHander to a protocol with a concrete  
LKDefaultErrorHandler for the current implementation.

Please add a userInfo to the error methods, allowing things to add  
extra info as required.  For example, the superclass and class names  
for the SuperclassNotPresent error.

You seem to be discarding information now.  For example, in an  
LKDeclRef, you are not passing the token  to the error handler, so the  
user has no way of knowing which variable is referencing an undefined  
value.  Same with SuperclassNotPresent, which isn't returning the  
superclass token and most others.

Please don't make changes which do nothing except change the  
formatting to deviate from our style guides - e.g. in LKCompiler where  
you change spaces to tabs.  Étoilé style guides specify tabs for  
indentation, spaces for alignment.

Please get rid of the NS_DURING in LKCompiler and modify smalltalk.y  
to call the error delegate on errors directly.

-errorHandler in LKModule should be returning a singleton instance of  
LKDefaultErrorHandler if the ivar is nil.  Please create it in  
+initialize and set it in -init.  Also provide a  
+setDefaultErrorHandler: to allow it to be modified.

Not sure what you think you are trying to do with setting the default  
handler in SmalltalkParser, but it's something that shouldn't be done  
in the front end anyway.

Thanks for working on this,

David

On 23 Dec 2008, at 16:06, Guenther Noack wrote:

> Hi!
>
> As the review system seems to be down at the moment, here's the
> error handling patch for LanguageKit and SmalltalkKit. This patch
> changes the AST nodes to report errors using delegation to an
> error handler object instead of throwing exceptions.
>
> You can have a look at the applied diff in my branches of the
> two kits.
>
> Please have a look at it and tell me what you think! :)
>
> Best regards,
> Günther
>
> <errorhandling.diff>_______________________________________________
> Etoile-dev mailing list
> [email protected]
> https://mail.gna.org/listinfo/etoile-dev


_______________________________________________
Etoile-dev mailing list
[email protected]
https://mail.gna.org/listinfo/etoile-dev

Reply via email to