[#JANINO-117] UnparseVisitor does not handle Byte or Short Literals - jira.codehaus.org

Details

Type: Bug
Status: Closed
Priority: Major
Resolution: Fixed
Affects Version/s: None
Fix Version/s: 2.5.15
Component/s: None
Labels:
None

Testcase included:
yes
Patch Submitted:

Yes
Number of attachments :
1

Description

The following test will fail with a RuntimeException. While parsed java cannot create a literal Byte or Short value, it is fairly easy to create them directly from the AST, and handling them is quite simple.

public void testLiterals() throws Exception {
        Object[][] tests = new Object[][] {
                { new Java.Literal(null, new Short((short)1)), "((short)1)" },
                { new Java.Literal(null, new Byte((byte)1)),   "((byte)1)"  },
        };
        for(int i = 0; i < tests.length; ++i) {
            Atom expr = (Atom) tests[i][0];
            String expected = (String) tests[i][1];
            
            StringWriter sw = new StringWriter();
            UnparseVisitor uv = new UnparseVisitor(sw);
            expr.accept(uv);
            Assert.assertEquals(expected, sw.toString());
        }
    }

More Options
- Sort By Name
- Sort By Date
- Ascending
- Descending
- Download All

Attachments

literals.diff

28/May/08 10:26 AM

2 kB

Matt Fowles

Activity

Ascending order - Click to sort in descending order

Hide

Permalink

Arno Unkrig added a comment - 14/Jun/08 3:33 PM

Applied the patch.

Please test.

Show

Arno Unkrig added a comment - 14/Jun/08 3:33 PM Applied the patch. Please test.

Hide

Permalink

Matt Fowles added a comment - 16/Jun/08 9:28 AM

Arno, I do not have the ability to close this issue. I think my login to Jira does not have the power. You can consider this closed.

Show

Matt Fowles added a comment - 16/Jun/08 9:28 AM Arno, I do not have the ability to close this issue. I think my login to Jira does not have the power. You can consider this closed.

Hide

Permalink

Arno Unkrig added a comment - 18/Jun/08 2:40 PM

I see. This is not compatible with my understanding of the JIRA standard workflow.

Show

Arno Unkrig added a comment - 18/Jun/08 2:40 PM I see. This is not compatible with my understanding of the JIRA standard workflow.

Hide

Permalink

Arno Unkrig added a comment - 18/Jun/08 2:41 PM

However, I have bad news for you: I removed your fix. The reason being is that in Java there is no such thing as a "byte literal" or "short literal". "new Literal(new Byte(8))" worked only accidentially!

But Java's conversion rules (JLS2 5.2) imply that you can legally assign an integer literal to, say, a byte variable, if the constant value of the integer literal is in range:

byte a = 3;
byte b = -128;

I added an argument type check to "Literal(Object)" which throws IllegalArgumentExceptions.

Show

Arno Unkrig added a comment - 18/Jun/08 2:41 PM However, I have bad news for you: I removed your fix. The reason being is that in Java there is no such thing as a "byte literal" or "short literal". "new Literal(new Byte(8))" worked only accidentially! But Java's conversion rules (JLS2 5.2) imply that you can legally assign an integer literal to, say, a byte variable, if the constant value of the integer literal is in range: byte a = 3; byte b = -128; I added an argument type check to "Literal(Object)" which throws IllegalArgumentExceptions.

Hide

Permalink

Matt Fowles added a comment - 18/Jun/08 2:50 PM

I would strongly urge you to reconsider this choice. While Java does not allow byte literals directly, making your AST support them increases the expressive power of your library, without exposing any bugs.

In fact, this ability greatly simplifies targeting Janino's AST from a different compiler, which is exactly what we are doing.

Show

Matt Fowles added a comment - 18/Jun/08 2:50 PM I would strongly urge you to reconsider this choice. While Java does not allow byte literals directly, making your AST support them increases the expressive power of your library, without exposing any bugs. In fact, this ability greatly simplifies targeting Janino's AST from a different compiler, which is exactly what we are doing.

Hide

Permalink

Matt Fowles added a comment - 05/Jun/09 3:10 PM

You can find a fixed version of this at http://code.google.com/p/janino-streambase/

Show

Matt Fowles added a comment - 05/Jun/09 3:10 PM You can find a fixed version of this at http://code.google.com/p/janino-streambase/

Hide

Permalink

Matt Fowles added a comment - 23/Dec/09 10:19 AM

Fixed in r378

Show

Matt Fowles added a comment - 23/Dec/09 10:19 AM Fixed in r378

People

Assignee:

Matt Fowles

Reporter:

Matt Fowles

Vote (0)

Watch (0)

Dates

Created:

28/May/08 10:26 AM

Updated:

23/Dec/09 10:19 AM

Resolved:

23/Dec/09 10:19 AM