[gwt-contrib] There seems to be some issue in a Features.xml of the GWT 2.2 eclipse plugin.
I posted my message in the GWT group first, but no responses. https://groups.google.com/forum/#!topic/google-web-toolkit/2gWDTKw1tcA The problem is because of a parsing error in this Feature.xml file, I can no longer disable/enable other features in my eclipse environment. It is really annoying because the Oracle Enterprise Pack is adding a lot of stuff that I never use (like Python or Spring support). so I want them out of the way, out of the memory of my limited dev box. David Nouls. -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Test for compile failure if attributes don't match. (issue1384803)
LGTM http://gwt-code-reviews.appspot.com/1384803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adds a cache (PersistenUnitCache) to store CompilationUnits (issue1375802)
I didn't respond to Scott's initial comments: > - My gut reaction is that it's odd to make it a command-line option. It's not > really a user-facing 'option' in the same way that draftCompile or > style=PRETTY > is, it's just an internal implementation detail. It would seem more like a > jvm > flag, and perhaps later just always on. Toby, Jason and I had planned to put all of these dev mode changes behind a -X command line flag, but I went ahead and took out the command line flag and made it a system property because it simplified the API (enabled boolean flag in static api) > - Same with having to encode the persistence dir into every single entry > point. > Throwing it into WEB-INF in dev mode seems weird, and there's no fundamental > reason dev mode and the compiler shouldn't share a unit cache anyway. How > about > just using / creating a well-known directory under the current directory? If > you fail to create the directory (permission problem) then you don't use the > disk. First, I want to let you know I put a lot of thought into where to put the cache files and after discussions with Toby, we came up with the WEB-INF solution. There is precedent for putting the cache type files in WEB-INF (app engine integration with GWT does this). Also, its a terrific place when you consider that with our GWT ant setup, the cache gets removed when you run 'ant clean'.I've updated the Compiler entry point to also use the war/WEB-INF dir so the cache can be shared with web mode. The problem with using the current working directory is that potentially we are going to splatter cache directories all over the disk, and no user is going to thank us for that. Also, where would we tell users to clean things up? How could they write a reliable clean target? The problem with a 'well known' place, like /tmp/gwt-unitCache is that in a shared environment, that cache is not just going to be shared between compiles (what we want), it is going to be shared between different users on the same machine, and I'm pretty sure we don't want that. -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] [google-web-toolkit] r9850 committed - Fix generated JS for getting the locale property value from the...
Revision: 9850
Author: gwt.mirror...@gmail.com
Date: Tue Mar 15 07:52:10 2011
Log: Fix generated JS for getting the locale property value from the
user agent.
Public review at: http://gwt-code-reviews.appspot.com/1382803/
Patch by: jat
Review by: jlabanca
http://code.google.com/p/google-web-toolkit/source/detail?r=9850
Modified:
/trunk/samples/showcase/src/com/google/gwt/sample/showcase/Showcase.gwt.xml
/trunk/user/src/com/google/gwt/i18n/linker/LocalePropertyProviderGenerator.java
===
---
/trunk/samples/showcase/src/com/google/gwt/sample/showcase/Showcase.gwt.xml
Mon Feb 28 08:19:11 2011
+++
/trunk/samples/showcase/src/com/google/gwt/sample/showcase/Showcase.gwt.xml
Tue Mar 15 07:52:10 2011
@@ -25,4 +25,5 @@
value="SHOWCASE_LOCALE"/>
+
===
---
/trunk/user/src/com/google/gwt/i18n/linker/LocalePropertyProviderGenerator.java
Tue Dec 21 05:40:19 2010
+++
/trunk/user/src/com/google/gwt/i18n/linker/LocalePropertyProviderGenerator.java
Tue Mar 15 07:52:10 2011
@@ -280,9 +280,15 @@
+ "navigator.browserLanguage : navigator.language;");
body.println("if (language) {");
body.indent();
-body.println("locale = language.replace(/-/g, \"_\");");
+body.println("var parts = language.split(/[-_]/);");
+body.println("if (parts.length > 1) {");
+body.indent();
+body.println("parts[1] = parts[1].toUpperCase();");
body.outdent();
-body.println();
+body.println("}");
+body.println("locale = parts.join(\"_\");");
+body.outdent();
+body.println("}");
}
/**
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fix issue 5807 on server side. Previously reviewed at 1370803. (issue1384802)
Add the base test class to RequestFactorySuite.
http://gwt-code-reviews.appspot.com/1384802/diff/4001/user/src/com/google/gwt/requestfactory/server/ServiceLayer.java
File user/src/com/google/gwt/requestfactory/server/ServiceLayer.java
(right):
http://gwt-code-reviews.appspot.com/1384802/diff/4001/user/src/com/google/gwt/requestfactory/server/ServiceLayer.java#newcode385
user/src/com/google/gwt/requestfactory/server/ServiceLayer.java:385: }
Fix whitespace diff.
http://gwt-code-reviews.appspot.com/1384802/diff/4001/user/test/com/google/gwt/requestfactory/server/ServiceInheritanceJreTest.java
File
user/test/com/google/gwt/requestfactory/server/ServiceInheritanceJreTest.java
(right):
http://gwt-code-reviews.appspot.com/1384802/diff/4001/user/test/com/google/gwt/requestfactory/server/ServiceInheritanceJreTest.java#newcode2
user/test/com/google/gwt/requestfactory/server/ServiceInheritanceJreTest.java:2:
* Copyright 2010 Google Inc.
Copyright date.
http://gwt-code-reviews.appspot.com/1384802/diff/4001/user/test/com/google/gwt/requestfactory/shared/ServiceInheritanceTest.java
File
user/test/com/google/gwt/requestfactory/shared/ServiceInheritanceTest.java
(right):
http://gwt-code-reviews.appspot.com/1384802/diff/4001/user/test/com/google/gwt/requestfactory/shared/ServiceInheritanceTest.java#newcode2
user/test/com/google/gwt/requestfactory/shared/ServiceInheritanceTest.java:2:
* Copyright 2010 Google Inc.
Date.
http://gwt-code-reviews.appspot.com/1384802/diff/4001/user/test/com/google/gwt/requestfactory/shared/ServiceInheritanceTest.java#newcode36
user/test/com/google/gwt/requestfactory/shared/ServiceInheritanceTest.java:36:
System.out.println(clazz);
Remove the println. Also, verify the input class is what you expect.
http://gwt-code-reviews.appspot.com/1384802/diff/4001/user/test/com/google/gwt/requestfactory/shared/ServiceInheritanceTest.java#newcode94
user/test/com/google/gwt/requestfactory/shared/ServiceInheritanceTest.java:94:
public void testInvokeMethodOnSubclass() {
Add a second test to ensure that the method in the base class can still
be invoked.
http://gwt-code-reviews.appspot.com/1384802/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] [google-web-toolkit] r9851 committed - Fix bug on IE where onload events don't fire for IFrames....
Revision: 9851
Author: p...@google.com
Date: Mon Mar 14 09:06:35 2011
Log: Fix bug on IE where onload events don't fire for IFrames.
This is a bugfix for
http://code.google.com/p/google-web-toolkit/issues/detail?id=1720
Review at http://gwt-code-reviews.appspot.com/1294801
http://code.google.com/p/google-web-toolkit/source/detail?r=9851
Added:
/trunk/user/test/com/google/gwt/dom/public-test/iframetest.html
Modified:
/trunk/user/src/com/google/gwt/user/client/impl/DOMImplTrident.java
/trunk/user/src/com/google/gwt/user/client/ui/Frame.java
/trunk/user/test/com/google/gwt/dom/DOMSuite.java
/trunk/user/test/com/google/gwt/dom/client/FrameTests.java
===
--- /dev/null
+++ /trunk/user/test/com/google/gwt/dom/public-test/iframetest.html Mon Mar
14 09:06:35 2011
@@ -0,0 +1,10 @@
+
+
+
+ IFRAME TEST
+
+
+
+IFRAME TEST
+
+
===
--- /trunk/user/src/com/google/gwt/user/client/impl/DOMImplTrident.java Tue
Mar 1 06:29:34 2011
+++ /trunk/user/src/com/google/gwt/user/client/impl/DOMImplTrident.java Mon
Mar 14 09:06:35 2011
@@ -30,6 +30,9 @@
@SuppressWarnings("unused")
private static JavaScriptObject callDispatchDblClickEvent;
+ @SuppressWarnings("unused")
+ private static JavaScriptObject callDispatchOnLoadEvent;
+
@SuppressWarnings("unused")
private static JavaScriptObject callDispatchUnhandledEvent;
@@ -168,15 +171,18 @@
$wnd['__gwt_dispatchEvent_' + moduleName] = dispatchEvent;
@com.google.gwt.user.client.impl.DOMImplTrident::callDispatchEvent =
new Function('w',
'return function() { w.__gwt_dispatchEvent_' + moduleName
+ '.call(this) }')($wnd);
-
+
$wnd['__gwt_dispatchDblClickEvent_' + moduleName] =
dispatchDblClickEvent;
@com.google.gwt.user.client.impl.DOMImplTrident::callDispatchDblClickEvent
= new Function('w',
'return function() { w.__gwt_dispatchDblClickEvent_' + moduleName
+ '.call(this)}')($wnd);
-
+
$wnd['__gwt_dispatchUnhandledEvent_' + moduleName] =
dispatchUnhandledEvent;
@com.google.gwt.user.client.impl.DOMImplTrident::callDispatchUnhandledEvent
= new Function('w',
'return function() { w.__gwt_dispatchUnhandledEvent_' + moduleName
+ '.call(this)}')($wnd);
+
@com.google.gwt.user.client.impl.DOMImplTrident::callDispatchOnLoadEvent =
new Function('w',
+ 'return function() { w.__gwt_dispatchUnhandledEvent_' + moduleName
+ '.call(w.event.srcElement)}')($wnd);
+
// We need to create these delegate functions to fix up the 'this'
context.
// Normally, 'this' is the firing element, but this is only true for
// 'onclick = ...' event handlers, not for handlers setup via
attachEvent().
@@ -268,8 +274,18 @@
@com.google.gwt.user.client.impl.DOMImplTrident::callDispatchEvent : null;
if (chMask & 0x04000) elem.onscroll = (bits & 0x04000) ?
@com.google.gwt.user.client.impl.DOMImplTrident::callDispatchEvent : null;
-if (chMask & 0x08000) elem.onload= (bits & 0x08000) ?
-
@com.google.gwt.user.client.impl.DOMImplTrident::callDispatchUnhandledEvent :
null;
+if (chMask & 0x08000) {
+ if (elem.nodeName == "IFRAME") {
+if (bits & 0x08000) {
+ elem.attachEvent('onload',
@com.google.gwt.user.client.impl.DOMImplTrident::callDispatchOnLoadEvent);
+} else {
+ elem.detachEvent('onload',
@com.google.gwt.user.client.impl.DOMImplTrident::callDispatchOnLoadEvent);
+}
+ } else {
+ elem.onload = (bits & 0x08000) ?
+
@com.google.gwt.user.client.impl.DOMImplTrident::callDispatchUnhandledEvent :
null;
+ }
+}
if (chMask & 0x1) elem.onerror = (bits & 0x1) ?
@com.google.gwt.user.client.impl.DOMImplTrident::callDispatchEvent : null;
if (chMask & 0x2) elem.onmousewheel = (bits & 0x2) ?
===
--- /trunk/user/src/com/google/gwt/user/client/ui/Frame.java Tue Mar 1
06:29:34 2011
+++ /trunk/user/src/com/google/gwt/user/client/ui/Frame.java Mon Mar 14
09:06:35 2011
@@ -19,6 +19,10 @@
import com.google.gwt.dom.client.Element;
import com.google.gwt.dom.client.FrameElement;
import com.google.gwt.dom.client.IFrameElement;
+import com.google.gwt.event.dom.client.HasLoadHandlers;
+import com.google.gwt.event.dom.client.LoadEvent;
+import com.google.gwt.event.dom.client.LoadHandler;
+import com.google.gwt.event.shared.HandlerRegistration;
/**
* A widget that wraps an IFRAME element, which can contain an arbitrary
web
@@ -37,7 +41,7 @@
* Example {@example com.google.gwt.examples.FrameExample}
*
*/
-public class Frame extends Widget {
+public class Frame extends Widget implements HasLoadHandlers {
static final String DEFAULT_STYLENAME = "gwt-Frame";
@@ -91,6 +95,17 @@
IFrameElement.as(element);
setElement(element);
}
+
+ /**
+ * Adds a {@link LoadEvent} load
[gwt-contrib] [google-web-toolkit] r9852 committed - Fixing touch scrolling support for devices that support it natively. O...
Revision: 9852
Author: gwt.mirror...@gmail.com
Date: Tue Mar 15 07:55:50 2011
Log: Fixing touch scrolling support for devices that support it natively.
Our current touch support overrides native touch scrolling on tablets,
which breaks touch scrolling.
Review at http://gwt-code-reviews.appspot.com/1369809
Review by: p...@google.com
http://code.google.com/p/google-web-toolkit/source/detail?r=9852
Added:
/trunk/user/src/com/google/gwt/event/dom/TouchEvent.gwt.xml
Modified:
/trunk/user/src/com/google/gwt/event/dom/DomEvent.gwt.xml
/trunk/user/src/com/google/gwt/event/dom/client/TouchEvent.java
/trunk/user/src/com/google/gwt/touch/Touch.gwt.xml
/trunk/user/src/com/google/gwt/touch/client/TouchScroller.java
/trunk/user/test/com/google/gwt/touch/client/TouchScrollTest.java
===
--- /dev/null
+++ /trunk/user/src/com/google/gwt/event/dom/TouchEvent.gwt.xml Tue Mar 15
07:55:50 2011
@@ -0,0 +1,37 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+ class="com.google.gwt.event.dom.TouchEvent.TouchSupportDetectorNo">
+class="com.google.gwt.event.dom.TouchEvent.TouchSupportDetector" />
+
+
+
===
--- /trunk/user/src/com/google/gwt/event/dom/DomEvent.gwt.xml Fri Feb 6
13:06:24 2009
+++ /trunk/user/src/com/google/gwt/event/dom/DomEvent.gwt.xml Tue Mar 15
07:55:50 2011
@@ -1,5 +1,6 @@
-
-
+
+
+
===
--- /trunk/user/src/com/google/gwt/event/dom/client/TouchEvent.java Fri
Dec 3 07:31:58 2010
+++ /trunk/user/src/com/google/gwt/event/dom/client/TouchEvent.java Mon Mar
14 09:18:30 2011
@@ -15,6 +15,7 @@
*/
package com.google.gwt.event.dom.client;
+import com.google.gwt.core.client.GWT;
import com.google.gwt.core.client.JsArray;
import com.google.gwt.dom.client.Touch;
import com.google.gwt.event.shared.EventHandler;
@@ -27,6 +28,53 @@
*/
public abstract class TouchEvent
extends HumanInputEvent {
+
+ /**
+ * Dectector for browser support for touch events.
+ */
+ private static class TouchSupportDetector {
+
+private final boolean isSupported = detectTouchSupport();
+
+public boolean isSupported() {
+ return isSupported;
+}
+
+private native boolean detectTouchSupport() /*-{
+ var elem = document.createElement('div');
+ elem.setAttribute('ontouchstart', 'return;');
+ return (typeof elem.ontouchstart) == "function";
+}-*/;
+ }
+
+ /**
+ * Detector for browsers that do not support touch events.
+ */
+ @SuppressWarnings("unused")
+ private static class TouchSupportDetectorNo extends TouchSupportDetector
{
+@Override
+public boolean isSupported() {
+ return false;
+}
+ }
+
+ /**
+ * The implementation singleton.
+ */
+ private static TouchSupportDetector impl;
+
+ /**
+ * Runtime check for whether touch scrolling is supported in this
browser. Returns true if touch
+ * events are supported but touch based scrolling is not natively
supported.
+ *
+ * @return true if touch events are supported, false if not
+ */
+ public static boolean isSupported() {
+if (impl == null) {
+ impl = GWT.create(TouchSupportDetector.class);
+}
+return impl.isSupported();
+ }
/**
* Get an array of {@link Touch touches} which have changed since the
last
===
--- /trunk/user/src/com/google/gwt/touch/Touch.gwt.xml Sat Mar 5 06:16:25
2011
+++ /trunk/user/src/com/google/gwt/touch/Touch.gwt.xml Mon Mar 14 09:18:30
2011
@@ -13,25 +13,5 @@
-
-
-
-
-
-
-
-
-
-
-
-
-
-
- class="com.google.gwt.touch.client.TouchScroller.TouchSupportDetectorNo">
-class="com.google.gwt.touch.client.TouchScroller.TouchSupportDetector" />
-
-
+
===
--- /trunk/user/src/com/google/gwt/touch/client/TouchScroller.java Sat Mar
5 06:16:25 2011
+++ /trunk/user/src/com/google/gwt/touch/client/TouchScroller.java Mon Mar
14 09:18:30 2011
@@ -16,7 +16,6 @@
package com.google.gwt.touch.client;
import com.google.gwt.core.client.Duration;
-import com.google.gwt.core.client.GWT;
import com.google.gwt.core.client.JsArray;
import com.google.gwt.core.client.Scheduler;
import com.google.gwt.core.client.Scheduler.RepeatingCommand;
@@ -43,6 +42,11 @@
/**
* Adds touch based scrolling to a scroll panel.
+ *
+ *
+ * Touch based scrolling is only supported on devices that support touch
events
+ * and do not implement native touch based scrolling.
+ *
*/
@PartialSupport
public class TouchScroller {
@@ -136,35 +140,6 @@
return notDone;
}
}
-
- /**
- * Dectector for browser support for touch events.
- */
- private static class TouchSupportDetector {
-
-private final boolean isSupported = detectTouchSupport();
-
-public boolean isSupported() {
- return isSupported;
-}
-
-p
[gwt-contrib] Re: Fix problem with user-agent based locale selection (issue1382803)
Thanks, committed at r9850. http://gwt-code-reviews.appspot.com/1382803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] [google-web-toolkit] r9853 committed - Overhaul overlay types....
Revision: 9853
Author: cromwell...@google.com
Date: Mon Mar 14 10:45:50 2011
Log: Overhaul overlay types.
In general, JSOs now treated like other Java types, except for
cast/instanceof
and instantiability.
1) Eliminate JavaScriptObjectNormalizer. JSO subtypes are no longer
rewritten as
JSO, and trampoline functions not installed until after optimizations
2) No longer copy every Single JSO interface onto JSO itself
3) Teach TypeOracle about cross-cast JSO semantics
4) Each compiler pass recomputes whether a JSO interface is dually
implemented
5) ControlFlowAnalyzer treats implicit casts to JSO as instantiations
6) JsoDevirtualizer now devirtualizes any JSO subtype method (not just
java.lang.Object)
7) CastNormalizer treats non-dual Single JSO Interfaces as casts/instanceof
against the concrete implementor.
This patch allows the compiler to effectively remove trampoline functions
from
singlely implemented interface dispatches. It also rationalizes the way
JSOs are
treated to bring them more in line with regular Java types by preserving
their
type hierarchy in the AST.
Review by: sco...@google.com
http://code.google.com/p/google-web-toolkit/source/detail?r=9853
Deleted:
/trunk/dev/core/src/com/google/gwt/dev/jjs/impl/JavaScriptObjectNormalizer.java
Modified:
/trunk/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
/trunk/dev/core/src/com/google/gwt/dev/jjs/ast/JTypeOracle.java
/trunk/dev/core/src/com/google/gwt/dev/jjs/impl/CastNormalizer.java
/trunk/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java
/trunk/dev/core/src/com/google/gwt/dev/jjs/impl/ImplementClassLiteralsAsFields.java
/trunk/dev/core/src/com/google/gwt/dev/jjs/impl/JsoDevirtualizer.java
/trunk/dev/core/test/com/google/gwt/dev/jjs/JavaAstConstructor.java
/trunk/dev/core/test/com/google/gwt/dev/jjs/JjsTypeTest.java
/trunk/dev/core/test/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzerTest.java
/trunk/dev/core/test/com/google/gwt/dev/jjs/impl/JJSTestBase.java
===
---
/trunk/dev/core/src/com/google/gwt/dev/jjs/impl/JavaScriptObjectNormalizer.java
Thu Mar 3 14:34:14 2011
+++ /dev/null
@@ -1,286 +0,0 @@
-/*
- * Copyright 2008 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.impl;
-
-import com.google.gwt.dev.jjs.SourceInfo;
-import com.google.gwt.dev.jjs.ast.Context;
-import com.google.gwt.dev.jjs.ast.JArrayType;
-import com.google.gwt.dev.jjs.ast.JCastOperation;
-import com.google.gwt.dev.jjs.ast.JClassLiteral;
-import com.google.gwt.dev.jjs.ast.JClassType;
-import com.google.gwt.dev.jjs.ast.JConditional;
-import com.google.gwt.dev.jjs.ast.JDeclaredType;
-import com.google.gwt.dev.jjs.ast.JExpression;
-import com.google.gwt.dev.jjs.ast.JField;
-import com.google.gwt.dev.jjs.ast.JInstanceOf;
-import com.google.gwt.dev.jjs.ast.JLocal;
-import com.google.gwt.dev.jjs.ast.JLocalRef;
-import com.google.gwt.dev.jjs.ast.JMethod;
-import com.google.gwt.dev.jjs.ast.JMethodBody;
-import com.google.gwt.dev.jjs.ast.JMethodCall;
-import com.google.gwt.dev.jjs.ast.JModVisitor;
-import com.google.gwt.dev.jjs.ast.JNewArray;
-import com.google.gwt.dev.jjs.ast.JNonNullType;
-import com.google.gwt.dev.jjs.ast.JParameter;
-import com.google.gwt.dev.jjs.ast.JProgram;
-import com.google.gwt.dev.jjs.ast.JReferenceType;
-import com.google.gwt.dev.jjs.ast.JType;
-import com.google.gwt.dev.jjs.ast.js.JMultiExpression;
-import com.google.gwt.dev.util.log.speedtracer.CompilerEventType;
-import com.google.gwt.dev.util.log.speedtracer.SpeedTracerLogger;
-import com.google.gwt.dev.util.log.speedtracer.SpeedTracerLogger.Event;
-
-import java.util.Stack;
-
-/**
- * Replace references to JSO subtypes with JSO itself.
- */
-public class JavaScriptObjectNormalizer {
- /**
- * Map types from JSO subtypes to JSO itself.
- */
- private class NormalizeVisitor extends JModVisitor {
-
-private final Stack currentMethodBody = new
Stack();
-
-@Override
-public void endVisit(JCastOperation x, Context ctx) {
- JType newType = translate(x.getCastType());
- if (newType != x.getCastType()) {
-ctx.replaceMe(new JCastOperation(x.getSourceInfo(), newType,
-x.getExpr()));
- }
-}
-
-@Override
-public void endVisit(JClassLiteral x, Context ctx) {
- JType newType = translate(x.getRefType());
- if (newType != x.getRefType()) {
-ctx.replac
[gwt-contrib] [google-web-toolkit] r9855 committed - Rolling back jsinliner patch, issues have been identified...
Revision: 9855
Author: jbrosenb...@google.com
Date: Mon Mar 14 12:59:51 2011
Log: Rolling back jsinliner patch, issues have been identified
Review by: zun...@google.com
http://code.google.com/p/google-web-toolkit/source/detail?r=9855
Modified:
/trunk/dev/core/src/com/google/gwt/dev/js/JsInliner.java
/trunk/dev/core/test/com/google/gwt/dev/js/JsInlinerTest.java
===
--- /trunk/dev/core/src/com/google/gwt/dev/js/JsInliner.java Mon Mar 14
11:04:13 2011
+++ /trunk/dev/core/src/com/google/gwt/dev/js/JsInliner.java Mon Mar 14
12:59:51 2011
@@ -612,25 +612,10 @@
private boolean maintainsOrder = true;
private final List toEvaluate;
private final List unevaluated;
-private final Set paramsOrLocals = new HashSet();
-
-public EvaluationOrderVisitor(List toEvaluate, JsFunction
callee) {
+
+public EvaluationOrderVisitor(List toEvaluate) {
this.toEvaluate = toEvaluate;
this.unevaluated = new ArrayList(toEvaluate);
- // collect params and locals from callee function
- new JsVisitor() {
-@Override
-public void endVisit(JsParameter x, JsContext ctx) {
- paramsOrLocals.add(x.getName());
-}
-
-@Override
-public boolean visit(JsVar x, JsContext ctx) {
- // record this before visiting initializer
- paramsOrLocals.add(x.getName());
- return true;
-}
- }.accept(callee);
}
@Override
@@ -685,7 +670,12 @@
*/
@Override
public void endVisit(JsInvocation x, JsContext ctx) {
- if (unevaluated.size() > 0) {
+ /*
+ * The check for isExecuteOnce() is potentially incorrect here,
however
+ * the original Java semantics of the clinit would have made the code
+ * incorrect anyway.
+ */
+ if ((isExecuteOnce(x) == null) && unevaluated.size() > 0) {
maintainsOrder = false;
}
}
@@ -719,35 +709,11 @@
return maintainsOrder && unevaluated.size() == 0;
}
-/**
- * Check to see if the evaluation of this JsName will break program
order assumptions given
- * the parameters left to be substituted.
- *
- * The cases are as follows:
- * 1) JsName is a function parameter name which has side effects or is
affected by side effects
- * (hereafter called 'volatile'), so it will be in 'toEvaluate'
- * 2) JsName is a function parameter which is not volatile (not in
toEvaluate)
- * 3) JsName is a reference to a global variable
- * 4) JsName is a reference to a local variable
- *
- * A reference to a global while there are still parameters left to
evaluate / substitute
- * implies an order violation.
- *
- * A reference to a volatile parameter is ok if it is the next
parameter in sequence to
- * be evaluated (beginning of unevaluated list). Else, it is either
being evaluated out of
- * order with respect to other parameters, or it is being evaluated
more than once.
- */
private void checkName(JsName name) {
if (!toEvaluate.contains(name)) {
-// if the name is a non-local/non-parameter (e.g. global) and
there are params left to eval
-if (!paramsOrLocals.contains(name) && unevaluated.size() > 0) {
- maintainsOrder = false;
-}
-// else this may be a local, or all volatile params have already
been evaluated, so it's ok.
return;
}
- // either this param is being evaled twice, or out of order
if (unevaluated.size() == 0 || !unevaluated.remove(0).equals(name)) {
maintainsOrder = false;
}
@@ -1648,7 +1614,7 @@
* code to be inlined, but at a cost of larger JS output.
*/
private static final double MAX_COMPLEXITY_INCREASE =
Double.parseDouble(System.getProperty(
- "gwt.jsinlinerRatio", "1.7"));
+ "gwt.jsinlinerRatio", "5.0"));
/**
* Static entry point used by JavaToJavaScriptCompiler.
@@ -1968,7 +1934,7 @@
* order.
*/
EvaluationOrderVisitor orderVisitor = new EvaluationOrderVisitor(
- requiredOrder, callee);
+ requiredOrder);
orderVisitor.accept(toInline);
if (!orderVisitor.maintainsOrder()) {
return false;
===
--- /trunk/dev/core/test/com/google/gwt/dev/js/JsInlinerTest.java Mon Mar
14 11:04:13 2011
+++ /trunk/dev/core/test/com/google/gwt/dev/js/JsInlinerTest.java Mon Mar
14 12:59:51 2011
@@ -83,120 +83,6 @@
+ "function c1() { return ex2 ? a1() :c1(); } c1()";
compare(expected, input);
}
-
- /**
- * Test that a global array reference breaks argument ordering.
- */
- public void testOrderingArrayGlobal() throws Exception {
-StringBuffer code = new StringBuffer();
-
-code.append("var array; ");
-code.append("function clinit() { clinit = null; }");
-
-// callee references array[0] before evaluating argument
-code.append("fu
[gwt-contrib] [google-web-toolkit] r9854 committed - Reintroduces JsInliner patch with minor tweaks, addresses issue 5707 (...
Revision: 9854
Author: jbrosenb...@google.com
Date: Mon Mar 14 11:04:13 2011
Log: Reintroduces JsInliner patch with minor tweaks, addresses issue 5707
(previously submitted by
cromwellian at rev 9362)
Review at http://gwt-code-reviews.appspot.com/1386801
http://code.google.com/p/google-web-toolkit/source/detail?r=9854
Modified:
/trunk/dev/core/src/com/google/gwt/dev/js/JsInliner.java
/trunk/dev/core/test/com/google/gwt/dev/js/JsInlinerTest.java
===
--- /trunk/dev/core/src/com/google/gwt/dev/js/JsInliner.java Wed Feb 9
11:52:03 2011
+++ /trunk/dev/core/src/com/google/gwt/dev/js/JsInliner.java Mon Mar 14
11:04:13 2011
@@ -612,10 +612,25 @@
private boolean maintainsOrder = true;
private final List toEvaluate;
private final List unevaluated;
-
-public EvaluationOrderVisitor(List toEvaluate) {
+private final Set paramsOrLocals = new HashSet();
+
+public EvaluationOrderVisitor(List toEvaluate, JsFunction
callee) {
this.toEvaluate = toEvaluate;
this.unevaluated = new ArrayList(toEvaluate);
+ // collect params and locals from callee function
+ new JsVisitor() {
+@Override
+public void endVisit(JsParameter x, JsContext ctx) {
+ paramsOrLocals.add(x.getName());
+}
+
+@Override
+public boolean visit(JsVar x, JsContext ctx) {
+ // record this before visiting initializer
+ paramsOrLocals.add(x.getName());
+ return true;
+}
+ }.accept(callee);
}
@Override
@@ -670,12 +685,7 @@
*/
@Override
public void endVisit(JsInvocation x, JsContext ctx) {
- /*
- * The check for isExecuteOnce() is potentially incorrect here,
however
- * the original Java semantics of the clinit would have made the code
- * incorrect anyway.
- */
- if ((isExecuteOnce(x) == null) && unevaluated.size() > 0) {
+ if (unevaluated.size() > 0) {
maintainsOrder = false;
}
}
@@ -709,11 +719,35 @@
return maintainsOrder && unevaluated.size() == 0;
}
+/**
+ * Check to see if the evaluation of this JsName will break program
order assumptions given
+ * the parameters left to be substituted.
+ *
+ * The cases are as follows:
+ * 1) JsName is a function parameter name which has side effects or is
affected by side effects
+ * (hereafter called 'volatile'), so it will be in 'toEvaluate'
+ * 2) JsName is a function parameter which is not volatile (not in
toEvaluate)
+ * 3) JsName is a reference to a global variable
+ * 4) JsName is a reference to a local variable
+ *
+ * A reference to a global while there are still parameters left to
evaluate / substitute
+ * implies an order violation.
+ *
+ * A reference to a volatile parameter is ok if it is the next
parameter in sequence to
+ * be evaluated (beginning of unevaluated list). Else, it is either
being evaluated out of
+ * order with respect to other parameters, or it is being evaluated
more than once.
+ */
private void checkName(JsName name) {
if (!toEvaluate.contains(name)) {
+// if the name is a non-local/non-parameter (e.g. global) and
there are params left to eval
+if (!paramsOrLocals.contains(name) && unevaluated.size() > 0) {
+ maintainsOrder = false;
+}
+// else this may be a local, or all volatile params have already
been evaluated, so it's ok.
return;
}
+ // either this param is being evaled twice, or out of order
if (unevaluated.size() == 0 || !unevaluated.remove(0).equals(name)) {
maintainsOrder = false;
}
@@ -1614,7 +1648,7 @@
* code to be inlined, but at a cost of larger JS output.
*/
private static final double MAX_COMPLEXITY_INCREASE =
Double.parseDouble(System.getProperty(
- "gwt.jsinlinerRatio", "5.0"));
+ "gwt.jsinlinerRatio", "1.7"));
/**
* Static entry point used by JavaToJavaScriptCompiler.
@@ -1934,7 +1968,7 @@
* order.
*/
EvaluationOrderVisitor orderVisitor = new EvaluationOrderVisitor(
- requiredOrder);
+ requiredOrder, callee);
orderVisitor.accept(toInline);
if (!orderVisitor.maintainsOrder()) {
return false;
===
--- /trunk/dev/core/test/com/google/gwt/dev/js/JsInlinerTest.java Wed Feb
2 13:13:58 2011
+++ /trunk/dev/core/test/com/google/gwt/dev/js/JsInlinerTest.java Mon Mar
14 11:04:13 2011
@@ -83,6 +83,120 @@
+ "function c1() { return ex2 ? a1() :c1(); } c1()";
compare(expected, input);
}
+
+ /**
+ * Test that a global array reference breaks argument ordering.
+ */
+ public void testOrderingArrayGlobal() throws Exception {
+StringBuffer code = new StringBuffer();
+
+code.append("var array; ");
+code.append("function clinit() { clinit = null;
[gwt-contrib] [google-web-toolkit] r9856 committed - Test for compile failure if attributes don't match....
Revision: 9856
Author: ncha...@google.com
Date: Tue Mar 15 04:56:41 2011
Log: Test for compile failure if attributes don't match.
[JSR 303 TCK Result] 117 of 257 (45.53%) Pass with 14 Failures and 8 Errors.
Review at http://gwt-code-reviews.appspot.com/1384803
Review by: rchan...@google.com
http://code.google.com/p/google-web-toolkit/source/detail?r=9856
Added:
/trunk/user/test/org/hibernate/jsr303/tck/tests/constraints/constraintcomposition/OverriddenAttributesMustMatchInTypeTest.gwt.xml
/trunk/user/test/org/hibernate/jsr303/tck/tests/constraints/constraintcomposition/OverriddenAttributesMustMatchInTypeValidatorFactory.java
Modified:
/trunk/user/test/org/hibernate/jsr303/tck/tests/constraints/constraintcomposition/ConstraintCompositionCompileTest.java
/trunk/user/test/org/hibernate/jsr303/tck/tests/constraints/constraintcomposition/ConstraintCompositionGwtTest.java
/trunk/user/test/org/hibernate/jsr303/tck/util/TckCompileTestCase.java
===
--- /dev/null
+++
/trunk/user/test/org/hibernate/jsr303/tck/tests/constraints/constraintcomposition/OverriddenAttributesMustMatchInTypeTest.gwt.xml
Tue Mar 15 04:56:41 2011
@@ -0,0 +1,27 @@
+
+2.0.1//EN" "http://google-web-toolkit.googlecode.com/svn/tags/2.0.1/distro-source/core/src/gwt-module.dtd";>
+
+
+
+
+
+
+
+ class="org.hibernate.jsr303.tck.tests.constraints.constraintcomposition.OverriddenAttributesMustMatchInTypeValidatorFactory">
+
+
+
===
--- /dev/null
+++
/trunk/user/test/org/hibernate/jsr303/tck/tests/constraints/constraintcomposition/OverriddenAttributesMustMatchInTypeValidatorFactory.java
Tue Mar 15 04:56:41 2011
@@ -0,0 +1,32 @@
+package org.hibernate.jsr303.tck.tests.constraints.constraintcomposition;
+
+import com.google.gwt.core.client.GWT;
+import com.google.gwt.validation.client.AbstractGwtValidatorFactory;
+import com.google.gwt.validation.client.GwtValidation;
+import com.google.gwt.validation.client.impl.AbstractGwtValidator;
+
+import
org.hibernate.jsr303.tck.tests.constraints.constraintcomposition.ConstraintCompositionTest.DummyEntityWithZipCode;
+
+import javax.validation.Validator;
+
+/**
+ * {@link AbstractGwtValidatorFactory} implementation that uses
+ * {@link com.google.gwt.validation.client.GwtValidation GwtValidation}.
+ */
+public final class OverriddenAttributesMustMatchInTypeValidatorFactory
extends
+AbstractGwtValidatorFactory {
+
+ /**
+ * Validator for
+ * {@link
ConstraintCompositionTest#testOverriddenAttributesMustMatchInType()}
+ */
+ @GwtValidation(value = {DummyEntityWithZipCode.class})
+ public static interface OverriddenAttributesMustMatchInTypeValidator
extends
+ Validator {
+ }
+
+ @Override
+ public AbstractGwtValidator createValidator() {
+return GWT.create(OverriddenAttributesMustMatchInTypeValidator.class);
+ }
+}
===
---
/trunk/user/test/org/hibernate/jsr303/tck/tests/constraints/constraintcomposition/ConstraintCompositionCompileTest.java
Mon Feb 28 07:12:19 2011
+++
/trunk/user/test/org/hibernate/jsr303/tck/tests/constraints/constraintcomposition/ConstraintCompositionCompileTest.java
Tue Mar 15 04:56:41 2011
@@ -24,6 +24,7 @@
import org.hibernate.jsr303.tck.util.TckCompileTestCase;
+import javax.validation.ConstraintDefinitionException;
import javax.validation.UnexpectedTypeException;
/**
@@ -53,4 +54,30 @@
MustBeApplicableValidatorFactory.MustBeApplicableValidator.class,
Shoe.class);
}
-}
+
+ /**
+ * Replacement for
+ * {@link
ConstraintCompositionTest#testOverriddenAttributesMustMatchInType()}
+ *
+ * @throws UnableToCompleteException
+ */
+ public void testOverriddenAttributesMustMatchInType()
+ throws UnableToCompleteException {
+UnitTestTreeLogger.Builder builder = new UnitTestTreeLogger.Builder();
+builder.expect(Type.ERROR, "Unable to create a validator for "
+
+ "org.hibernate.jsr303.tck.tests.constraints.constraintcomposition."
++ "ConstraintCompositionTest.DummyEntityWithZipCode "
++ "because The overriding type of a composite constraint must be "
++ "identical to the overridden one. "
++ "Expected int found class java.lang.String",
+ConstraintDefinitionException.class);
+builder.setLowestLogLevel(Type.INFO);
+UnitTestTreeLogger testLogger = builder.createLogger();
+assertModuleFails(
+testLogger,
+getFullyQaulifiedModuleName(getClass(),
+"OverriddenAttributesMustMatchInTypeTest"),
+
OverriddenAttributesMustMatchInTypeValidatorFactory.OverriddenAttributesMustMatchInTypeValidator.class);
+ }
+
+}
===
---
/trunk/user/test/org/hibernate/jsr303/tck/tests/constraints/constraintcomposition/ConstraintCompositionGwtTest.java
Thu Mar 10 05:54:28 2011
+++
/trunk/user/test/org/hibernate/jsr303/tck/tests/constraints/con
[gwt-contrib] Re: Adds a cache (PersistenUnitCache) to store CompilationUnits (issue1375802)
On 2011/03/15 13:59:15, zundel wrote: First, I want to let you know I put a lot of thought into where to put the cache files and after discussions with Toby, we came up with the WEB-INF solution. There is precedent for putting the cache type files in WEB-INF (app engine integration with GWT does this). Also, its a terrific place when you consider that with our GWT ant setup, the cache gets removed when you run 'ant clean'.I've updated the Compiler entry point to also use the war/WEB-INF dir so the cache can be shared with web mode. SGTM, thanks for the explanation. http://gwt-code-reviews.appspot.com/1375802/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] Are there any docs describing internals of gwtc?
Hi Scott: I'm taking the opportunity to somewhat hijack this thread to ask a question regarding this topic: Is there an opportunity to use these internals to augment GWT testing strategies? Specifically, I'm wondering if the data structure generated during compilation can be used to help baseline testing. Can we take the information and create a network that represents a series of decisions taken in a particular way? This network would be used during the test phase. Cheers, jec -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Fix the worst concurrent modification problems in compiler memory-light set/map. (issue1384804)
Reviewers: zundel, Message: (I ran into a bug while debugging your PersistentUnitCache patch.) Due to a bug in com.google.gwt.dev.util.collect.HashMap.put(), we were eagerly growing the underlying table even when the key was already mapped. This causes an iterators to be invalidated on a non-structural change. Of course, we weren't actually tracking structural changes, which would cause the iterator to quietly continue, returning a 'null' keyed entry when it should not have, and possibly duplicating / skipping other entries. 1) I fixed the bug in HashMap.put() and HashSet.add() to defer growth until we're sure the key doesn't exist already. 2) Added basic concurrent modification tracking to the associated iterators, in a way that doesn't add additional memory to the collections themselves. 3) Other various and sundry cleanups. Please review this at http://gwt-code-reviews.appspot.com/1384804/ Affected files: M dev/core/src/com/google/gwt/dev/util/collect/HashMap.java M dev/core/src/com/google/gwt/dev/util/collect/HashSet.java M dev/core/src/com/google/gwt/dev/util/collect/IdentityHashSet.java M dev/core/src/com/google/gwt/dev/util/collect/IdentitySets.java M dev/core/src/com/google/gwt/dev/util/collect/Sets.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adds a cache (PersistenUnitCache) to store CompilationUnits (issue1375802)
Couple more comments after reading Jason's feedback. http://gwt-code-reviews.appspot.com/1375802/diff/9001/dev/core/src/com/google/gwt/dev/javac/PersistentUnitCache.java File dev/core/src/com/google/gwt/dev/javac/PersistentUnitCache.java (right): http://gwt-code-reviews.appspot.com/1375802/diff/9001/dev/core/src/com/google/gwt/dev/javac/PersistentUnitCache.java#newcode188 dev/core/src/com/google/gwt/dev/javac/PersistentUnitCache.java:188: ObjectOutputStream stream = null; This never gets closed, either. http://gwt-code-reviews.appspot.com/1375802/diff/9001/dev/core/src/com/google/gwt/dev/javac/PersistentUnitCache.java#newcode324 dev/core/src/com/google/gwt/dev/javac/PersistentUnitCache.java:324: super.add(newUnit); Agreed; although I also wonder if super.add() needs to wait for load to finish, otherwise newly-loaded-from-disk units will tend to clobber built units, unless you handle this in loadUnitMap(). http://gwt-code-reviews.appspot.com/1375802/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adds a cache (PersistenUnitCache) to store CompilationUnits (issue1375802)
Minor nits, overall looks like a good refactoring.
http://gwt-code-reviews.appspot.com/1375802/diff/9001/dev/core/src/com/google/gwt/dev/GWTCompiler.java
File dev/core/src/com/google/gwt/dev/GWTCompiler.java (right):
http://gwt-code-reviews.appspot.com/1375802/diff/9001/dev/core/src/com/google/gwt/dev/GWTCompiler.java#newcode179
dev/core/src/com/google/gwt/dev/GWTCompiler.java:179: tempWorkDir =
true;
What is the default here? Will we have a good persistentCacheDir in the
default case, for free? I'd think that would be desirable.
http://gwt-code-reviews.appspot.com/1375802/diff/9001/dev/core/src/com/google/gwt/dev/GWTShell.java
File dev/core/src/com/google/gwt/dev/GWTShell.java (right):
http://gwt-code-reviews.appspot.com/1375802/diff/9001/dev/core/src/com/google/gwt/dev/GWTShell.java#newcode200
dev/core/src/com/google/gwt/dev/GWTShell.java:200: protected boolean
doStartup() {
If getWorkDir() is null, what will be the behavior here (shouldn't be
the same as in GWTCompiler.java?
http://gwt-code-reviews.appspot.com/1375802/diff/9001/dev/core/src/com/google/gwt/dev/javac/BytecodeSignatureMaker.java
File dev/core/src/com/google/gwt/dev/javac/BytecodeSignatureMaker.java
(right):
http://gwt-code-reviews.appspot.com/1375802/diff/9001/dev/core/src/com/google/gwt/dev/javac/BytecodeSignatureMaker.java#newcode51
dev/core/src/com/google/gwt/dev/javac/BytecodeSignatureMaker.java:51:
~(Opcodes.ACC_DEPRECATED | Opcodes.ACC_NATIVE | Opcodes.ACC_STRICT
line wrap? Bad formatter artifact?
http://gwt-code-reviews.appspot.com/1375802/diff/9001/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java
File dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java
(right):
http://gwt-code-reviews.appspot.com/1375802/diff/9001/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java#newcode258
dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java:258:
}
whitespace
http://gwt-code-reviews.appspot.com/1375802/diff/9001/dev/core/src/com/google/gwt/dev/javac/Dependencies.java
File dev/core/src/com/google/gwt/dev/javac/Dependencies.java (right):
http://gwt-code-reviews.appspot.com/1375802/diff/9001/dev/core/src/com/google/gwt/dev/javac/Dependencies.java#newcode182
dev/core/src/com/google/gwt/dev/javac/Dependencies.java:182: } else {
all on one line?
http://gwt-code-reviews.appspot.com/1375802/diff/9001/dev/core/src/com/google/gwt/dev/javac/MemoryUnitCache.java
File dev/core/src/com/google/gwt/dev/javac/MemoryUnitCache.java (right):
http://gwt-code-reviews.appspot.com/1375802/diff/9001/dev/core/src/com/google/gwt/dev/javac/MemoryUnitCache.java#newcode32
dev/core/src/com/google/gwt/dev/javac/MemoryUnitCache.java:32: /**
What "weak" hash map are you referring to here?
http://gwt-code-reviews.appspot.com/1375802/diff/9001/dev/core/src/com/google/gwt/dev/javac/MemoryUnitCache.java#newcode74
dev/core/src/com/google/gwt/dev/javac/MemoryUnitCache.java:74: */
Seems this doesn't need to be initialized to a value here, it can be
declared final, and set to this value as a default else case in the
constructor below.
http://gwt-code-reviews.appspot.com/1375802/diff/9001/dev/core/src/com/google/gwt/dev/javac/MemoryUnitCache.java#newcode93
dev/core/src/com/google/gwt/dev/javac/MemoryUnitCache.java:93:
This should be AtomicBoolean (or at least volatile).
http://gwt-code-reviews.appspot.com/1375802/diff/9001/dev/core/src/com/google/gwt/dev/javac/PersistentUnitCache.java
File dev/core/src/com/google/gwt/dev/javac/PersistentUnitCache.java
(right):
http://gwt-code-reviews.appspot.com/1375802/diff/9001/dev/core/src/com/google/gwt/dev/javac/PersistentUnitCache.java#newcode109
dev/core/src/com/google/gwt/dev/javac/PersistentUnitCache.java:109:
setName("UnitCacheLoader");
Isn't this the default priority anyway?
http://gwt-code-reviews.appspot.com/1375802/diff/9001/dev/core/src/com/google/gwt/dev/javac/PersistentUnitCache.java#newcode228
dev/core/src/com/google/gwt/dev/javac/PersistentUnitCache.java:228: if
(unitWriteQueue.isEmpty()) {
flush if msg == UnitWriteMessage.SHUTDOWN_THREAD also?
http://gwt-code-reviews.appspot.com/1375802/diff/9001/dev/core/src/com/google/gwt/dev/javac/PersistentUnitCache.java#newcode231
dev/core/src/com/google/gwt/dev/javac/PersistentUnitCache.java:231: }
catch (IOException ex) {
Why use a level variable here?
http://gwt-code-reviews.appspot.com/1375802/diff/9001/dev/core/src/com/google/gwt/dev/javac/PersistentUnitCache.java#newcode303
dev/core/src/com/google/gwt/dev/javac/PersistentUnitCache.java:303: */
make final
http://gwt-code-reviews.appspot.com/1375802/diff/9001/dev/core/src/com/google/gwt/dev/javac/PersistentUnitCache.java#newcode307
dev/core/src/com/google/gwt/dev/javac/PersistentUnitCache.java:307: //
TODO(zundel): do we need to be thread safe?
It looks like addCount is only ever used for logging (in TRACE mode), so
it's probably not required to use AtomicInteger for program correctness.
But if you really want an accurate value, then AtomicInteger is a goo
[gwt-contrib] Re: Adds a cache (PersistenUnitCache) to store CompilationUnits (issue1375802)
I think I now understand the high-level design. Definitely seems
workable, and probably pretty fast, too. Just a lot of nits to work
out, I think.
http://gwt-code-reviews.appspot.com/1375802/diff/1/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java
File dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java
(right):
http://gwt-code-reviews.appspot.com/1375802/diff/1/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java#newcode232
dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java:232:
logger.log(TreeLogger.TRACE, "Invalid units found: " +
invalidatedUnits.size());
Okay, after some deep digging, this appears to be a bad interaction
between Dependencies and HashMap, see note in Dependencies.
http://gwt-code-reviews.appspot.com/1375802/diff/5002/dev/core/src/com/google/gwt/dev/javac/PersistentUnitCache.java
File dev/core/src/com/google/gwt/dev/javac/PersistentUnitCache.java
(right):
http://gwt-code-reviews.appspot.com/1375802/diff/5002/dev/core/src/com/google/gwt/dev/javac/PersistentUnitCache.java#newcode324
dev/core/src/com/google/gwt/dev/javac/PersistentUnitCache.java:324: *
out any old cached files.
On 2011/03/14 23:24:26, zundel wrote:
Here's my logic. If you are compiling new units, then it is likely
you'd be
invalidating old units. Plus, I am just frightened of a cache that
never gets
purged. This is currently the only mechanism to purge the cache.
We had issues with the old CacheManager where you'd get into bad states
and have to delete the cache directory to get back into a good state. I
kind of think it would be better to assume from the start that the cache
directory never, ever gets purged, and then prove to ourselves that
we'll never do something incorrect, and that it won't grow without
bound.
http://gwt-code-reviews.appspot.com/1375802/diff/5002/dev/core/src/com/google/gwt/dev/util/arg/ArgHandlerPersistentUnitCache.java
File
dev/core/src/com/google/gwt/dev/util/arg/ArgHandlerPersistentUnitCache.java
(right):
http://gwt-code-reviews.appspot.com/1375802/diff/5002/dev/core/src/com/google/gwt/dev/util/arg/ArgHandlerPersistentUnitCache.java#newcode33
dev/core/src/com/google/gwt/dev/util/arg/ArgHandlerPersistentUnitCache.java:33:
return "Enable persistent CompliationUnit caching. If this argument "
That's exactly what we told people with CacheManager. :D
http://gwt-code-reviews.appspot.com/1375802/diff/9001/dev/core/src/com/google/gwt/dev/Precompile.java
File dev/core/src/com/google/gwt/dev/Precompile.java (left):
http://gwt-code-reviews.appspot.com/1375802/diff/9001/dev/core/src/com/google/gwt/dev/Precompile.java#oldcode133
dev/core/src/com/google/gwt/dev/Precompile.java:133: Serializable {
Does this change have anything to do with this CL?
http://gwt-code-reviews.appspot.com/1375802/diff/9001/dev/core/src/com/google/gwt/dev/Precompile.java
File dev/core/src/com/google/gwt/dev/Precompile.java (right):
http://gwt-code-reviews.appspot.com/1375802/diff/9001/dev/core/src/com/google/gwt/dev/Precompile.java#newcode664
dev/core/src/com/google/gwt/dev/Precompile.java:664:
CompilationStateBuilder.init(logger, options.getWorkDir());
I would have expected this to be war dir.
http://gwt-code-reviews.appspot.com/1375802/diff/9001/dev/core/src/com/google/gwt/dev/PrecompileOnePerm.java
File dev/core/src/com/google/gwt/dev/PrecompileOnePerm.java (right):
http://gwt-code-reviews.appspot.com/1375802/diff/9001/dev/core/src/com/google/gwt/dev/PrecompileOnePerm.java#newcode186
dev/core/src/com/google/gwt/dev/PrecompileOnePerm.java:186:
CompilationStateBuilder.init(logger, options.getWorkDir());
Seems weird that this is workDir sometimes, and war dir other times.
http://gwt-code-reviews.appspot.com/1375802/diff/9001/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java
File dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java
(left):
http://gwt-code-reviews.appspot.com/1375802/diff/9001/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java#oldcode394
dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java:394:
ResourceTag tag = resourceContentCache.get(location);
I think this cache is actually important. Essentially, this cache says
"assume a source file is the same if its lastModified is the same".
Without this cache, you have no choice but to read in the contents of
ALL source files on every refresh just to compute the content ID, and
you can't take advantage of lastModified.
I think you either need to roll into UnitCache the concept of a
(Location,lastModified) -> ContentId, or else resurrect some of this
code and only compute ContentID on the initial load sequence rather than
on every refresh.
http://gwt-code-reviews.appspot.com/1375802/diff/9001/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java
File dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java
(right):
http://gwt-code-reviews.appspot.com/1375802/diff/9001/dev/core/src/com/google/gwt/dev/ja
[gwt-contrib] Re: Fix issue 5807 on server side. Previously reviewed at 1370803. (issue1384802)
http://gwt-code-reviews.appspot.com/1384802/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Operations like i += d where i is an int and d is a double are not properly (issue1385803)
Reviewers: scottb,
Description:
Operations like i += d where i is an int and d is a double are not
properly
truncated (narrowed) to the LHS type. This patch forces i += d to be
written as
i = i + d, and applies a narrowing cast as needed, e.g. i = (int)(i +
d);
Updated to fix String concat issue:
Please review this at http://gwt-code-reviews.appspot.com/1385803/
Affected files:
M dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
M
dev/core/src/com/google/gwt/dev/jjs/impl/CompoundAssignmentNormalizer.java
M
dev/core/src/com/google/gwt/dev/jjs/impl/PostOptimizationCompoundAssignmentNormalizer.java
Index: dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
===
--- dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
(revision 9856)
+++ dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
(working copy)
@@ -292,10 +292,10 @@
// (5) "Normalize" the high-level Java tree into a lower-level tree
more
// suited for JavaScript code generation. Don't go reordering these
// willy-nilly because there are some subtle interdependencies.
- LongCastNormalizer.exec(jprogram);
JsoDevirtualizer.exec(jprogram);
CatchBlockNormalizer.exec(jprogram);
PostOptimizationCompoundAssignmentNormalizer.exec(jprogram);
+ LongCastNormalizer.exec(jprogram);
LongEmulationNormalizer.exec(jprogram);
CastNormalizer.exec(jprogram, options.isCastCheckingDisabled());
ArrayNormalizer.exec(jprogram);
Index:
dev/core/src/com/google/gwt/dev/jjs/impl/CompoundAssignmentNormalizer.java
===
---
dev/core/src/com/google/gwt/dev/jjs/impl/CompoundAssignmentNormalizer.java
(revision 9856)
+++
dev/core/src/com/google/gwt/dev/jjs/impl/CompoundAssignmentNormalizer.java
(working copy)
@@ -161,8 +161,10 @@
new JMultiExpression(x.getSourceInfo()));
JExpression newLhs = replacer.accept(x.getLhs());
- JBinaryOperation operation = new JBinaryOperation(x.getSourceInfo(),
+ JExpression operation = new JBinaryOperation(x.getSourceInfo(),
newLhs.getType(), op.getNonAssignmentOf(), newLhs, x.getRhs());
+ operation = modifyResultOperation((JBinaryOperation) operation);
+
// newLhs is cloned below because it was used in operation
JBinaryOperation asg = new JBinaryOperation(x.getSourceInfo(),
newLhs.getType(),
JBinaryOperator.ASG, cloner.cloneExpression(newLhs),
@@ -285,6 +287,16 @@
return lhs;
}
+ /**
+ * Decide what expression to return when breaking up a compound
assignment of
+ * the form lhs op= rhs. The breakup creates an expression
of
+ * the form lhs = lhs op rhs, and the right hand side of the
+ * newly created expression is passed to this method.
+ */
+ protected JExpression modifyResultOperation(JBinaryOperation op) {
+return op;
+ }
+
protected abstract boolean shouldBreakUp(JBinaryOperation x);
protected abstract boolean shouldBreakUp(JPostfixOperation x);
Index:
dev/core/src/com/google/gwt/dev/jjs/impl/PostOptimizationCompoundAssignmentNormalizer.java
===
---
dev/core/src/com/google/gwt/dev/jjs/impl/PostOptimizationCompoundAssignmentNormalizer.java
(revision 9856)
+++
dev/core/src/com/google/gwt/dev/jjs/impl/PostOptimizationCompoundAssignmentNormalizer.java
(working copy)
@@ -17,10 +17,13 @@
import com.google.gwt.dev.jjs.ast.JBinaryOperation;
import com.google.gwt.dev.jjs.ast.JBinaryOperator;
+import com.google.gwt.dev.jjs.ast.JCastOperation;
+import com.google.gwt.dev.jjs.ast.JExpression;
import com.google.gwt.dev.jjs.ast.JPostfixOperation;
import com.google.gwt.dev.jjs.ast.JPrefixOperation;
import com.google.gwt.dev.jjs.ast.JPrimitiveType;
import com.google.gwt.dev.jjs.ast.JProgram;
+import com.google.gwt.dev.jjs.ast.JType;
/**
* Normalize compound assignments as needed after optimization. Integer
division
@@ -36,6 +39,21 @@
}
@Override
+ protected JExpression modifyResultOperation(JBinaryOperation op) {
+JType lhsType = op.getLhs().getType();
+JType rhsType = op.getRhs().getType();
+if (lhsType != rhsType) {
+ // first widen binary op to encompass both sides, then add narrow
cast
+ return new JCastOperation(op.getSourceInfo(), lhsType,
+ new JBinaryOperation(op.getSourceInfo(),
+ widenType(lhsType, rhsType),
+ op.getOp(),
+ op.getLhs(), op.getRhs()));
+}
+return op;
+ }
+
+ @Override
protected boolean shouldBreakUp(JBinaryOperation x) {
if (x.getType() == JPrimitiveType.LONG) {
return true;
@@ -43,6 +61,13 @@
if (x.getOp() == JBinaryOperator.ASG_DIV
&& x.getType() != JPrimitiveType.FLOAT
&& x.getType() != JPrimitiveType.DOUBLE) {
+ return tr
[gwt-contrib] Re: Operations like i += d where i is an int and d is a double are not properly (issue1385803)
http://gwt-code-reviews.appspot.com/1385803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Operations like i += d where i is an int and d is a double are not properly (issue1385803)
http://gwt-code-reviews.appspot.com/1385803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Operations like i += d where i is an int and d is a double are not properly (issue1385803)
http://gwt-code-reviews.appspot.com/1385803/diff/2002/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
File dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
(right):
http://gwt-code-reviews.appspot.com/1385803/diff/2002/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java#newcode298
dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java:298:
LongCastNormalizer.exec(jprogram);
Can you just describe for posterity why this is needed?
http://gwt-code-reviews.appspot.com/1385803/diff/2002/dev/core/src/com/google/gwt/dev/jjs/impl/PostOptimizationCompoundAssignmentNormalizer.java
File
dev/core/src/com/google/gwt/dev/jjs/impl/PostOptimizationCompoundAssignmentNormalizer.java
(right):
http://gwt-code-reviews.appspot.com/1385803/diff/2002/dev/core/src/com/google/gwt/dev/jjs/impl/PostOptimizationCompoundAssignmentNormalizer.java#newcode70
dev/core/src/com/google/gwt/dev/jjs/impl/PostOptimizationCompoundAssignmentNormalizer.java:70:
if (isIntegral(lhsType) && !isIntegral(rhsType)) {
I almost think this ought to be:
if (widen(lhsType, rhsType) != lhsType)) {
...
}
http://gwt-code-reviews.appspot.com/1385803/diff/2002/dev/core/src/com/google/gwt/dev/jjs/impl/PostOptimizationCompoundAssignmentNormalizer.java#newcode95
dev/core/src/com/google/gwt/dev/jjs/impl/PostOptimizationCompoundAssignmentNormalizer.java:95:
widenType(type, JPrimitiveType.LONG) == JPrimitiveType.LONG;
Cute, but it hard to just read this and prove to yourself it's correct.
Can we just do the explicit thing here?
http://gwt-code-reviews.appspot.com/1385803/diff/2002/dev/core/src/com/google/gwt/dev/jjs/impl/PostOptimizationCompoundAssignmentNormalizer.java#newcode98
dev/core/src/com/google/gwt/dev/jjs/impl/PostOptimizationCompoundAssignmentNormalizer.java:98:
private JType widenType(JType lhsType, JType rhsType) {
Did you copy this from somewhere, or is there a JLS section you can cite
for this?
http://gwt-code-reviews.appspot.com/1385803/diff/2002/dev/core/test/com/google/gwt/dev/jjs/impl/PostOptimizationCompoundAssignmentNormalizerTest.java
File
dev/core/test/com/google/gwt/dev/jjs/impl/PostOptimizationCompoundAssignmentNormalizerTest.java
(right):
http://gwt-code-reviews.appspot.com/1385803/diff/2002/dev/core/test/com/google/gwt/dev/jjs/impl/PostOptimizationCompoundAssignmentNormalizerTest.java#newcode74
dev/core/test/com/google/gwt/dev/jjs/impl/PostOptimizationCompoundAssignmentNormalizerTest.java:74:
"int x=2; short d=3; x += d;");
OTOH, short += int should get split up, can you add a test for that?
http://gwt-code-reviews.appspot.com/1385803/diff/2002/dev/core/test/com/google/gwt/dev/jjs/impl/PostOptimizationCompoundAssignmentNormalizerTest.java#newcode77
dev/core/test/com/google/gwt/dev/jjs/impl/PostOptimizationCompoundAssignmentNormalizerTest.java:77:
"int x=2; long d=3L; x += (int)d;");
I'm surprised, I would have thought this would be split. If you write
int = int + long you have to coerce the result to int.
http://gwt-code-reviews.appspot.com/1385803/diff/2002/dev/core/test/com/google/gwt/dev/jjs/impl/PostOptimizationCompoundAssignmentNormalizerTest.java#newcode86
dev/core/test/com/google/gwt/dev/jjs/impl/PostOptimizationCompoundAssignmentNormalizerTest.java:86:
"float x=2; double d=3.0; x += d;");
Same here, float = float + double forces you to coerce.
http://gwt-code-reviews.appspot.com/1385803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fix the worst concurrent modification problems in compiler memory-light set/map. (issue1384804)
LGTM http://gwt-code-reviews.appspot.com/1384804/diff/1/dev/core/src/com/google/gwt/dev/util/collect/HashMap.java File dev/core/src/com/google/gwt/dev/util/collect/HashMap.java (right): http://gwt-code-reviews.appspot.com/1384804/diff/1/dev/core/src/com/google/gwt/dev/util/collect/HashMap.java#newcode54 dev/core/src/com/google/gwt/dev/util/collect/HashMap.java:54: throw new ConcurrentModificationException(); I wonder how many places we'll find that start throwing this exception after this patch? http://gwt-code-reviews.appspot.com/1384804/diff/1/dev/core/src/com/google/gwt/dev/util/collect/HashMap.java#newcode111 dev/core/src/com/google/gwt/dev/util/collect/HashMap.java:111: HashMap.this.ensureSizeFor(Math.max(size(), c.size())); At first glance, this change doesn't make sense = couldn't we end up with size() + c.size() items in the worst case? (c is all new entries). OK, so we don't know until we see if the entries are actually in the map already. But do we really need this here? Is addAll() going to end up calling EntrySet.add()? We don't worry about ensureSizeFor there - we could just let HashMap.this.put() worry about it. My guess is that this is a heuristic to make sure we don't resize the table multiple times in the middle of addAll(). Comment please. http://gwt-code-reviews.appspot.com/1384804/diff/1/dev/core/src/com/google/gwt/dev/util/collect/HashMap.java#newcode482 dev/core/src/com/google/gwt/dev/util/collect/HashMap.java:482: HashMap.this.ensureSizeFor(Math.max(size(), m.size())); So here again, this looks odd http://gwt-code-reviews.appspot.com/1384804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fix the worst concurrent modification problems in compiler memory-light set/map. (issue1384804)
Thanks, will add doc. http://gwt-code-reviews.appspot.com/1384804/diff/1/dev/core/src/com/google/gwt/dev/util/collect/HashMap.java File dev/core/src/com/google/gwt/dev/util/collect/HashMap.java (right): http://gwt-code-reviews.appspot.com/1384804/diff/1/dev/core/src/com/google/gwt/dev/util/collect/HashMap.java#newcode54 dev/core/src/com/google/gwt/dev/util/collect/HashMap.java:54: throw new ConcurrentModificationException(); Hehe, good question. :) All our tests pass, at least. http://gwt-code-reviews.appspot.com/1384804/diff/1/dev/core/src/com/google/gwt/dev/util/collect/HashMap.java#newcode111 dev/core/src/com/google/gwt/dev/util/collect/HashMap.java:111: HashMap.this.ensureSizeFor(Math.max(size(), c.size())); On 2011/03/16 03:33:45, zundel wrote: At first glance, this change doesn't make sense = couldn't we end up with size() + c.size() items in the worst case? (c is all new entries). OK, so we don't know until we see if the entries are actually in the map already. But do we really need this here? Is addAll() going to end up calling EntrySet.add()? We don't worry about ensureSizeFor there - we could just let HashMap.this.put() worry about it. My guess is that this is a heuristic to make sure we don't resize the table multiple times in the middle of addAll(). Comment please. Yes, the original reason for size() + c.size() was to prevent having to do a bunch of rehashes during an addAll(). However, I realized when debugging this that we had a map with 24 entries and 64 capacity. Since a big design goal for these classes is memory efficiency, it seemed better to err on the side of less memory. It's true we could end up with size() + c.size() in the worst case. But it works out not so bad, actually. In the worst case, where size() and c.size() are roughly equal and the sets are completely disjoint, you might do 1 initial rehash and then 1 additional rehash down the road. But even this is an edge case that requires you to get unlucky on both boundaries. Most of the time, you'll do either 1 initial rehash or 1 down the road. I'll add a comment, thanks. http://gwt-code-reviews.appspot.com/1384804/diff/1/dev/core/src/com/google/gwt/dev/util/collect/HashMap.java#newcode482 dev/core/src/com/google/gwt/dev/util/collect/HashMap.java:482: HashMap.this.ensureSizeFor(Math.max(size(), m.size())); I think I'll just factor out a method, and javadoc it. http://gwt-code-reviews.appspot.com/1384804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Add Support for @ReportAsSingleViolation. (issue1388801)
Reviewers: rchandia, Description: Add Support for @ReportAsSingleViolation. [JSR 303 TCK Result] 119 of 257 (46.30%) Pass with 12 Failures and 8 Errors. Please review this at http://gwt-code-reviews.appspot.com/1388801/ Affected files: M user/src/com/google/gwt/validation/client/impl/AbstractGwtSpecificValidator.java M user/src/com/google/gwt/validation/rebind/GwtSpecificValidatorCreator.java M user/test/org/hibernate/jsr303/tck/tests/constraints/constraintcomposition/ConstraintCompositionGwtTest.java M user/test/org/hibernate/jsr303/tck/tests/validation/ValidateGwtTest.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add Support for @ReportAsSingleViolation. (issue1388801)
http://gwt-code-reviews.appspot.com/1388801/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add Support for @ReportAsSingleViolation. (issue1388801)
http://gwt-code-reviews.appspot.com/1388801/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add Support for @ReportAsSingleViolation. (issue1388801)
On 2011/03/16 05:29:29, Nick Chalko wrote: Ok, tests are passing now. Please take a look. http://gwt-code-reviews.appspot.com/1388801/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
