David Grundberg | 1 Aug 2009 08:44
Picon
Picon
Picon
Favicon

Re: Classdef parser patch

John W. Eaton skrev:
> On 28-Apr-2009, Ryan Rusaw wrote:
>
> | Attached is a first stab at adding the basis for classdef files to the
> | lexer & the parser. With it the parser should be able to correctly
> | parse classdef files, although it currently doesn't do anything more
> | with them, as a number of items are still necessary to complete a
> | serviceable implementation:
> | 
> | 1. +package directory support.
> | 2. classdef support added to pt-* hierarchy.
> | 3. octave_value subclass for classdef objects.
> | 4. integrate with metaclass code, either the code Michael Goffioul
> | provided previously on the mailing list or something similar.
> | 5. add support for class events.
> | 6. add support for the automatic resolution of class property get & set methods.
> | 
> | Note: I used the Matlab documentation available at
> | http://www.mathworks.com/access/helpdesk/help/techdoc/index.html?/access/helpdesk/help/techdoc/matlab_oop/ug_intropage.html
> | as the basis for the changes, so it should be as close to complete as
> | the documentation allowed. If anyone finds something I've missed or
> | overlooked, please let me know.
>
> I checked in this changeset with some additional changes.
>
> | +maybe_property_get_set : // empty
> | +		  { 
> | +		    if (reading_classdef_file || lexer_flags.parsing_classdef) 
> | +			  {
> | +			    lexer_flags.maybe_classdef_get_set_method = true; 
> | +			  } 
> | +		  }
> | +		;
> | +
> |  function_beg	: push_fcn_symtab FCN stash_comment
> | -		  { $$ = $3; }
> | -		;
> | -
> | -function	: function_beg function1
> | +		  { $$ = $3; 
> | +		    if (reading_classdef_file || lexer_flags.parsing_classdef) 
> | +			  {
> | +		  	    lexer_flags.parsing_class_method = true;
> | +		  	  } 
> | +		  }
> | +		;
> | +		
> | +function : function_beg function1
> |  		  {
> |  		    $$ = finish_function (0, $2, $1);
> |  		    recover_from_parsing_function ();
> |  		  }
> | -		| function_beg return_list '=' function1
> | -		  {
> | -		    $$ = finish_function ($2, $4, $1);
> | +		| function_beg return_list maybe_property_get_set '=' function1
> | +		  {
> | +		    $$ = finish_function ($2, $5, $1);
> |  		    recover_from_parsing_function ();
> |  		  }
> |  		;
>
> This doesn't look quite right to me.  It appears that
> lexer_flags.parsing_class_method will be set to true after
> function_beg is recognized in a classdef file, but then you have
> maybe_property_get_set doing that again in the
>
>   function_beg return_list maybe_property_get_set '=' function1
>
> pattern.  Since that seems redundant, I eliminated the
> maybe_property_get_set non-terminal.  But it seems that you might have
> intended to avoid having lexer_flags.parsing_class_method set to true
> when the return_list is parsed.  I don't see how to do that without
> adding another shift/reduce conflict to the parser, but maybe you or
> someone else will have a clue about how it could be done, or maybe
> there is a better way to handle the get/set methods.
>
> David, I'm copying you on this mail because you recently modified the
> parser.  Not surprisingly, your changes caused some conflicts when
> applying Ryan's patch.  I fixed them up as best I could, but it would
> be helpful if both of you could look at the current versions of
> parse.y and lex.l and see if I screwed anything up with my merge.
>   

I've looked at the diff and it seems alright, as far as I can tell.

Regards,
David

> Ryan, also created a ChangeLog entry for you.  It would be helpful if
> you could include a ChangeLog entry with future changesets.
>
> Thanks,
>
> jwe
>   


Gmane