Re: weird macro params issue

Jonathan -- can I have your attention for a bit on this one?

Problem is, adding this to 2.4 is tricky. I added it to 2.3, and also  
added unit test for it. I did port the logic of what I did in 2.3 Macro 
$MacroContext.sanityCheck() to 2.4 in ParameterList.fillInDefaults().  
But then I hit the assertIsDefined() call in  
getParameterMap(NamedArgsList args, Environment env). I decided to  
comment it out and see where it goes.

My unit test template contains this snippet:

<#macro m3 d b=c[a] a=d c={"3":"4"}>
${b}
</#macro>
<@m3 d="3"/>

And now it fails with:

     [junit] Java backtrace for programmers:
     [junit] ----------
     [junit] freemarker.core.InvalidReferenceException: Expression c  
is undefined on line 9, column 16 in test-macro.html.
     [junit] 	at  
freemarker 
.core 
.helpers 
.DefaultReferenceChecker.assertNonNull(DefaultReferenceChecker.java:18)
     [junit] 	at  
freemarker.core.ast.TemplateNode.assertNonNull(TemplateNode.java:108)
     [junit] 	at  
freemarker 
.core.ast.DynamicKeyName._getAsTemplateModel(DynamicKeyName.java:79)
     [junit] 	at  
freemarker.core.ast.Expression.getAsTemplateModel(Expression.java:102)
     [junit] 	at  
freemarker.core.ast.ParameterList.getParameterMap(ParameterList.java: 
257)
     [junit] 	at  
freemarker.core.ast.ParameterList.getParameterMap(ParameterList.java: 
234)
     [junit] 	at freemarker.core.Environment.render(Environment.java: 
673)
     [junit] 	at  
freemarker.core.ast.UnifiedCall.execute(UnifiedCall.java:122)

Now, obviously, getParameterMap will run before fillInDefaults(). And,  
as the crux of the matter, I'm getting to think that fillInDefaults()  
should be called from other places as well, not just from  
MacroContext.runMacro() -- if I'm not mistaken, TemplateMethodModel/ 
TemplateTransformModel/TemplateDirectiveModel can now carry  
annotations that allow defining formal parameter lists, complete with  
defaults, so I'm thinking that maybe instead of getParameterMap()/ 
fillInDefaults() we'd need a combined method, maybe named  
applyToScope() that'd both evaluate defaults and define all parameters  
(required and optional) into a passed scope -- we could spin up a  
small scope for Template{Method|Transform|Directive}Model invocations  
for sake of uniformity.

Does this direction of changes sound okay to you?

Attila.

On 2008.05.19., at 19:26, Attila Szegedi wrote:

> Ok, I have this working on 2.3 branch. Getting it to work in 2.4  
> seems to be a bit more involved - actually, I just need to  
> familiarize myself a bit with it first. Jonathan made few  
> assumptions in the code that default expressions will always  
> evaluate at first; I need to work around that.
>
> Attila.
>
> On 2008.05.02., at 21:08, Attila Szegedi wrote:
>
>> On 2008.05.02., at 19:43, Jonathan Revusky wrote:
>>
>>> I misspoke above. I meant, here, that it would be better if it  
>>> always
>>> worked than if it worked some of the time and failed other times. I
>>> mean, it should always work or always fail... :-)
>>>
>>> JR
>>> but I think I would just tend towards it always failing.
>>
>> What's funny is that I have an idea how to make it work whenever  
>> possible, but making it always fail seems less trivial, as you need  
>> to prove none of the default expressions actually depend on other  
>> parameters; i.e. you could have an argument list of "a  
>> b=someGlobalVar[a]"; you'd need to analyze the full default  
>> expression of 'b' for any occurrence of 'a', and then it can be  
>> even more hidden, like b=someGlobalVar[.local["a"]]; you get the  
>> idea. The "always work" would however cover even these horrors  
>> naturally :-)
>>
>> It would have one last issue with not being deterministic with  
>> existence built-ins, i.e.
>>
>> [#macro x a b=a c=b?default("foo")]
>> ${c}
>> [/#macro]
>>
>> [@x "bar"/]
>>
>> would nondeterministically print either "foo" or "bar", which can  
>> be further rectified by always doing the evaluation in declaration  
>> order within every iteration. I guess that would have us covered...  
>> I *know* it'd be a godawful practice to use an existence builtin in  
>> defaulted arg list because the engine doesn't allow you to have any  
>> param be undefined, but these are still corner cases allowed by the  
>> language, so we'd better address them to be deterministic...
>>
>> Attila.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft 
Defy all challenges. Microsoft(R) Visual Studio 2008. 
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/