Stephen Haberman has submitted this change and it was merged.
Change subject: Fix non-final field initializers running before the super
cstr.
......................................................................
Fix non-final field initializers running before the super cstr.
Previously, any field with an initializer would get assigned at
the top-level scope, before any cstrs had run.
However, this does not match the JVM behavior, which is that final fields
behave
this way, but non-field fields have their type's default value assigned when
super cstrs run, and then only later in their cstr are assigned to the
initializer.
Bug: issue 380
Change-Id: I4c8ed0cd718a2188b33cc290fec6071c89be7918
---
M dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
M user/test/com/google/gwt/dev/jjs/test/CompilerTest.java
A user/test/com/google/gwt/dev/jjs/test/FieldInitOrderBase.java
A user/test/com/google/gwt/dev/jjs/test/FieldInitOrderChild.java
4 files changed, 113 insertions(+), 10 deletions(-)
Approvals:
Roberto Lublinerman: Looks good to me, approved
Leeroy Jenkins: Verified
diff --git
a/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
b/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
index c0e1423..240f6a0 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
@@ -857,10 +857,13 @@
JsNameRef localRef = (JsNameRef) pop(); // localRef
JVariable target = x.getVariableRef().getTarget();
- if (target instanceof JField && ((JField)
target).getLiteralInitializer() != null) {
- // Will initialize at top scope; no need to double-initialize.
- push(null);
- return;
+ if (target instanceof JField) {
+ JField field = (JField) target;
+ if (field.getLiteralInitializer() != null && (field.isStatic() ||
field.isFinal())) {
+ // Will initialize at top scope; no need to double-initialize.
+ push(null);
+ return;
+ }
}
JsBinaryOperation binOp =
@@ -890,15 +893,15 @@
@Override
public void endVisit(JField x, Context ctx) {
// if we need an initial value, create an assignment
- if (x.getLiteralInitializer() != null) {
+ if (x.getLiteralInitializer() != null && (x.isFinal() ||
x.isStatic())) {
// setup the constant value
accept(x.getLiteralInitializer());
- } else if (!x.hasInitializer() && x.getEnclosingType() !=
program.getTypeJavaLangObject()) {
- // setup a default value
- accept(x.getType().getDefaultValue());
- } else {
- // the variable is setup during clinit, no need to initialize here
+ } else if (x.getEnclosingType() == program.getTypeJavaLangObject()) {
+ // Special fields whose initialization is done somewhere else.
push(null);
+ } else {
+ // setup the default value, see Issue 380
+ accept(x.getType().getDefaultValue());
}
JsExpression rhs = (JsExpression) pop();
JsName name = names.get(x);
diff --git a/user/test/com/google/gwt/dev/jjs/test/CompilerTest.java
b/user/test/com/google/gwt/dev/jjs/test/CompilerTest.java
index 4107144..812c6bb 100644
--- a/user/test/com/google/gwt/dev/jjs/test/CompilerTest.java
+++ b/user/test/com/google/gwt/dev/jjs/test/CompilerTest.java
@@ -787,6 +787,14 @@
assertEquals(1, x);
}
+ /** Ensure that only final fields are initializers when cstrs run, see
issue 380. */
+ public void testFieldInitializationOrder() {
+ ArrayList<String> seenValues = new ArrayList<String>();
+ new FieldInitOrderChild(seenValues);
+ assertEquals("i1=1,i2=0,i3=null,i4=null,i5=1,i6=1,i7=1",
seenValues.get(0));
+ assertEquals("i1=1,i2=1,i3=1,i4=2,i5=1,i6=2,i7=2", seenValues.get(1));
+ }
+
public void testForStatement() {
{
int i;
diff --git a/user/test/com/google/gwt/dev/jjs/test/FieldInitOrderBase.java
b/user/test/com/google/gwt/dev/jjs/test/FieldInitOrderBase.java
new file mode 100644
index 0000000..de1146b
--- /dev/null
+++ b/user/test/com/google/gwt/dev/jjs/test/FieldInitOrderBase.java
@@ -0,0 +1,33 @@
+/*
+ * Copyright 2013 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may
not
+ * use this file except in compliance with the License. You may obtain a
copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
under
+ * the License.
+ */
+package com.google.gwt.dev.jjs.test;
+
+import java.util.ArrayList;
+
+/**
+ * A superclass that invokes a method in its cstr, so that subclasses can
see their state before
+ * their own cstr has run.
+ *
+ * See {@link CompilerTest#testFieldInitializationOrder()}.
+ */
+class FieldInitOrderBase {
+ FieldInitOrderBase(ArrayList<String> seenValues, int x) {
+ method(seenValues, x);
+ }
+
+ void method(ArrayList<String> seenValues, int x) {
+ }
+}
\ No newline at end of file
diff --git a/user/test/com/google/gwt/dev/jjs/test/FieldInitOrderChild.java
b/user/test/com/google/gwt/dev/jjs/test/FieldInitOrderChild.java
new file mode 100644
index 0000000..dc00759
--- /dev/null
+++ b/user/test/com/google/gwt/dev/jjs/test/FieldInitOrderChild.java
@@ -0,0 +1,59 @@
+/*
+ * Copyright 2013 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may
not
+ * use this file except in compliance with the License. You may obtain a
copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
under
+ * the License.
+ */
+package com.google.gwt.dev.jjs.test;
+
+import java.util.ArrayList;
+
+/**
+ * A subclass that overrides {@link #method(ArrayList, int)} to see what
the values of its own
+ * fields are from within the superclass's cstr (before our own cstr has
run).
+ *
+ * See {@link CompilerTest#testFieldInitializationOrder()}.
+ */
+class FieldInitOrderChild extends FieldInitOrderBase {
+
+ private final int i1 = 1;
+ private int i2 = 1;
+ private Integer i3 = new Integer(1);
+ private Integer i4;
+ private final static int i5 = 1;
+ private static int i6 = 1;
+ private static Integer i7 = new Integer(1);
+
+ FieldInitOrderChild(ArrayList<String> seenValues) {
+ // the superclass calls method(), which will record the pre-cstr value
of our fields
+ super(seenValues, 2);
+ recordValues(seenValues);
+ }
+
+ // invoked by the super classes before our cstr has run
+ @Override
+ void method(ArrayList<String> seenValues, int x) {
+ recordValues(seenValues);
+ // i1 is final
+ i2 = x;
+ i3 = new Integer(x);
+ i4 = new Integer(x);
+ // i5 is final
+ i6 = 2;
+ i7 = new Integer(x);
+ }
+
+ private void recordValues(ArrayList<String> seenValues) {
+ seenValues.add("i1=" + i1 + ",i2=" + i2 + ",i3=" + i3 + ",i4=" + i4
+ ",i5=" + i5 + ",i6=" + i6
+ + ",i7=" + i7);
+ }
+}
\ No newline at end of file
--
To view, visit https://gwt-review.googlesource.com/3030
To unsubscribe, visit https://gwt-review.googlesource.com/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I4c8ed0cd718a2188b33cc290fec6071c89be7918
Gerrit-PatchSet: 8
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Stephen Haberman <[email protected]>
Gerrit-Reviewer: Leeroy Jenkins <[email protected]>
Gerrit-Reviewer: Matthew Dempsky <[email protected]>
Gerrit-Reviewer: Ray Cromwell <[email protected]>
Gerrit-Reviewer: Roberto Lublinerman <[email protected]>
Gerrit-Reviewer: Stephen Haberman <[email protected]>
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
---
You received this message because you are subscribed to the Google Groups "GWT Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
