Paulo Gaspar wrote:
>
> Hi again,
>
> Something looks wrong in the ASTMethod.execute() method.
>
[SNIP]
>
> Now, it looks like the Method to call is cached by node (as in the Iterator
> class), with
> context.icacheGet( this );
> and by class, with
Yes, cached by node per context.
> if ( ... && icd.contextData == c )
>
> Now, this commentary in the code
> /*
> * like ASTIdentifier, if we have cache information, and the
> * Class of Object o is the same as that in the cache, we are
> * safe.
> */
> looks wrong. It is even a practical contradiction to this one:
> /*
> * sadly, we do need recalc the values of the args, as this
> can
> * change from visit to visit
> */
> since it is _not only_ the values of the arguments that might change. Their
> _class_ might change too and that can determine that another method would be
> the right match.
That's true. I didn't think of that. We can fix that by caching args
as well, and seeing if the change. Hm. This would only come into play
when a context is resused, rather than during node revisitation. Will
fix today... good catch.
> The right method match depends on the object class, the method name _AND_
> the
> number and classes of the passed arguments.
> The strange part is that the org.apache.velocity.util.introspection package
> considers all these bits!
> =:o)
But local caching of this information via the 'local context' is
extremely efficient. Combining the two would be a good thing, I guess.
> Again, I am putting my hands on this, but just want to check if I am missing
> something.
Again, I still don't understand what that means... Are you are doing a
rewrite of this stuff to submit as a patch? Let us know :)
> Have fun... and say something,
> Paulo Gaspar
--
Geir Magnusson Jr. [EMAIL PROTECTED]
Velocity : it's not just a good idea. It should be the law.
http://jakarta.apache.org/velocity
