You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@velocity.apache.org by Byron Foster <by...@base2.cc> on 2008/10/11 02:04:20 UTC
Strange pass by name behavior
Given the following:
#macro(test $a $b)
#foreach($i in $a) $b #end
#end
#test( [1, 2, 3] "#if($i == 2) yes #else no #end")
I would expect the output:
no yes no
Instead, I get
no no no
However, if I have
#macro(test $a $b)
#set($i = 1) $b #set($i = 2) $b #set($i = 3) $b
#end
#test( [1, 2, 3] "#if($i == 2) yes #else no #end")
then I get:
no yes no
as expected.
I would say there is a bug here. The Foreach directive explicitly
sets $i in the localscope. If I replace the explicit put to the
localscope with a regular put, the first macro case works. I was
looking at the following issues where the modification to Foreach was
mode.
https://issues.apache.org/jira/browse/VELOCITY-285
Now, my test change to Foreach breaks issue 285, but I'm not convinced
that the resolution to 285 is complete. Thoughts?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org
Re: Strange pass by name behavior
Posted by Nathan Bubna <nb...@gmail.com>.
Yeah, everyone's vote matters. Besides, i lean towards 1 as well, for
the "option soup" reason.
I don't know that 630 is any more likely than 285, but i think we
should default to being strict about pass by name, since that is the
set pattern.
By the way, the "suggestion box" for 2.0 is JIRA. No better place to
track such ideas and arguments. :)
On Wed, Oct 15, 2008 at 3:57 PM, Byron Foster <by...@base2.cc> wrote:
>
> Hmm, well if it means anything I would vote for 1. I think adding another config option is contributing to option soup. And I think the majority of users, even experienced users aren't really going to want to consider the implications of strict pass by name. With 1) there is an option for those who are really effected by 285.
>
> Between 285 and 630, I think 630 is going to hit users more, even though I agree 285 is not desirable.
>
> If I could drop a note into the suggestion box for 2.0 I would say that pass by value should be the default, with pass by name being the exception. 285 shows that people intuitively assume pass by value behavior. There is clear utility in being able to pass by name, but a developer should specify pass by name because they know why they want to use it, and have considered the implications.
>
> Additionally, there are performance gains to be realized with pass by value. Currently, All references in a macro are resolved by searching up the macro "stack", or context chain. In fact, even pass by name could be passed by value since ASTReference could simply look to see if a value is an ASTprocess, and if so execute it. A specialized context for storing ASTs would not be necessary.
>
> While I'm rambling... I would also make macros local scope, and create a global directive. Pass by name would work with local scope, which is currently not the case. I think all of this would lead to more intuitive behavior that could be clearly described in the Velocity docs, and that users would not find surprising.
>
> On Oct 15, 2008, at 11:01 , Nathan Bubna wrote:
>
>> ok, so here's my current thought. since we've already changed this in
>> the last production release, i'm hesitant to just "change our mind"
>> and leave the VELOCITY-285 advocates hanging. so, here's two options
>> i think i'd be comfortable with:
>>
>> 1:
>> Index: ../src/java/org/apache/velocity/runtime/directive/Foreach.java
>> ===================================================================
>> --- ../src/java/org/apache/velocity/runtime/directive/Foreach.java
>> (revision 701169)
>> +++ ../src/java/org/apache/velocity/runtime/directive/Foreach.java
>> (working copy)
>> @@ -247,6 +247,15 @@
>> }
>>
>> /**
>> + * Extension hook to allow VELOCITY-285 behavior by overriding
>> + * and using context.localPut(key, value). See VELOCITY-630.
>> + */
>> + protected void put(InternalContextAdapter context, String key,
>> Object value)
>> + {
>> + context.put(key, value);
>> + }
>> +
>> + /**
>> * renders the #foreach() block
>> * @param context
>> * @param writer
>> @@ -325,11 +334,11 @@
>>
>> while (!maxNbrLoopsExceeded && i.hasNext())
>> {
>> - // TODO: JDK 1.4+ -> Integer.valueOf()
>> - context.localPut(counterName , new Integer(counter));
>> - context.localPut(hasNextName, Boolean.valueOf(i.hasNext()));
>> + // TODO: JDK 1.5+ -> Integer.valueOf()
>> + put(context, counterName , new Integer(counter));
>> + put(context, hasNextName, Boolean.valueOf(i.hasNext()));
>> Object value = i.next();
>> - context.localPut(elementKey, value);
>> + put(context, elementKey, value);
>>
>> or 2:
>> Index: ../src/java/org/apache/velocity/runtime/directive/Foreach.java
>> ===================================================================
>> --- ../src/java/org/apache/velocity/runtime/directive/Foreach.java
>> (revision 701169)
>> +++ ../src/java/org/apache/velocity/runtime/directive/Foreach.java
>> (working copy)
>> @@ -177,6 +177,11 @@
>> private boolean skipInvalidIterator;
>>
>> /**
>> + * Whether or not {@link #put} uses context.put or context.localPut
>> + */
>> + private boolean strictPassByName;
>> +
>> + /**
>> * The reference name used to access each
>> * of the elements in the list object. It
>> * is the $item in the following:
>> @@ -217,6 +222,7 @@
>> }
>> skipInvalidIterator =
>> rsvc.getBoolean(RuntimeConstants.SKIP_INVALID_ITERATOR, true);
>> + strictPassByName =
>> rsvc.getBoolean(RuntimeConstants.STRICT_FOREACH_PASS_BY_NAME, true);
>>
>> /*
>> * this is really the only thing we can do here as everything
>> @@ -247,6 +253,22 @@
>> }
>>
>> /**
>> + * Controls whether loop vars are set locally or generally. See
>> VELOCITY-630
>> + * and VELOCITY-285.
>> + */
>> + protected void put(InternalContextAdapter context, String key,
>> Object value)
>> + {
>> + if (strictPassByName)
>> + {
>> + context.put(key, value);
>> + }
>> + else
>> + {
>> + context.localPut(key, value);
>> + }
>> + }
>> +
>> + /**
>> * renders the #foreach() block
>> * @param context
>> * @param writer
>> @@ -325,11 +347,11 @@
>>
>> while (!maxNbrLoopsExceeded && i.hasNext())
>> {
>> - // TODO: JDK 1.4+ -> Integer.valueOf()
>> - context.localPut(counterName , new Integer(counter));
>> - context.localPut(hasNextName, Boolean.valueOf(i.hasNext()));
>> + // TODO: JDK 1.5+ -> Integer.valueOf()
>> + put(context, counterName , new Integer(counter));
>> + put(context, hasNextName, Boolean.valueOf(i.hasNext()));
>> Object value = i.next();
>> - context.localPut(elementKey, value);
>> + put(context, elementKey, value);
>>
>> On Tue, Oct 14, 2008 at 9:40 PM, Nathan Bubna <nb...@gmail.com> wrote:
>>>
>>> hmm. i suppose that makes sense... of course, i'm too tired to think
>>> straight right now. let me re-examine this tomorrow.
>>>
>>> On Tue, Oct 14, 2008 at 6:39 PM, Byron Foster <by...@base2.cc> wrote:
>>>>
>>>> So I looked into this some more and here is the deal. This bug was
>>>> introduced with VELOCITY-285. The solution to 285 was to make the #foreach
>>>> index variable forced to local scope, which introduced bug VELOCITY-630.
>>>> However, I assert that 285 was never a bug!
>>>>
>>>> Given the following VTL:
>>>>
>>>> #macro(dig $x)
>>>> x1: $x
>>>> #foreach($i in $x)
>>>> x2 $x
>>>> #dig($i)
>>>> #end
>>>> #end
>>>> #dig([[1,2],[3,4]])
>>>>
>>>> The premiss of 285 is that reference $x at x1: and x2: should render with
>>>> the same value. However, in a pass by name world this is not true. On at
>>>> least the second recursion level to #dig the reference $x is essentially an
>>>> alias for $i. Since $i changes value between x1 and x2, rendering $x will
>>>> change.
>>>>
>>>> I agree with the sentiment that this is surprising, but it is working as it
>>>> should.
>>>>
>>>> On Oct 14, 2008, at 1:58 , Byron Foster wrote:
>>>>
>>>>> Actually I don't think it's all that odd, the error case is somewhat
>>>>> general (it came up in my own usage of Velocity to generate HTML lists). Any
>>>>> pass by name reference to the #foreach iteration variable will fail, it
>>>>> doesn't need to be in a string literal, for example:
>>>>>
>>>>> #macro(test $x)#foreach($i in [1, 2])$x#end#end
>>>>> #test($i)
>>>>>
>>>>> Maybe a different disscussion, but somewhat related to how #foreach is
>>>>> implemented with local scope. The macro localscope setting
>>>>> (velocimacro.context.localscope) has the same behavior as above for the #set
>>>>> directive. However, I'm not sure what Velocity's intended behavior is with
>>>>> scoping rules and pass by name.
>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
>>>> For additional commands, e-mail: dev-help@velocity.apache.org
>>>>
>>>>
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
>> For additional commands, e-mail: dev-help@velocity.apache.org
>>
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
> For additional commands, e-mail: dev-help@velocity.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org
Re: Strange pass by name behavior
Posted by Byron Foster <by...@base2.cc>.
Serge Knystautas wrote:
> On Wed, Oct 15, 2008 at 6:57 PM, Byron Foster <by...@base2.cc> wrote:
>> Hmm, well if it means anything I would vote for 1. I think adding
>> another config option is contributing to option soup. And I think
>> the majority of users, even experienced users aren't really going
>> to want to consider the implications of strict pass by name. With
>> 1) there is an option for those who are really effected by 285.
>
> The example with x1 and x2 and $x was helpful in understanding the
> problem. Could someone put option 1 and 2 in terms of how it would
> affect that example?
Given the VTL:
#macro(dig $x)
x1: $x
#foreach($i in $x)
x2 $x
#dig($i)
#end
#end
#dig([[1,2],[3,4]])
option 1) (Which Nathan applied) will cause $x at x1: and at x2 to
render to two different values on at least the second recursion to dig.
This essentially rolls back the behavioral change in VELOCITY-285, but
provides a hook that allows developers to get back to option 2 behavior
if they depend on it...
option 2) $x at x1: and x2: would render to the same. But a config
property would be added to get option 1 behavior.
It's really just a question of what is the default behavior. My
personal feeling is that option 1 is the correct behavior for pass by
name, and so it is consistent. I think pass by name can be confusing,
but it is certainly compounded by adding special cases that modify the
behavior.
I mean to, but havn't gottten to it yet, modify Velocity so that it is
pass by value, and find how many test cases I break. I would be curious
to see how much of a shift this would be for Velocity.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org
Re: Strange pass by name behavior
Posted by Serge Knystautas <sk...@gmail.com>.
On Wed, Oct 15, 2008 at 6:57 PM, Byron Foster <by...@base2.cc> wrote:
> Hmm, well if it means anything I would vote for 1. I think adding another
> config option is contributing to option soup. And I think the majority of
> users, even experienced users aren't really going to want to consider the
> implications of strict pass by name. With 1) there is an option for those
> who are really effected by 285.
The example with x1 and x2 and $x was helpful in understanding the
problem. Could someone put option 1 and 2 in terms of how it would
affect that example?
--
Serge Knystautas
Lokitech >> software . strategy . design >> http://www.lokitech.com
p. 301.656.5501
e. sergek@lokitech.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org
Re: Strange pass by name behavior
Posted by Byron Foster <by...@base2.cc>.
Hmm, well if it means anything I would vote for 1. I think adding
another config option is contributing to option soup. And I think the
majority of users, even experienced users aren't really going to want
to consider the implications of strict pass by name. With 1) there is
an option for those who are really effected by 285.
Between 285 and 630, I think 630 is going to hit users more, even
though I agree 285 is not desirable.
If I could drop a note into the suggestion box for 2.0 I would say
that pass by value should be the default, with pass by name being the
exception. 285 shows that people intuitively assume pass by value
behavior. There is clear utility in being able to pass by name, but a
developer should specify pass by name because they know why they want
to use it, and have considered the implications.
Additionally, there are performance gains to be realized with pass by
value. Currently, All references in a macro are resolved by
searching up the macro "stack", or context chain. In fact, even pass
by name could be passed by value since ASTReference could simply look
to see if a value is an ASTprocess, and if so execute it. A
specialized context for storing ASTs would not be necessary.
While I'm rambling... I would also make macros local scope, and create
a global directive. Pass by name would work with local scope, which
is currently not the case. I think all of this would lead to more
intuitive behavior that could be clearly described in the Velocity
docs, and that users would not find surprising.
On Oct 15, 2008, at 11:01 , Nathan Bubna wrote:
> ok, so here's my current thought. since we've already changed this in
> the last production release, i'm hesitant to just "change our mind"
> and leave the VELOCITY-285 advocates hanging. so, here's two options
> i think i'd be comfortable with:
>
> 1:
> Index: ../src/java/org/apache/velocity/runtime/directive/Foreach.java
> ===================================================================
> --- ../src/java/org/apache/velocity/runtime/directive/Foreach.java
> (revision 701169)
> +++ ../src/java/org/apache/velocity/runtime/directive/Foreach.java
> (working copy)
> @@ -247,6 +247,15 @@
> }
>
> /**
> + * Extension hook to allow VELOCITY-285 behavior by overriding
> + * and using context.localPut(key, value). See VELOCITY-630.
> + */
> + protected void put(InternalContextAdapter context, String key,
> Object value)
> + {
> + context.put(key, value);
> + }
> +
> + /**
> * renders the #foreach() block
> * @param context
> * @param writer
> @@ -325,11 +334,11 @@
>
> while (!maxNbrLoopsExceeded && i.hasNext())
> {
> - // TODO: JDK 1.4+ -> Integer.valueOf()
> - context.localPut(counterName , new Integer(counter));
> - context.localPut(hasNextName,
> Boolean.valueOf(i.hasNext()));
> + // TODO: JDK 1.5+ -> Integer.valueOf()
> + put(context, counterName , new Integer(counter));
> + put(context, hasNextName, Boolean.valueOf(i.hasNext()));
> Object value = i.next();
> - context.localPut(elementKey, value);
> + put(context, elementKey, value);
>
> or 2:
> Index: ../src/java/org/apache/velocity/runtime/directive/Foreach.java
> ===================================================================
> --- ../src/java/org/apache/velocity/runtime/directive/Foreach.java
> (revision 701169)
> +++ ../src/java/org/apache/velocity/runtime/directive/Foreach.java
> (working copy)
> @@ -177,6 +177,11 @@
> private boolean skipInvalidIterator;
>
> /**
> + * Whether or not {@link #put} uses context.put or
> context.localPut
> + */
> + private boolean strictPassByName;
> +
> + /**
> * The reference name used to access each
> * of the elements in the list object. It
> * is the $item in the following:
> @@ -217,6 +222,7 @@
> }
> skipInvalidIterator =
> rsvc.getBoolean(RuntimeConstants.SKIP_INVALID_ITERATOR,
> true);
> + strictPassByName =
> rsvc.getBoolean(RuntimeConstants.STRICT_FOREACH_PASS_BY_NAME, true);
>
> /*
> * this is really the only thing we can do here as everything
> @@ -247,6 +253,22 @@
> }
>
> /**
> + * Controls whether loop vars are set locally or generally. See
> VELOCITY-630
> + * and VELOCITY-285.
> + */
> + protected void put(InternalContextAdapter context, String key,
> Object value)
> + {
> + if (strictPassByName)
> + {
> + context.put(key, value);
> + }
> + else
> + {
> + context.localPut(key, value);
> + }
> + }
> +
> + /**
> * renders the #foreach() block
> * @param context
> * @param writer
> @@ -325,11 +347,11 @@
>
> while (!maxNbrLoopsExceeded && i.hasNext())
> {
> - // TODO: JDK 1.4+ -> Integer.valueOf()
> - context.localPut(counterName , new Integer(counter));
> - context.localPut(hasNextName,
> Boolean.valueOf(i.hasNext()));
> + // TODO: JDK 1.5+ -> Integer.valueOf()
> + put(context, counterName , new Integer(counter));
> + put(context, hasNextName, Boolean.valueOf(i.hasNext()));
> Object value = i.next();
> - context.localPut(elementKey, value);
> + put(context, elementKey, value);
>
> On Tue, Oct 14, 2008 at 9:40 PM, Nathan Bubna <nb...@gmail.com>
> wrote:
>> hmm. i suppose that makes sense... of course, i'm too tired to think
>> straight right now. let me re-examine this tomorrow.
>>
>> On Tue, Oct 14, 2008 at 6:39 PM, Byron Foster <by...@base2.cc> wrote:
>>> So I looked into this some more and here is the deal. This bug was
>>> introduced with VELOCITY-285. The solution to 285 was to make the
>>> #foreach
>>> index variable forced to local scope, which introduced bug
>>> VELOCITY-630.
>>> However, I assert that 285 was never a bug!
>>>
>>> Given the following VTL:
>>>
>>> #macro(dig $x)
>>> x1: $x
>>> #foreach($i in $x)
>>> x2 $x
>>> #dig($i)
>>> #end
>>> #end
>>> #dig([[1,2],[3,4]])
>>>
>>> The premiss of 285 is that reference $x at x1: and x2: should
>>> render with
>>> the same value. However, in a pass by name world this is not
>>> true. On at
>>> least the second recursion level to #dig the reference $x is
>>> essentially an
>>> alias for $i. Since $i changes value between x1 and x2, rendering
>>> $x will
>>> change.
>>>
>>> I agree with the sentiment that this is surprising, but it is
>>> working as it
>>> should.
>>>
>>> On Oct 14, 2008, at 1:58 , Byron Foster wrote:
>>>
>>>> Actually I don't think it's all that odd, the error case is
>>>> somewhat
>>>> general (it came up in my own usage of Velocity to generate HTML
>>>> lists). Any
>>>> pass by name reference to the #foreach iteration variable will
>>>> fail, it
>>>> doesn't need to be in a string literal, for example:
>>>>
>>>> #macro(test $x)#foreach($i in [1, 2])$x#end#end
>>>> #test($i)
>>>>
>>>> Maybe a different disscussion, but somewhat related to how
>>>> #foreach is
>>>> implemented with local scope. The macro localscope setting
>>>> (velocimacro.context.localscope) has the same behavior as above
>>>> for the #set
>>>> directive. However, I'm not sure what Velocity's intended
>>>> behavior is with
>>>> scoping rules and pass by name.
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
>>> For additional commands, e-mail: dev-help@velocity.apache.org
>>>
>>>
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
> For additional commands, e-mail: dev-help@velocity.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org
Re: Strange pass by name behavior
Posted by Nathan Bubna <nb...@gmail.com>.
ok, so here's my current thought. since we've already changed this in
the last production release, i'm hesitant to just "change our mind"
and leave the VELOCITY-285 advocates hanging. so, here's two options
i think i'd be comfortable with:
1:
Index: ../src/java/org/apache/velocity/runtime/directive/Foreach.java
===================================================================
--- ../src/java/org/apache/velocity/runtime/directive/Foreach.java
(revision 701169)
+++ ../src/java/org/apache/velocity/runtime/directive/Foreach.java
(working copy)
@@ -247,6 +247,15 @@
}
/**
+ * Extension hook to allow VELOCITY-285 behavior by overriding
+ * and using context.localPut(key, value). See VELOCITY-630.
+ */
+ protected void put(InternalContextAdapter context, String key,
Object value)
+ {
+ context.put(key, value);
+ }
+
+ /**
* renders the #foreach() block
* @param context
* @param writer
@@ -325,11 +334,11 @@
while (!maxNbrLoopsExceeded && i.hasNext())
{
- // TODO: JDK 1.4+ -> Integer.valueOf()
- context.localPut(counterName , new Integer(counter));
- context.localPut(hasNextName, Boolean.valueOf(i.hasNext()));
+ // TODO: JDK 1.5+ -> Integer.valueOf()
+ put(context, counterName , new Integer(counter));
+ put(context, hasNextName, Boolean.valueOf(i.hasNext()));
Object value = i.next();
- context.localPut(elementKey, value);
+ put(context, elementKey, value);
or 2:
Index: ../src/java/org/apache/velocity/runtime/directive/Foreach.java
===================================================================
--- ../src/java/org/apache/velocity/runtime/directive/Foreach.java
(revision 701169)
+++ ../src/java/org/apache/velocity/runtime/directive/Foreach.java
(working copy)
@@ -177,6 +177,11 @@
private boolean skipInvalidIterator;
/**
+ * Whether or not {@link #put} uses context.put or context.localPut
+ */
+ private boolean strictPassByName;
+
+ /**
* The reference name used to access each
* of the elements in the list object. It
* is the $item in the following:
@@ -217,6 +222,7 @@
}
skipInvalidIterator =
rsvc.getBoolean(RuntimeConstants.SKIP_INVALID_ITERATOR, true);
+ strictPassByName =
rsvc.getBoolean(RuntimeConstants.STRICT_FOREACH_PASS_BY_NAME, true);
/*
* this is really the only thing we can do here as everything
@@ -247,6 +253,22 @@
}
/**
+ * Controls whether loop vars are set locally or generally. See
VELOCITY-630
+ * and VELOCITY-285.
+ */
+ protected void put(InternalContextAdapter context, String key,
Object value)
+ {
+ if (strictPassByName)
+ {
+ context.put(key, value);
+ }
+ else
+ {
+ context.localPut(key, value);
+ }
+ }
+
+ /**
* renders the #foreach() block
* @param context
* @param writer
@@ -325,11 +347,11 @@
while (!maxNbrLoopsExceeded && i.hasNext())
{
- // TODO: JDK 1.4+ -> Integer.valueOf()
- context.localPut(counterName , new Integer(counter));
- context.localPut(hasNextName, Boolean.valueOf(i.hasNext()));
+ // TODO: JDK 1.5+ -> Integer.valueOf()
+ put(context, counterName , new Integer(counter));
+ put(context, hasNextName, Boolean.valueOf(i.hasNext()));
Object value = i.next();
- context.localPut(elementKey, value);
+ put(context, elementKey, value);
On Tue, Oct 14, 2008 at 9:40 PM, Nathan Bubna <nb...@gmail.com> wrote:
> hmm. i suppose that makes sense... of course, i'm too tired to think
> straight right now. let me re-examine this tomorrow.
>
> On Tue, Oct 14, 2008 at 6:39 PM, Byron Foster <by...@base2.cc> wrote:
>> So I looked into this some more and here is the deal. This bug was
>> introduced with VELOCITY-285. The solution to 285 was to make the #foreach
>> index variable forced to local scope, which introduced bug VELOCITY-630.
>> However, I assert that 285 was never a bug!
>>
>> Given the following VTL:
>>
>> #macro(dig $x)
>> x1: $x
>> #foreach($i in $x)
>> x2 $x
>> #dig($i)
>> #end
>> #end
>> #dig([[1,2],[3,4]])
>>
>> The premiss of 285 is that reference $x at x1: and x2: should render with
>> the same value. However, in a pass by name world this is not true. On at
>> least the second recursion level to #dig the reference $x is essentially an
>> alias for $i. Since $i changes value between x1 and x2, rendering $x will
>> change.
>>
>> I agree with the sentiment that this is surprising, but it is working as it
>> should.
>>
>> On Oct 14, 2008, at 1:58 , Byron Foster wrote:
>>
>>> Actually I don't think it's all that odd, the error case is somewhat
>>> general (it came up in my own usage of Velocity to generate HTML lists). Any
>>> pass by name reference to the #foreach iteration variable will fail, it
>>> doesn't need to be in a string literal, for example:
>>>
>>> #macro(test $x)#foreach($i in [1, 2])$x#end#end
>>> #test($i)
>>>
>>> Maybe a different disscussion, but somewhat related to how #foreach is
>>> implemented with local scope. The macro localscope setting
>>> (velocimacro.context.localscope) has the same behavior as above for the #set
>>> directive. However, I'm not sure what Velocity's intended behavior is with
>>> scoping rules and pass by name.
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
>> For additional commands, e-mail: dev-help@velocity.apache.org
>>
>>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org
Re: Strange pass by name behavior
Posted by Nathan Bubna <nb...@gmail.com>.
hmm. i suppose that makes sense... of course, i'm too tired to think
straight right now. let me re-examine this tomorrow.
On Tue, Oct 14, 2008 at 6:39 PM, Byron Foster <by...@base2.cc> wrote:
> So I looked into this some more and here is the deal. This bug was
> introduced with VELOCITY-285. The solution to 285 was to make the #foreach
> index variable forced to local scope, which introduced bug VELOCITY-630.
> However, I assert that 285 was never a bug!
>
> Given the following VTL:
>
> #macro(dig $x)
> x1: $x
> #foreach($i in $x)
> x2 $x
> #dig($i)
> #end
> #end
> #dig([[1,2],[3,4]])
>
> The premiss of 285 is that reference $x at x1: and x2: should render with
> the same value. However, in a pass by name world this is not true. On at
> least the second recursion level to #dig the reference $x is essentially an
> alias for $i. Since $i changes value between x1 and x2, rendering $x will
> change.
>
> I agree with the sentiment that this is surprising, but it is working as it
> should.
>
> On Oct 14, 2008, at 1:58 , Byron Foster wrote:
>
>> Actually I don't think it's all that odd, the error case is somewhat
>> general (it came up in my own usage of Velocity to generate HTML lists). Any
>> pass by name reference to the #foreach iteration variable will fail, it
>> doesn't need to be in a string literal, for example:
>>
>> #macro(test $x)#foreach($i in [1, 2])$x#end#end
>> #test($i)
>>
>> Maybe a different disscussion, but somewhat related to how #foreach is
>> implemented with local scope. The macro localscope setting
>> (velocimacro.context.localscope) has the same behavior as above for the #set
>> directive. However, I'm not sure what Velocity's intended behavior is with
>> scoping rules and pass by name.
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
> For additional commands, e-mail: dev-help@velocity.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org
Re: Strange pass by name behavior
Posted by Byron Foster <by...@base2.cc>.
So I looked into this some more and here is the deal. This bug was
introduced with VELOCITY-285. The solution to 285 was to make the
#foreach index variable forced to local scope, which introduced bug
VELOCITY-630. However, I assert that 285 was never a bug!
Given the following VTL:
#macro(dig $x)
x1: $x
#foreach($i in $x)
x2 $x
#dig($i)
#end
#end
#dig([[1,2],[3,4]])
The premiss of 285 is that reference $x at x1: and x2: should render
with the same value. However, in a pass by name world this is not
true. On at least the second recursion level to #dig the reference $x
is essentially an alias for $i. Since $i changes value between x1 and
x2, rendering $x will change.
I agree with the sentiment that this is surprising, but it is working
as it should.
On Oct 14, 2008, at 1:58 , Byron Foster wrote:
> Actually I don't think it's all that odd, the error case is somewhat
> general (it came up in my own usage of Velocity to generate HTML
> lists). Any pass by name reference to the #foreach iteration
> variable will fail, it doesn't need to be in a string literal, for
> example:
>
> #macro(test $x)#foreach($i in [1, 2])$x#end#end
> #test($i)
>
> Maybe a different disscussion, but somewhat related to how #foreach
> is implemented with local scope. The macro localscope setting
> (velocimacro.context.localscope) has the same behavior as above for
> the #set directive. However, I'm not sure what Velocity's intended
> behavior is with scoping rules and pass by name.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org
Re: Strange pass by name behavior
Posted by Byron Foster <by...@base2.cc>.
Actually I don't think it's all that odd, the error case is somewhat
general (it came up in my own usage of Velocity to generate HTML lists).
Any pass by name reference to the #foreach iteration variable will fail,
it doesn't need to be in a string literal, for example:
#macro(test $x)#foreach($i in [1, 2])$x#end#end
#test($i)
Maybe a different disscussion, but somewhat related to how #foreach is
implemented with local scope. The macro localscope setting
(velocimacro.context.localscope) has the same behavior as above for the
#set directive. However, I'm not sure what Velocity's intended behavior
is with scoping rules and pass by name.
Nathan Bubna wrote:
> to my knowledge, macro arguments have always been passed by name,
> even string literal ones. still, this is an odd case, using the
> foreach index reference in a string literal passed to a macro that
> includes the foreach which defines that reference. yeah, that's a
> bug, but my, what a tangled web it took to catch it. :)
>
> On Sun, Oct 12, 2008 at 7:08 PM, Will Glass-Husain
> <wg...@gmail.com> wrote:
>> I would have assumed the quoted string would be evaluated before
>> being passed in to the macro. Hence $i is invalid reference when
>> it is evaluated, and we'd always see "no no no".
>>
>> WILL
>>
>> On Sun, Oct 12, 2008 at 5:17 PM, Byron Foster <by...@base2.cc>
>> wrote:
>>
>>> I went back and pulled the velocity source to the revision
>>> (471881) before the changes to bug 285 and found that the
>>> behavior is as expected, so I conclude this is a bug :) which has
>>> been created here:
>>>
>>> https://issues.apache.org/jira/browse/VELOCITY-630
>>>
>>>
>>> On Oct 10, 2008, at 17:04 , Byron Foster wrote:
>>>
>>> Given the following:
>>>> #macro(test $a $b) #foreach($i in $a) $b #end #end #test( [1,
>>>> 2, 3] "#if($i == 2) yes #else no #end")
>>>>
>>>> I would expect the output:
>>>>
>>>> no yes no
>>>>
>>>> Instead, I get
>>>>
>>>> no no no
>>>>
>>>> However, if I have
>>>>
>>>> #macro(test $a $b) #set($i = 1) $b #set($i = 2) $b #set($i = 3)
>>>> $b #end #test( [1, 2, 3] "#if($i == 2) yes #else no #end")
>>>>
>>>> then I get: no yes no as expected.
>>>>
>>>> I would say there is a bug here. The Foreach directive
>>>> explicitly sets $i in the localscope. If I replace the
>>>> explicit put to the localscope with a regular put, the first
>>>> macro case works. I was looking at the following issues where
>>>> the modification to Foreach was mode.
>>>>
>>>> https://issues.apache.org/jira/browse/VELOCITY-285
>>>>
>>>> Now, my test change to Foreach breaks issue 285, but I'm not
>>>> convinced that the resolution to 285 is complete. Thoughts?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org
Re: Strange pass by name behavior
Posted by Nathan Bubna <nb...@gmail.com>.
to my knowledge, macro arguments have always been passed by name, even
string literal ones. still, this is an odd case, using the foreach
index reference in a string literal passed to a macro that includes
the foreach which defines that reference. yeah, that's a bug, but my,
what a tangled web it took to catch it. :)
On Sun, Oct 12, 2008 at 7:08 PM, Will Glass-Husain
<wg...@gmail.com> wrote:
> I would have assumed the quoted string would be evaluated before being
> passed in to the macro. Hence $i is invalid reference when it is evaluated,
> and we'd always see "no no no".
>
> WILL
>
> On Sun, Oct 12, 2008 at 5:17 PM, Byron Foster <by...@base2.cc> wrote:
>
>> I went back and pulled the velocity source to the revision (471881) before
>> the changes to bug 285 and found that the behavior is as expected, so I
>> conclude this is a bug :) which has been created here:
>>
>> https://issues.apache.org/jira/browse/VELOCITY-630
>>
>>
>> On Oct 10, 2008, at 17:04 , Byron Foster wrote:
>>
>> Given the following:
>>>
>>> #macro(test $a $b)
>>> #foreach($i in $a) $b #end
>>> #end
>>> #test( [1, 2, 3] "#if($i == 2) yes #else no #end")
>>>
>>> I would expect the output:
>>>
>>> no yes no
>>>
>>> Instead, I get
>>>
>>> no no no
>>>
>>> However, if I have
>>>
>>> #macro(test $a $b)
>>> #set($i = 1) $b #set($i = 2) $b #set($i = 3) $b
>>> #end
>>> #test( [1, 2, 3] "#if($i == 2) yes #else no #end")
>>>
>>> then I get:
>>> no yes no
>>> as expected.
>>>
>>> I would say there is a bug here. The Foreach directive explicitly sets $i
>>> in the localscope. If I replace the explicit put to the localscope with a
>>> regular put, the first macro case works. I was looking at the following
>>> issues where the modification to Foreach was mode.
>>>
>>> https://issues.apache.org/jira/browse/VELOCITY-285
>>>
>>> Now, my test change to Foreach breaks issue 285, but I'm not convinced
>>> that the resolution to 285 is complete. Thoughts?
>>>
>>>
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
>>> For additional commands, e-mail: dev-help@velocity.apache.org
>>>
>>>
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
>> For additional commands, e-mail: dev-help@velocity.apache.org
>>
>>
>
>
> --
> Forio Business Simulations
>
> Will Glass-Husain
> wglass@forio.com
> www.forio.com
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org
Re: Strange pass by name behavior
Posted by Nathan Bubna <nb...@gmail.com>.
On Sun, Oct 12, 2008 at 8:31 PM, Byron Foster <by...@base2.cc> wrote:
> From my perusal the way the macro implementation works is that all
> parameters passed to a macros are AST, even if they are in quotes. This is
> consistent with pass by name. And anyway, if the string was evaluated
> before being passed then that doesn't explain the behavior of the second
> macro where $i is modified with #set directives instead of #foreach.
> Shouldn't the results be the same?
>
> I could simplify the call with no quotes:
>
> #test( [1, 2, 3] #if($i=2)yes#end)
>
> Which with the first macro returns nothing and with the second macro returns
> "yes"
>
> But, even without using directives (Which is probably what I should have
> done):
>
> #macro(test $x)#foreach($i in [1, 2])$x#end#end
> #test($i)
>
> returns: $i$i
>
> #macro(test $x)#set($i = 1)$x #set($i = 2)$x#end
> #test($i)
>
> returns: 1 2
>
> On a side note... the following generates a parse error.. Is this correct?
>
> #macro(test $x)#set($i = 1)$x#set($i = 2)$x#end
>
> Encountered "=" at line 6, column 38 of /foo.vm
> Was expecting:
> <RPAREN> ...
no that's not correct. looks like a parser bug. ugh.
> On Oct 12, 2008, at 19:08 , Will Glass-Husain wrote:
>
>> I would have assumed the quoted string would be evaluated before being
>> passed in to the macro. Hence $i is invalid reference when it is
>> evaluated,
>> and we'd always see "no no no".
>>
>> WILL
>>
>> On Sun, Oct 12, 2008 at 5:17 PM, Byron Foster <by...@base2.cc> wrote:
>>
>>> I went back and pulled the velocity source to the revision (471881)
>>> before
>>> the changes to bug 285 and found that the behavior is as expected, so I
>>> conclude this is a bug :) which has been created here:
>>>
>>> https://issues.apache.org/jira/browse/VELOCITY-630
>>>
>>>
>>> On Oct 10, 2008, at 17:04 , Byron Foster wrote:
>>>
>>> Given the following:
>>>>
>>>> #macro(test $a $b)
>>>> #foreach($i in $a) $b #end
>>>> #end
>>>> #test( [1, 2, 3] "#if($i == 2) yes #else no #end")
>>>>
>>>> I would expect the output:
>>>>
>>>> no yes no
>>>>
>>>> Instead, I get
>>>>
>>>> no no no
>>>>
>>>> However, if I have
>>>>
>>>> #macro(test $a $b)
>>>> #set($i = 1) $b #set($i = 2) $b #set($i = 3) $b
>>>> #end
>>>> #test( [1, 2, 3] "#if($i == 2) yes #else no #end")
>>>>
>>>> then I get:
>>>> no yes no
>>>> as expected.
>>>>
>>>> I would say there is a bug here. The Foreach directive explicitly sets
>>>> $i
>>>> in the localscope. If I replace the explicit put to the localscope with
>>>> a
>>>> regular put, the first macro case works. I was looking at the following
>>>> issues where the modification to Foreach was mode.
>>>>
>>>> https://issues.apache.org/jira/browse/VELOCITY-285
>>>>
>>>> Now, my test change to Foreach breaks issue 285, but I'm not convinced
>>>> that the resolution to 285 is complete. Thoughts?
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
> For additional commands, e-mail: dev-help@velocity.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org
Re: Strange pass by name behavior
Posted by Byron Foster <by...@base2.cc>.
From my perusal the way the macro implementation works is that all
parameters passed to a macros are AST, even if they are in quotes.
This is consistent with pass by name. And anyway, if the string was
evaluated before being passed then that doesn't explain the behavior
of the second macro where $i is modified with #set directives instead
of #foreach. Shouldn't the results be the same?
I could simplify the call with no quotes:
#test( [1, 2, 3] #if($i=2)yes#end)
Which with the first macro returns nothing and with the second macro
returns "yes"
But, even without using directives (Which is probably what I should
have done):
#macro(test $x)#foreach($i in [1, 2])$x#end#end
#test($i)
returns: $i$i
#macro(test $x)#set($i = 1)$x #set($i = 2)$x#end
#test($i)
returns: 1 2
On a side note... the following generates a parse error.. Is this
correct?
#macro(test $x)#set($i = 1)$x#set($i = 2)$x#end
Encountered "=" at line 6, column 38 of /foo.vm
Was expecting:
<RPAREN> ...
On Oct 12, 2008, at 19:08 , Will Glass-Husain wrote:
> I would have assumed the quoted string would be evaluated before being
> passed in to the macro. Hence $i is invalid reference when it is
> evaluated,
> and we'd always see "no no no".
>
> WILL
>
> On Sun, Oct 12, 2008 at 5:17 PM, Byron Foster <by...@base2.cc> wrote:
>
>> I went back and pulled the velocity source to the revision (471881)
>> before
>> the changes to bug 285 and found that the behavior is as expected,
>> so I
>> conclude this is a bug :) which has been created here:
>>
>> https://issues.apache.org/jira/browse/VELOCITY-630
>>
>>
>> On Oct 10, 2008, at 17:04 , Byron Foster wrote:
>>
>> Given the following:
>>>
>>> #macro(test $a $b)
>>> #foreach($i in $a) $b #end
>>> #end
>>> #test( [1, 2, 3] "#if($i == 2) yes #else no #end")
>>>
>>> I would expect the output:
>>>
>>> no yes no
>>>
>>> Instead, I get
>>>
>>> no no no
>>>
>>> However, if I have
>>>
>>> #macro(test $a $b)
>>> #set($i = 1) $b #set($i = 2) $b #set($i = 3) $b
>>> #end
>>> #test( [1, 2, 3] "#if($i == 2) yes #else no #end")
>>>
>>> then I get:
>>> no yes no
>>> as expected.
>>>
>>> I would say there is a bug here. The Foreach directive explicitly
>>> sets $i
>>> in the localscope. If I replace the explicit put to the
>>> localscope with a
>>> regular put, the first macro case works. I was looking at the
>>> following
>>> issues where the modification to Foreach was mode.
>>>
>>> https://issues.apache.org/jira/browse/VELOCITY-285
>>>
>>> Now, my test change to Foreach breaks issue 285, but I'm not
>>> convinced
>>> that the resolution to 285 is complete. Thoughts?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org
Re: Strange pass by name behavior
Posted by Will Glass-Husain <wg...@gmail.com>.
I would have assumed the quoted string would be evaluated before being
passed in to the macro. Hence $i is invalid reference when it is evaluated,
and we'd always see "no no no".
WILL
On Sun, Oct 12, 2008 at 5:17 PM, Byron Foster <by...@base2.cc> wrote:
> I went back and pulled the velocity source to the revision (471881) before
> the changes to bug 285 and found that the behavior is as expected, so I
> conclude this is a bug :) which has been created here:
>
> https://issues.apache.org/jira/browse/VELOCITY-630
>
>
> On Oct 10, 2008, at 17:04 , Byron Foster wrote:
>
> Given the following:
>>
>> #macro(test $a $b)
>> #foreach($i in $a) $b #end
>> #end
>> #test( [1, 2, 3] "#if($i == 2) yes #else no #end")
>>
>> I would expect the output:
>>
>> no yes no
>>
>> Instead, I get
>>
>> no no no
>>
>> However, if I have
>>
>> #macro(test $a $b)
>> #set($i = 1) $b #set($i = 2) $b #set($i = 3) $b
>> #end
>> #test( [1, 2, 3] "#if($i == 2) yes #else no #end")
>>
>> then I get:
>> no yes no
>> as expected.
>>
>> I would say there is a bug here. The Foreach directive explicitly sets $i
>> in the localscope. If I replace the explicit put to the localscope with a
>> regular put, the first macro case works. I was looking at the following
>> issues where the modification to Foreach was mode.
>>
>> https://issues.apache.org/jira/browse/VELOCITY-285
>>
>> Now, my test change to Foreach breaks issue 285, but I'm not convinced
>> that the resolution to 285 is complete. Thoughts?
>>
>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
>> For additional commands, e-mail: dev-help@velocity.apache.org
>>
>>
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
> For additional commands, e-mail: dev-help@velocity.apache.org
>
>
--
Forio Business Simulations
Will Glass-Husain
wglass@forio.com
www.forio.com
Re: Strange pass by name behavior
Posted by Byron Foster <by...@base2.cc>.
I went back and pulled the velocity source to the revision (471881)
before the changes to bug 285 and found that the behavior is as
expected, so I conclude this is a bug :) which has been created here:
https://issues.apache.org/jira/browse/VELOCITY-630
On Oct 10, 2008, at 17:04 , Byron Foster wrote:
> Given the following:
>
> #macro(test $a $b)
> #foreach($i in $a) $b #end
> #end
> #test( [1, 2, 3] "#if($i == 2) yes #else no #end")
>
> I would expect the output:
>
> no yes no
>
> Instead, I get
>
> no no no
>
> However, if I have
>
> #macro(test $a $b)
> #set($i = 1) $b #set($i = 2) $b #set($i = 3) $b
> #end
> #test( [1, 2, 3] "#if($i == 2) yes #else no #end")
>
> then I get:
> no yes no
> as expected.
>
> I would say there is a bug here. The Foreach directive explicitly
> sets $i in the localscope. If I replace the explicit put to the
> localscope with a regular put, the first macro case works. I was
> looking at the following issues where the modification to Foreach
> was mode.
>
> https://issues.apache.org/jira/browse/VELOCITY-285
>
> Now, my test change to Foreach breaks issue 285, but I'm not
> convinced that the resolution to 285 is complete. Thoughts?
>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
> For additional commands, e-mail: dev-help@velocity.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org