Possible bug?

Hi,

I found this odd behavior on resin 2.1.6 regarding custom tags, which I 
consider might be a bug due to too aggressive optimization. 

Suppose I have a custom tag, and in my custom tag I have a set method to set an 
integer, and the getHtml() method simply displays that integer: 

public class MyTag extends TagSupport {
        ...
    public void setFoo(String s) {
        foo_ = (Integer) ((HttpServletRequest) 
pageContext.getRequest()).getAttribute(s); 
        // notice that foo_ is set by reading an Integer from the request 
object 
    }
        ...
}       

And in my jsp, I am using the tag in the following way:

<!-- jsp listing 1 -->
<% for (int i = 0; i < 10; ++i) {
        request.setAttribute("bar", new Integer(i));// notice that I am setting 
"bar" in requests object every time.
%>
        <mytag foo="bar" /> <!-- Notice the literal string "bar" -->
<% } %>

You would think it will display 0, 1, 2, ... 9, but actually it will only show 
0 for 10 times, if you change the jsp code slightly, 

<!-- jsp listing 2 -->
<% String bar = "bar";
   for (int i = 0; i < 10; ++i) {
        request.setAttribute("bar", new Integer(i));// notice that I am setting 
"bar" in requests object every time.
%>
        <mytag foo="<%=bar%>" /> <!-- Notice the use of variable (String) bar 
-->
<% } %>

although the only change is to use a variable "String bar" to replace the 
literal string, this time it produces the expected result. 

A closer look at resin compiled java code shows that it's a result of 
(overly-aggressive?) optimization:

The jsp listing 1 would compile to something like:

for (int i = 0; i < 10; ++i) {
        request.setAttribute("bar", new Integer(i));
        if (_jsp_tag1 == null) {
                _jsp_tag1 = new MyTag();
                _jsp_tag1.setPageContext(pageContext);
                _jsp_tag1.setParent((javax.servlet.jsp.tagext.Tag) null);
                _jsp_tag.setFoo("bar"); //Notice that the setFoo method is 
called only once in the loop
                
        }
        ...
}       

The jsp listing 2 would compile to something like:

String bar = "bar";
for (int i = 0; i < 10; ++i) {  
        request.setAttribute("bar", new Integer(i));
        if (_jsp_tag1 == null) {
                _jsp_tag1 = new MyTag();
                _jsp_tag1.setPageContext(pageContext);
                _jsp_tag1.setParent((javax.servlet.jsp.tagext.Tag) null);       
        
        }
      _jsp_tag.setFoo(bar); //Notice that the setFoo method now is called for 
every iteration
        ...
}       

So is it a bug? IMO, it looks like one. Resin is trying to optimize the code 
(listing 1) by thinking "hmm, if the setFoo method is called on a literal 
("bar"), then the result must be the same all the time, therefore I can move it 
into the block that creates the tag".  In listing 2, resin sees a String, 
therefore moves setFoo(bar) outside the tag creation block. But as the code 
shows, setFoo("bar") may be dynamic even though it is called on a literal 
string, and probably most people would think listing 1 and listing 2 shouldn't 
yield different results. If that is indeed a bug, then a possible fix may be 
just as simple as "don't move set methods into tag creation block". (?)

For me, currently I have a workaround (have to), which is to scan our code and 
move any logic in the set method that set anything based on request context 
into a separate function, and that function gets called in the very beginning 
of getHtml().

Comments?

--jake