Re: weird macro params issue

On Wed, May 21, 2008 at 3:55 PM, Attila Szegedi <[email protected]> wrote:
> 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"}>

I'm looking at this and getting confused. Okay, a human looks at this
and untangles it as follows: if d is "3" and  and a's default value is
d, then a is "3" as well and c[a] is c["3"] which is "4".

Okay, fine. But this is constructed so that nothing circular is going
on. You could throw a monkey wrench in the works by putting in a
circular thing, like:

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

That would seem to lead you into an infinite recursive loop, of
course. It also has me wondering offhand if you could construct
examples that evaluated differently depending on the order, which
incomplete expression you started with first. (I honestly don't know.)
But, you know, when presented this usage pattern my initial reaction
was just that it wasn't worth supporting, though I didn't think about
it all that much. But just to say, basically, that b, a, and c have to
evaluate in the enclosing scope and not be referring to one another,
which seems like it could get rather hairy, and also, allows people to
write code like this unit test example that, to me, just looks too
tricky, excessively clever in an 'emits a  bad smell' sort of way.

> ${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.

Well, this is arguably correct, if we say that the default b=c[a] has
to evaluate in the enclosing scope. Or IOW, the default parameter
c={"3":"4"} only applies once you actually enter the body of the
macro, but not when you are in the invocation step, so at the point
where b=c[a] is trying to get at it, it's undefined.

Well, hold on. Established languages that have default parameters,
like Python, say. Does Python let you do this? I have to check.
Somehow, my sense of this is that this is not something that Guido
would care for. Larry Wall, OTOH, loves weird stuff like this.

>     [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?

Well, I'm not married to the current implementation. But to tell the
truth, I'm looking at this pragmatically and just thinking it's likely
better just to disallow the whole coding pattern and just say that
when it throws the exception in the unit test case above, it's
behaving correctly because c is not in fact defind at that exact point
in the code, and anybody who relied on this in earlier versions was
relying on buggy behavior. now, I grant, it's not that person's fault
because the behavior was never really fully specified.

So, I mean, I'm not married to the current implementation just because
I wrote or anything, but I would be very reluctant to tear it apart
and redo it just to support this coding pattern.

JR

>
> 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/
> _______________________________________________
> FreeMarker-user mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/freemarker-user
>

-------------------------------------------------------------------------
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/