problemm with inner types
Sebastian Gurin
The problem is with inner type definition. Take for example the following java class:
package org.sgx.j2s.base.test.java;
public class Bug1 {
public static void main(String[] args) {
for(int k=1; k<6; k++) {
final int i=k;
Object obj = new Object(){
private final int attr1 = i; <---here it is the problem
public String toString() {return "object"+attr1;};
};
System.out.println(obj.toString());
}
}
}
it is currently translated to :
Clazz.declarePackage ("org.sgx.j2s.base.test.java");
c$ = Clazz.declareType (org.sgx.j2s.base.test.java, "Bug1");
c$.main = Clazz.defineMethod (c$, "main",
function (args) {
for (var k = 1; k < 6; k++) {
var i = k;
var obj = ((Clazz.isClassDefined ("org.sgx.j2s.base.test.java.Bug1$1") ? 0 : org.sgx.j2s.base.test.java.Bug1.$Bug1$1$ ()), Clazz.innerTypeInstance (org.sgx.j2s.base.test.java.Bug1$1, this, null));
System.out.println (obj.toString ());
}
}, "~A");
c$.$Bug1$1$ = function () {
Clazz.pu$h ();
c$ = Clazz.decorateAsClass (function () {
Clazz.prepareCallback (this, arguments);
this.attr1 = 0;
Clazz.instantialize (this, arguments);
}, org.sgx.j2s.base.test.java, "Bug1$1");
Clazz.prepareFields (c$, function () {
this.attr1 = i; // <-------- THIS IS WRONG!!!
});
Clazz.defineMethod (c$, "toString",
function () {
return "object" + this.attr1;
});
c$ = Clazz.p0p ();
};
As you can see, the inner type is declared at the bottom, outside the block that contains the declaration. So, the this.attr1 = i; statement is incorrect because there the i variable cannot be accessed (because we are outside the containing block).
If I fix the javascript generated code, putting the inner type definition INSIDE the for{} block, then it works fine:
Clazz.declarePackage ("org.sgx.j2s.base.test.java");
c$ = Clazz.declareType (org.sgx.j2s.base.test.java, "Bug1");
c$.main = Clazz.defineMethod (c$, "main",
function (args) {
for (var k = 1; k < 6; k++) {
var i = k;
//class definition moved here
c$.$Bug1$1$ = function () {
Clazz.pu$h ();
c$ = Clazz.decorateAsClass (function () {
Clazz.prepareCallback (this, arguments);
this.attr1 = 0;
Clazz.instantialize (this, arguments);
}, org.sgx.j2s.base.test.java, "Bug1$1");
Clazz.prepareFields (c$, function () {
this.attr1 = i; <----- THIS IS OK
});
Clazz.defineMethod (c$, "toString",
function () {
return "object" + this.attr1;
});
c$ = Clazz.p0p ();
};
//and the rest
var obj = ((Clazz.isClassDefined ("org.sgx.j2s.base.test.java.Bug1$1") ? 0 : org.sgx.j2s.base.test.java.Bug1.$Bug1$1$ ()),
Clazz.innerTypeInstance (org.sgx.j2s.base.test.java.Bug1$1, this, null));
System.out.println (obj.toString ());
}
}, "~A");
I'm planning to introduce this fix, but I have a lot of doubts about the correctness of this sollution (putting the inner type definition inside the original statement block). I would appreciate any ideas, and suggestions about how to do it properly.
Regards,
--
Sebastian Gurin <sgu...@softpoint.org>
Sebastian Gurin
....
final int gr = Integer.parseInt(new String(
repl, ++index, 1));
....
replacementParts[replacementParts.length] = new Object() { //$NON-LOCK-1$
private final int grN = gr;
public String toString() {
return group(grN);
}
};
If I change that to the following java it will work fine:
final int gr = Integer.parseInt(new String(
repl, ++index, 1));
....
class A {
int grN;
public A(int grN) {
this.grN=grN;
}
public String toString() {
return group(grN);
}
}
replacementParts[replacementParts.length] = new A(gr);
with that fix in Matcher.java, now examples using Matcher will work, like for example, replacing captured regex groups in strings:
String patternStr = "\\((\\w+)\\)";
String replaceStr = "<$1>";
Pattern pattern = Pattern.compile(patternStr);
CharSequence inputStr = "a (b c) d (ef) g";
Matcher matcher = pattern.matcher(inputStr);
String output = matcher.replaceAll(replaceStr);
System.out.println(output); <-- "a (b c) d <ef> g"
Nevertheless, I won't commit that fix or others like it, because, IMHO, the cause should be corrected instead of its consecuences.
Regards,
> --
> You received this message because you are subscribed to the Google Groups "Java2Script" group.
> To post to this group, send email to java2...@googlegroups.com.
> To unsubscribe from this group, send email to java2script...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/java2script?hl=en.
>
--
Sebastian Gurin <sgu...@softpoint.org>
Zhou Renjian
for(int k=1; k<6; k++) {
final int i=k;
Object obj = new Object(){
};
System.out.println(obj.toString());
}
c$.main = Clazz.defineMethod (c$, "main",
for (var b = 1; b < 6; b++) {
var c = b;
var d = ((Clazz.isClassDefined ("net.sf.j2s.test.Test$1") ? 0 : net.sf.j2s.test.Test.$Test$1$ ()), Clazz.innerTypeInstance (net.sf.j2s.test.Test$1, this, Clazz.cloneFinals ("c", c)));
System.out.println (d.toString ());
}
}, "~A");
c$.$Test$1$ = function () {
Clazz.pu$h ();
c$ = Clazz.declareAnonymous (net.sf.j2s.test, "Test$1");
Clazz.defineMethod (c$, "toString",
function () {
});
c$ = Clazz.p0p ();
};
Regards,
Zhou Renjian
--
http://webuzz.im/ web instant messengers (Gtalk/MSN/Yahoo!/Facebook/AIM/ICQ/Jabber)
http://j2s.sourceforge.net/ Java2Script
Sebastian Gurin
Doing some experimentation I think I understand the compiler behaviour here: if the anon class reference in their body outer final variables, then the anon class is instantiated passing the final variable values (using Clazz.cloneFinals ("i", i))
The bug is, as you said, is with anon class attribute initialization with a final variable value (private int attr1=finalVariable;). In the anon class instantiation, the final variable values are not passed. and IN THE BODY of the anon func, (only) attributes are not well initializated.
So I think the bug is in the logic that decides if an anon class references final variables. When searching for final variable references in the anon class body, it should also count attribute initialization.
So the bug only will affect code with anon classes that references final variables ONLY in its attribute initializations. It is less serious that I thought...
Well, I'm trying to find out where that logic is and try to solve it anyway. Any help is appreciated
Regards
Sebastian Gurin
when translating the relevant code for this bug, in ASTScriptVisitor tha visiting path is (debugging visit() methods of ASTScriptVisitor):
CassInstanceCreation :
AnonimousClassDeclaration
FieldDeclaration
SimpleName (the i final variable referenced in bug statement this.attr=i)
So as I understand, visit(SimpleName) is the place where we must "count" a variable reference as a final variable. This is in fact done in method simpleNameInVarBinding, about line 2106. There, if I change the condition for also counting final variable initialized fields, like the following:
boolean isFieldDeclWithFinalInit = parent != null && /* added by me so initialized fields with final vars are also added */
parent instanceof VariableDeclarationFragment &&
parent.getParent() instanceof FieldDeclaration;
if (methodDeclareStack.size() == 0 || !key.equals(methodDeclareStack.peek()) || /*sgurin*/ isFieldDeclWithFinalInit) {
buffer.append("this.$finals.");
if (currentBlockForVisit != -1) {
List finalVars = ((ASTVariableVisitor) getAdaptable(ASTVariableVisitor.class)).finalVars;
List visitedVars = ((ASTVariableVisitor) getAdaptable(ASTVariableVisitor.class)).visitedVars;
int size = finalVars.size();
for (int i = 0; i < size; i++) {
ASTFinalVariable vv = (ASTFinalVariable) finalVars.get(size - i - 1);
if (vv.variableName.equals(varBinding.getName())
&& vv.blockLevel <= currentBlockForVisit) {
if (!visitedVars.contains(vv)) {
visitedVars.add(vv);
}
fieldVar = vv.toVariableName;
}
}
}
}
Well, with that small fix, the code is correctly translated, I mean my buggy java code :
public class J2SBug4 {
public static void main(String[] args) {
for (int k = 1; k < 6; k++) {
final int i = k;
Object obj = new Object() {
private int attr1 = i;
public String toString() {
return "object" + attr1;
};
};
System.out.println(obj.toString());
}
}
}
is now translated to the followign javascript:
Clazz.declarePackage ("sdf");
c$ = Clazz.declareType (sdf, "J2SBug4");
c$.main = Clazz.defineMethod (c$, "main",
function (args) {
for (var k = 1; k < 6; k++) {
var i = k;
var obj = ((Clazz.isClassDefined ("sdf.J2SBug4$1") ? 0 : sdf.J2SBug4.$J2SBug4$1$ ()), Clazz.innerTypeInstance (sdf.J2SBug4$1, this, Clazz.cloneFinals ("i", i)));
System.out.println (obj.toString ());
}
}, "~A");
c$.$J2SBug4$1$ = function () {
Clazz.pu$h ();
c$ = Clazz.decorateAsClass (function () {
Clazz.prepareCallback (this, arguments);
this.attr1 = 0;
Clazz.instantialize (this, arguments);
}, sdf, "J2SBug4$1");
Clazz.prepareFields (c$, function () {
this.attr1 = this.f$.i;
});
Clazz.defineMethod (c$, "toString",
function () {
return "object" + this.attr1;
});
c$ = Clazz.p0p ();
};
Notice that now Clazz.cloneFinals is correctly passed in translated anon class instantiation, and in the anon class declaration, field is correctly initialized: this.attr1 = this.f$.i;
Unfortunately in that line, the javascript error occurs: "this.f$ is undefined". I think Clazz.prepareFields is called before this.f$ property is setted. This is done in Clazz.innerTypeInstance (ClassExt.js). There the object is first instantiated and THEN this.finals$ is setted so the instantiation fails.
So, now I'm trying to study what is the best way of fixing the class loader for this case. Any help is very appreciated!
Regards.
On Fri, 13 May 2011 07:37:08 +0800
Zhou Renjian <zhour...@gmail.com> wrote:
Zhou Renjian
Ya, I think it is a correct way to fix this bug.
Regards,
Zhou Renjian
--
http://webuzz.im/ web instant messengers (Gtalk/MSN/Yahoo!/Facebook/AIM/ICQ/Jabber)
http://j2s.sourceforge.net/ Java2Script
Sebastian Gurin
Renjian: i will test this a little more, but, do you think it is ok to commit this?
patch for /net.sf.j2s.core/src/net/sf/j2s/core/astvisitors/ASTScriptVisitor.java
### Eclipse Workspace Patch 1.0
#P net.sf.j2s.core
Index: src/net/sf/j2s/core/astvisitors/ASTScriptVisitor.java
===================================================================
--- src/net/sf/j2s/core/astvisitors/ASTScriptVisitor.java (revisión: 1175)
+++ src/net/sf/j2s/core/astvisitors/ASTScriptVisitor.java (copia de trabajo)
@@ -2042,7 +2042,12 @@
&& (varBinding.getModifiers() & Modifier.FINAL) != 0
&& varBinding.getDeclaringMethod() != null) {
String key = varBinding.getDeclaringMethod().getKey();
- if (methodDeclareStack.size() == 0 || !key.equals(methodDeclareStack.peek())) {
+
+ boolean isFieldDeclWithFinalInit = parent != null && /* sgurin: fix for anon class attrs init with final vars */
+ parent instanceof VariableDeclarationFragment &&
+ parent.getParent() instanceof FieldDeclaration;
+
+ if (methodDeclareStack.size() == 0 || !key.equals(methodDeclareStack.peek()) || isFieldDeclWithFinalInit) {
buffer.append("this.$finals.");
if (currentBlockForVisit != -1) {
List finalVars = ((ASTVariableVisitor) getAdaptable(ASTVariableVisitor.class)).finalVars;
The patch for classExt.js:
### Eclipse Workspace Patch 1.0
#P net.sf.j2s.java.core
Index: src/java/lang/ClassExt.js
===================================================================
--- src/java/lang/ClassExt.js (revisión: 1175)
+++ src/java/lang/ClassExt.js (copia de trabajo)
@@ -124,6 +124,12 @@
clazzInner = arguments.callee.caller;
}
var obj = null;
+
+ /* sgurin: fix for anon class attrs init with final vars */
+ if(finalVars!=null) {
+ clazzInner.prototype.f$ = finalVars;
+ }
+
/*if (arguments.length == 2) {
obj = new clazzInner (objThis);
} else */if (arguments.length == 3) {
On Wed, 18 May 2011 10:35:53 -0300
Sebastian Gurin <sgu...@softpoint.org> wrote:
> Unfortunately in that line, the javascript error occurs: "this.f$ is undefined". I think Clazz.prepareFields is called before this.f$ property is setted. This is done in Clazz.innerTypeInstance (ClassExt.js). There the object is first instantiated and THEN this.finals$ is setted so the instantiation fails.
>
> So, now I'm trying to study what is the best way of fixing the class loader for this case. Any help is very appreciated!
--
Sebastian Gurin <sgu...@softpoint.org>
Zhou Renjian
clazzInner.prototype.f$ = finalVars;
is incorrect.
different anonymous objects should have different final variables.
Regards,
Zhou Renjian
--
http://webuzz.im/ web instant messengers (Gtalk/MSN/Yahoo!/Facebook/AIM/ICQ/Jabber)
http://j2s.sourceforge.net/ Java2Script
Sebastian Gurin
Zhou Renjian <zhour...@gmail.com> wrote:
> I think
> clazzInner.prototype.f$ = finalVars;
> is incorrect.
>
> different anonymous objects should have different final variables.
yes, I thought about that too. I will try to figure out other way for fixing this. Regards
--
Sebastian Gurin <sgu...@softpoint.org>
Sebastian Gurin
patch for /net.sf.j2s.core/src/net/sf/j2s/core/astvisitors/ASTScriptVisitor.java
### Eclipse Workspace Patch 1.0
#P net.sf.j2s.core
Index: src/net/sf/j2s/core/astvisitors/ASTScriptVisitor.java
===================================================================
--- src/net/sf/j2s/core/astvisitors/ASTScriptVisitor.java (revisión: 1175)
+++ src/net/sf/j2s/core/astvisitors/ASTScriptVisitor.java (copia de trabajo)
@@ -218,6 +218,10 @@
typeVisitor.setClassName(shortClassName);
// buffer.append(" = function () {\r\n");
buffer.append("function () {\r\n");
+
+ //sgurin: fix for anon class attrs init with final vars
+ buffer.append("if(arguments[0].f$!=null){this.f$=arguments[0].f$;}\r\n");
+
if (!(node.getParent() instanceof EnumConstantDeclaration)) {
buffer.append("Clazz.prepareCallback (this, arguments);\r\n");
}
@@ -2042,7 +2046,12 @@
&& (varBinding.getModifiers() & Modifier.FINAL) != 0
&& varBinding.getDeclaringMethod() != null) {
String key = varBinding.getDeclaringMethod().getKey();
- if (methodDeclareStack.size() == 0 || !key.equals(methodDeclareStack.peek())) {
+
+ boolean isFieldDeclWithFinalInit = parent != null && /* sgurin: fix for anon class attrs init with final vars */
+ parent instanceof VariableDeclarationFragment &&
+ parent.getParent() instanceof FieldDeclaration;
+
+ if (methodDeclareStack.size() == 0 || !key.equals(methodDeclareStack.peek()) || isFieldDeclWithFinalInit) {
buffer.append("this.$finals.");
if (currentBlockForVisit != -1) {
List finalVars = ((ASTVariableVisitor) getAdaptable(ASTVariableVisitor.class)).finalVars;
patch for ClassExt.js:
### Eclipse Workspace Patch 1.0
#P net.sf.j2s.java.core
Index: src/java/lang/ClassExt.js
===================================================================
--- src/java/lang/ClassExt.js (revisión: 1175)
+++ src/java/lang/ClassExt.js (copia de trabajo)
@@ -124,6 +124,12 @@
clazzInner = arguments.callee.caller;
}
var obj = null;
+
+ /* sgurin: fix for anon class attrs init with final vars */
+ if(finalVars!=null && objThis!=null) {
+ objThis.f$ = finalVars;
+ }
+
/*if (arguments.length == 2) {
obj = new clazzInner (objThis);
} else */if (arguments.length == 3) {
note that I added the finals initialization in the generated js code:
c$.$J2SBug1$1$ = function () {
Clazz.pu$h ();
c$ = Clazz.decorateAsClass (function () {
if(arguments[0].f$!=null){this.f$=arguments[0].f$;}
Clazz.prepareCallback (this, arguments);
this.attr1 = 0;
Clazz.instantialize (this, arguments);
}, sdf, "J2SBug1$1");
Clazz.prepareFields (c$, function () {
this.attr1 = this.f$.i;
});
...
--
Sebastian Gurin <sgu...@softpoint.org>
Zhou Renjian
if(arguments[0].f$!=null){this.f$=arguments[0].f$;}
Try to do it in method Clazz.instantialize may be a better way, IMO
Regards,
Zhou Renjian
--
http://webuzz.im/ web instant messengers (Gtalk/MSN/Yahoo!/Facebook/AIM/ICQ/Jabber)
http://j2s.sourceforge.net/ Java2Script
Sebastian Gurin
### Eclipse Workspace Patch 1.0
#P net.sf.j2s.core
Index: src/net/sf/j2s/core/astvisitors/ASTScriptVisitor.java
===================================================================
--- src/net/sf/j2s/core/astvisitors/ASTScriptVisitor.java (revisión: 1175)
+++ src/net/sf/j2s/core/astvisitors/ASTScriptVisitor.java (copia de trabajo)
@@ -2042,7 +2042,12 @@
&& (varBinding.getModifiers() & Modifier.FINAL) != 0
&& varBinding.getDeclaringMethod() != null) {
String key = varBinding.getDeclaringMethod().getKey();
- if (methodDeclareStack.size() == 0 || !key.equals(methodDeclareStack.peek())) {
+
+ boolean isFieldDeclWithFinalInit = parent != null && /* sgurin: fix for anon class attrs init with final vars */
+ parent instanceof VariableDeclarationFragment &&
+ parent.getParent() instanceof FieldDeclaration;
+
+ if (methodDeclareStack.size() == 0 || !key.equals(methodDeclareStack.peek()) || isFieldDeclWithFinalInit) {
buffer.append("this.$finals.");
if (currentBlockForVisit != -1) {
List finalVars = ((ASTVariableVisitor) getAdaptable(ASTVariableVisitor.class)).finalVars;
and
### Eclipse Workspace Patch 1.0
#P net.sf.j2s.java.core
Index: src/java/lang/ClassExt.js
===================================================================
--- src/java/lang/ClassExt.js (revisión: 1175)
+++ src/java/lang/ClassExt.js (copia de trabajo)
@@ -60,6 +60,9 @@
/* protected */
Clazz.prepareCallback = function (objThis, args) {
var classThisObj = args[0];
+ if(classThisObj.f$ != null){/* sgurin: fix for anon class attrs init with final vars */
+ objThis.f$ = classThisObj.f$;
+ }
var cbName = "b$"; // "callbacks";
if (objThis != null && classThisObj != null && classThisObj !== window) {
var obs = new Array ();
@@ -124,6 +127,12 @@
clazzInner = arguments.callee.caller;
}
var obj = null;
+ if(finalVars!=null && objThis!=null) {/* sgurin: fix for anon class attrs init with final vars */
+ objThis.f$ = finalVars;
+ }
/*if (arguments.length == 2) {
obj = new clazzInner (objThis);
} else */if (arguments.length == 3) {
I plan to test this modifications more, compiling java.core and other complex projects and comparing the results of running java.lang and java.util mauve tests for making sure the change has not undesired second effects. Then I plan to commit it to 3.6 branch.
Regards.