[gwt-contrib] Re: fix to issue 3277

2009-02-09 Thread spoon
On 2009/02/09 19:21:53, rjrjr wrote: Is there really no test coverage for this? That's right. Is there test coverage on any LWM logging? http://gwt-code-reviews.appspot.com/4802 --~--~-~--~~~---~--~~

[gwt-contrib] Re: fix to issue 3277

2009-02-09 Thread spoon
http://gwt-code-reviews.appspot.com/4802/diff/1/3#newcode166 Line 166: leftoversFragmentNumber(), -1); You always set the size to -1, mentioning that in the future some linker could supply the size of the downloaded code. Add some comment or javadoc indicating why this is currently -1, or

[gwt-contrib] Re: Add -soyc flag to Compiler

2009-04-14 Thread spoon
This is a good change overall. Compiler should accept -soyc. http://gwt-code-reviews.appspot.com/22802 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---

[gwt-contrib] Re: Fix Issue 1694: Frequent needless use of super.method()

2009-05-12 Thread spoon
LGTM. I added some people who know the widgets library to the CC list. Guys, please veto if you really think we need these super calls, presumably for performance. http://gwt-code-reviews.appspot.com/34803 --~--~-~--~~~---~--~~

[gwt-contrib] Re: Addresses issues 1820736 and 1843669 (Soyc dashboard improvements)

2009-05-15 Thread spoon
LGTM http://gwt-code-reviews.appspot.com/33822/diff/1001/10 File tools/soyc-vis/src/com/google/gwt/soyc/Settings.java (right): http://gwt-code-reviews.appspot.com/33822/diff/1001/10#newcode121 Line 121: + Settings.settingsHelp()); Maybe explicitly mention the -resources argument in the error

[gwt-contrib] add -out to SoycDashboard

2009-05-18 Thread spoon
Reviewers: kprobst, Description: This adds a -out option to SoycDashboard. If the option is specified, then the generated code goes to that directory rather than to the current working directory. Please review this at http://gwt-code-reviews.appspot.com/33825 Affected files:

[gwt-contrib] Re: Small clean-up in SOYC dashboard

2009-05-20 Thread spoon
LGTM. There was earlier a reason for some to be non-static, then that got removed through the review process, and then I forgot to make them consistent again. http://gwt-code-reviews.appspot.com/34813 --~--~-~--~~~---~--~~

[gwt-contrib] split up long var lines

2009-05-20 Thread spoon
Reviewers: jgw, Description: Per issue 3455, split up very long var lines to avoid a new Webkit bug. This also adds a CompilerParameters module. It looks useful for testing to be able to tune way down compiler settings such as this one. Please review this at

[gwt-contrib] Re: add -out to SoycDashboard

2009-05-20 Thread spoon
Thanks. Committed at r5442. http://gwt-code-reviews.appspot.com/33825 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---

[gwt-contrib] Re: split up long var lines

2009-05-20 Thread spoon
It is still possible for super-deep expressions to come out of the compiler. As yet it is not a practical problem, however. The depth limit is 1. We are only bumping into such a large limit because of the unusual encoding used for var statements. That said, if it's a practical problem,

[gwt-contrib] Re: split up long var lines

2009-05-21 Thread spoon
Thanks, Joel. Committed at r5459. http://gwt-code-reviews.appspot.com/33826 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---

[gwt-contrib] Re: split up long var lines

2009-05-21 Thread spoon
http://gwt-code-reviews.appspot.com/33826/diff/1/4 File user/src/com/google/gwt/core/CompilerParameters.gwt.xml (right): http://gwt-code-reviews.appspot.com/33826/diff/1/4#newcode25 Line 25: set-configuration-property name='compiler.max.vars.per.var' value='4900' / Okay, I'll change it to half.

[gwt-contrib] Re: Small bug in SOYC dashboard - unzipping input files

2009-05-27 Thread spoon
LGTM http://gwt-code-reviews.appspot.com/34821 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---

[gwt-contrib] one-liner for SoycDashboard to pay more attention to -out

2009-05-29 Thread spoon
Reviewers: kprobst, Description: I missed a case when adding -out to SoycDashboard, so one of the files goes to the current directory rather than to the -out directory. Please review this at http://gwt-code-reviews.appspot.com/33834 Affected files:

[gwt-contrib] Re: one-liner for SoycDashboard to pay more attention to -out

2009-05-29 Thread spoon
Thanks, Kathrin! It's in at r5478. http://gwt-code-reviews.appspot.com/33834 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---

[gwt-contrib] Re: Speedups for -soyc compilation

2009-05-29 Thread spoon
LGTM, with a few things needed: 1. There should be a test case for writeUtf8. 2. It's worth double checking that locally caching the StringBuilder helps. 3. Please double check the output if that hasn't been done already; for example, run the SOYC dashboard, or compare auto-formatted versions of

[gwt-contrib] SOYC gives drill-down on size for leftover and exclusive fragments

2009-06-05 Thread spoon
Reviewers: krpobst_google.com, Description: SOYC adds a drill-down size breakdown for the exclusive and leftover fragmetns, just like the one it currently gives for the total program and the initial download. Please review this at http://gwt-code-reviews.appspot.com/33843 Affected files:

[gwt-contrib] Re: SOYC gives drill-down on size for leftover and exclusive fragments

2009-06-08 Thread spoon
Thanks, Kathrin! There are few questions inline before I commit it. http://gwt-code-reviews.appspot.com/33843/diff/1/6 File tools/soyc-vis/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java (right): http://gwt-code-reviews.appspot.com/33843/diff/1/6#newcode629 Line 629: if (i ==

[gwt-contrib] Re: SOYC gives drill-down on size for leftover and exclusive fragments

2009-06-08 Thread spoon
Thank you! It is committed at r5521. http://gwt-code-reviews.appspot.com/33843 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---

[gwt-contrib] Re: Fixes obscure failure in linux hybrid-mode unit tests

2009-06-09 Thread spoon
LGTM. Great digging! http://gwt-code-reviews.appspot.com/33847 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---

[gwt-contrib] initial load sequence for runAsync's

2009-06-09 Thread spoon
Reviewers: bobv, Description: Design doc here: http://code.google.com/p/google-web-toolkit/wiki/InitialLoadSequence This patch allows specifying an initial load sequence for runAsync calls. If the specified sequence matches the actual order the runAsync's are reached at run time, then the

[gwt-contrib] fix a crash in code the JDT considers dead

2009-06-10 Thread spoon
Reviewers: scottb, Description: GenerateJavaAST can crash when trying to convert code the JDT compiler considers dead, because in such a case the JDT will leave ill-formed code in the tree. This patch covers one such case: if a block of code contains an if(true) that contains a throw, the JDT

[gwt-contrib] Re: fix a crash in code the JDT considers dead

2009-06-10 Thread spoon
Sure, will do. http://gwt-code-reviews.appspot.com/34835 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---

[gwt-contrib] Re: initial load sequence for runAsync's

2009-06-11 Thread spoon
-reviews.appspot.com/33848/diff/1/6#newcode340 Line 340: * TODO(spoon) accept a labeled runAsync call, once runAsyncs can be labeled Some kind of annotation at the runAsync call site. The details are TBD. http://gwt-code-reviews.appspot.com/33848/diff/1/6#newcode349 Line 349: + PROP_INITIAL_SEQUENCE

[gwt-contrib] Re: initial load sequence for runAsync's

2009-06-11 Thread spoon
How about now? In addition to the requested changes, I changed two other things: 1. The SOYC timings use PerfLogger now, rather than inlined timing code 2. There is a check for the same split point being specified multiple times in the initial sequence

[gwt-contrib] -draftCompile optimizes less

2009-06-11 Thread spoon
Reviewers: scottb, Description: In trunk, -draftCompile does one full outer loop of the optimization loop. This patch pares that one loop down much further. A one-permutation draft compile of Showcase now takes about 25 seconds, as compared to 27 seconds before. Additionally, the actual

[gwt-contrib] Re: Conditionally optimize precompiles

2009-06-11 Thread spoon
LGTM. Good idea. http://gwt-code-reviews.appspot.com/38802 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---

[gwt-contrib] Re: initial load sequence for runAsync's

2009-06-15 Thread spoon
Committed at r5560. http://gwt-code-reviews.appspot.com/33848/diff/4002/4005 File dev/core/src/com/google/gwt/core/ext/soyc/impl/SplitPointRecorder.java (right): http://gwt-code-reviews.appspot.com/33848/diff/4002/4005#newcode68 Line 68: String location = splitPointMap.get(sp); On 2009/06/15

[gwt-contrib] inliner handles implicit long cast at method return

2009-06-16 Thread spoon
Reviewers: scottb, Description: Issue 3710 is caused by the Java inliner failing to take into account an implicit cast to long in Integer::longValue(). The inliner already adds explicit casts to the parameters of inlined methods. This patch has it do so for return values as well. If we run

[gwt-contrib] Re: inliner handles implicit long cast at method return

2009-06-17 Thread spoon
Thanks! It's committed at r5574. http://gwt-code-reviews.appspot.com/39805 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---

[gwt-contrib] fixes runAsync initial load sequences and adds a test case

2009-06-18 Thread spoon
Reviewers: bobv, Description: In trunk, the use of an initial load sequence is currently broken. This patch has the one-line fix, and adds a test case of initial load sequences. Please review this at http://gwt-code-reviews.appspot.com/43802 Affected files:

[gwt-contrib] Re: fixes runAsync initial load sequences and adds a test case

2009-06-19 Thread spoon
http://gwt-code-reviews.appspot.com/43802/diff/1/4 File user/test/com/google/gwt/dev/jjs/InitialLoadSequence.gwt.xml (right): http://gwt-code-reviews.appspot.com/43802/diff/1/4#newcode2 Line 2: !-- Copyright 2007 Google Inc. -- On 2009/06/19 02:23:12, bobv wrote: 2009 Done.

[gwt-contrib] Re: fixes runAsync initial load sequences and adds a test case

2009-06-19 Thread spoon
Committed at r5590. http://gwt-code-reviews.appspot.com/43802 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---

[gwt-contrib] Re: RR : Export typeids through SymbolMap data

2009-06-19 Thread spoon
LGTM. That will be helpful information to have in the symbol maps. http://gwt-code-reviews.appspot.com/41802 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---

[gwt-contrib] Re: Fix JsInliner for mutually-recursive functions

2009-06-22 Thread spoon
LGTM http://gwt-code-reviews.appspot.com/44803 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---

[gwt-contrib] Re: Assertion enabled by default when running GWT web mode tests and add the disabled flag -da

2009-06-24 Thread spoon
I haven't looked at the code yet, but would be happy to do so if Freeland's review is not enough. I just want to say that I would vote for the -da flag to be longer and more obscure, because the use case is important but not common. Perhaps -XdisableAssertions?

[gwt-contrib] add a test for AsyncFragmentLoader

2009-06-26 Thread spoon
Reviewers: bobv, Description: This patch adds a test for AsyncFragmentLoader. It pokes the loader with different sequences of runAsync requests, load success events, and load failed events, and makes sure that the correct fragments are requested in the correct order. To make it work,

[gwt-contrib] Re: Manifest for SOYC permutations

2009-06-29 Thread spoon
LGTM. Ideally, the similar information in the symbol maps files could be reused. However, this patch certainly does the job as is. It's up to you. http://gwt-code-reviews.appspot.com/50801 --~--~-~--~~~---~--~~

[gwt-contrib] SOYC generates more dependency trees

2009-07-01 Thread spoon
Reviewers: kathrin, Description: Currently, SOYC shows dependency information for a whole-program dependency trace. This patch additionally shows the various dependency graphs used by the code splitter to decide what code goes into what fragment. The display of this information can use work.

[gwt-contrib] Re: SOYC generates more dependency trees

2009-07-06 Thread spoon
Committed at r5676. http://gwt-code-reviews.appspot.com/50806 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---

[gwt-contrib] Re: add a test for AsyncFragmentLoader

2009-07-16 Thread spoon
Thanks, Bob and Scott! I left a couple of style things as they are; if you really think they should happen I will change them in a followup patch. Committed at r5749. http://gwt-code-reviews.appspot.com/47806/diff/1/3 File dev/core/src/com/google/gwt/dev/jjs/impl/CodeSplitter.java (right):

[gwt-contrib] Re: RR : Emulate JS stack traces (phase 1)

2009-07-22 Thread spoon
This patch took me hours to absorb. It sounds simple to modify the compiler to emulate stacks, but understanding precisely where the stack updates need to happen is difficult! The implementation is very clean. There are just two nits in the comments. The line number recording looks like it

[gwt-contrib] Re: RR : Emulate JS stack traces (phase 1)

2009-07-22 Thread spoon
http://gwt-code-reviews.appspot.com/47816/diff/1/5 File dev/core/src/com/google/gwt/dev/js/JsStackEmulator.java (right): http://gwt-code-reviews.appspot.com/47816/diff/1/5#newcode184 Line 184: * the stack depth. Oh, right, the pop for a return has to be placed in the enclosing finally, and it

[gwt-contrib] fix a buffer overflow in AsyncFragmentLoader

2009-07-22 Thread spoon
Reviewers: bobv, Description: Freelend (fabbott) ran our test suite with assertions enabled, and found an assertion failure in AsyncFragmentLoader. Curiously, the code likely works in a browser, due to arrays not being bounds checked when compiling to JavaScript, but certainly the code is not

[gwt-contrib] Re: RR : Emulate JS stack traces (phase 1)

2009-07-22 Thread spoon
LGTM. http://gwt-code-reviews.appspot.com/47816/diff/3008/1037 File dev/core/src/com/google/gwt/dev/js/JsStackEmulator.java (right): http://gwt-code-reviews.appspot.com/47816/diff/3008/1037#newcode68 Line 68: private class Bootstrap extends JsVisitor { Bootstrap took me a few minutes to figure

[gwt-contrib] Re: Fix a catch block normalizer bug

2009-07-23 Thread spoon
LGTM. The new AST is cleaner and should be more robust. http://gwt-code-reviews.appspot.com/51813 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---

[gwt-contrib] Re: SOYC filenames all end with .html

2009-07-31 Thread spoon
Thanks! Commited at r5857. http://gwt-code-reviews.appspot.com/54813 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---

[gwt-contrib] fix and test the runAsync lightweight metrics

2009-07-31 Thread spoon
Reviewers: bobv, Description: In some cases, the runAsync lightweight metrics generated the wrong event or dropped an event entirely. This patch fixes the two known problems and adds two tests of the lightweight metrics. First, AsyncFragmentLoaderTest now also verifies that the correct

[gwt-contrib] Re: faster size breakdown with non-fractional billing

2009-08-03 Thread spoon
I've cleaned it up in response to your comments, Kathrin. I also merged it up to the latest trunk, and I realize that developments in trunk mean that a new way is needed to record string sizes. That's next. http://gwt-code-reviews.appspot.com/51804/diff/1/8 File

[gwt-contrib] Re: fix and test the runAsync lightweight metrics

2009-08-06 Thread spoon
I'll commit it soon. While running pre-submit tests, I noticed that the integration test fails in hosted mode, as is expected. I'm adding a GWT.isScript() guard around that test. http://gwt-code-reviews.appspot.com/51829/diff/1/3 File

[gwt-contrib] move SOYC source code underneath dev/core/src

2009-08-10 Thread spoon
Reviewers: fabbott, Description: This patch moves SOYC's source code underneath dev/core/src. It leaves gwt-soyc-vis.jar existing and containing the SOYC static resources (css and gifs). It has gwt-dev-platform.jar also contain the SOYC static resources. The goal is to phase out

[gwt-contrib] add names to runAsync calls

2009-08-11 Thread spoon
Reviewers: scottb, Description: This patch allows naming a runAsync call using a class literal. There are now two overloads of GWT.runAsync, and the second one takes a class literal as an argument. That literal is recorded by the ReplaceRunAsyncs pass. It can then be used in the specification

[gwt-contrib] Re: SOYC for multiple permutations, plus updated layout

2009-08-12 Thread spoon
These are both great changes! I believe we should figure out the upgrade path, though, before committing the part about multi-permutation support. There are a couple of strategies described below, or maybe you can think of another one. One way or another, the goal is to find a way to support

[gwt-contrib] Re: move SOYC source code underneath dev/core/src

2009-08-12 Thread spoon
Thanks, Freeland! You're right that the diff doesn't include the file adds. Here is the output from svn status , which shows where the add is supposed to be be: D tools/soyc-vis/src/com/google/gwt/soyc D tools/soyc-vis/src/com/google/gwt/soyc/Settings.java D

[gwt-contrib] reenable SOYC dependencies files for non-split programs

2009-08-12 Thread spoon
Reviewers: kathrin, Description: Currently, the compiler emits bogus dependencies files if the program has no calls to runAsync. This patch has the compiler generate a valid dependency file when -soyc is on but splitting is off. As part of making this work, the patch also adjusts the default

[gwt-contrib] Re: add names to runAsync calls

2009-08-13 Thread spoon
Okay, now to redo the tests. http://gwt-code-reviews.appspot.com/56814/diff/1/9 File dev/core/src/com/google/gwt/dev/jjs/impl/CodeSplitter.java (right): http://gwt-code-reviews.appspot.com/56814/diff/1/9#newcode455 Line 455: if (splitPoints.size() != 1) { On 2009/08/13 19:50:17, scottb wrote:

[gwt-contrib] Re: Issue 3936: work around JDT by not computing CUD from scope chain

2009-08-14 Thread spoon
LGTM. http://gwt-code-reviews.appspot.com/58801 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---

[gwt-contrib] move SOYC resources out of the top-level jar directory

2009-08-17 Thread spoon
Reviewers: kathrin, Description: This patch moves the SOYC static resources (gifs and css files) into a subdirectory within the resulting jar. This change should unblock the following issue, which moves all of the SOYC code and resources into the gwt-dev-jar. This part is a separate patch so

[gwt-contrib] Re: move SOYC resources out of the top-level jar directory

2009-08-17 Thread spoon
Committed at r5973. http://gwt-code-reviews.appspot.com/60803/diff/1/2 File tools/soyc-vis/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java (right): http://gwt-code-reviews.appspot.com/60803/diff/1/2#newcode126 Line 126: + /resources/; I went back and forth on it. I settled on inferring

[gwt-contrib] Re: Illegal XML characters in SOYC XML files

2009-08-19 Thread spoon
LGTM. There is a tricky problem here, probably deserving a comment in the SOYC code. Ideally, the XML file should be non-lossy, and the original string text should be recoverable. The best way I have run into to accomplish that would be to convert the string data back into string literal

[gwt-contrib] Re: Illegal XML characters in SOYC XML files

2009-08-19 Thread spoon
I like the #x; idea. There is just one potential problem: will XML readers support it? The linked XML spec has the same restrictions on encoded character entities as on raw characters appearing in the file. Does anyone know if that restriction is honored in practice? Anyone want to test on

[gwt-contrib] separate the ADD operation into two: one for numerics, one for string append

2009-08-21 Thread spoon
Reviewers: scottb, Description: A challenge for the nullness tracking patch, as well as any other patch that modifies the compiler's internal type system, is to keep track of which ADD operations mean string append and which ones mean numeric addition. This patch has GenerateJavaAST make that

[gwt-contrib] Re: separate the ADD operation into two: one for numerics, one for string append

2009-08-21 Thread spoon
I'll change the string versions to CONCAT/ASG_CONCAT. I slightly liked removing ADD, so as to force people to decide which one they wanted, but perhaps that's overthinking it. I'll change it back to ADD/ASG_ADD. http://gwt-code-reviews.appspot.com/61810

[gwt-contrib] Re: separate the ADD operation into two: one for numerics, one for string append

2009-08-21 Thread spoon
On 2009/08/21 16:44:38, cromwellian wrote: Wouldn't another option simply be to turn this into a method call when it comes out of the JDT? e.g. a + b - a.append(b)? That works, too, and in some cases I believe it works really well. The biggest issue I see is that there are several parts of

[gwt-contrib] Nullness tracking

2009-08-25 Thread spoon
Reviewers: scottb, Description: Tracks nullness within the compiler by adding a JNonNull type. See http://code.google.com/p/google-web-toolkit/issues/detail?id=1819 . Please review this at http://gwt-code-reviews.appspot.com/62805 Affected files:

[gwt-contrib] Re: Fix path issue in SoycDashboard after moving soyc resources to a sub-directory

2009-08-26 Thread spoon
LGTM http://gwt-code-reviews.appspot.com/61815 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---

[gwt-contrib] Re: Eliminate dead links in SOYC dashboard when displayDependencies is off

2009-09-08 Thread spoon
LGTM. Good catch! http://gwt-code-reviews.appspot.com/65803 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---

[gwt-contrib] Re: Fixes double escaping bug in UiBinder messages

2009-09-10 Thread spoon
LGTM. One bit of comment cruft would be good to delete. http://gwt-code-reviews.appspot.com/64810/diff/1/5 File user/src/com/google/gwt/uibinder/rebind/messages/PlaceholderInterpreter.java (right): http://gwt-code-reviews.appspot.com/64810/diff/1/5#newcode115 Line 115: * @return

[gwt-contrib] Re: Add permutation info to index page for each permutation

2009-09-10 Thread spoon
LGTM http://gwt-code-reviews.appspot.com/64808 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---

[gwt-contrib] Re: Introduces inline styles to ui.xml files

2009-09-10 Thread spoon
LGTM. One comment typo, one question about a possible extra test. http://gwt-code-reviews.appspot.com/64812/diff/1/5 File user/src/com/google/gwt/uibinder/rebind/model/ImplicitCssResource.java (right): http://gwt-code-reviews.appspot.com/64812/diff/1/5#newcode67 Line 67: * file rather than

[gwt-contrib] Re: Introduces inline styles to ui.xml files

2009-09-10 Thread spoon
Yes, I've been using LGTM with comments to mean go ahead and commit, and I'll look at the changes afterward. http://gwt-code-reviews.appspot.com/64812/diff/1/5 File user/src/com/google/gwt/uibinder/rebind/model/ImplicitCssResource.java (right):

[gwt-contrib] Re: Nullness tracking

2009-09-18 Thread spoon
Responses below. I'll upload a new patch in a moment. I still need to get size updates and to try and recover the reason why TypeTightener has this new code checking for a cast that only checks for nullness. I'm guessing the extra cast caused worse code generation somewhere.

[gwt-contrib] Re: Intraprocedural flow-based optimizations

2009-10-13 Thread spoon
LGTM, except with one small issue and one nit. I don't need to rereview it. http://gwt-code-reviews.appspot.com/66804/diff/1/3 File dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java (right): http://gwt-code-reviews.appspot.com/66804/diff/1/3#newcode239 Line 239: } while

[gwt-contrib] Re: maximal sharding of builds and linkers

2009-10-14 Thread spoon
IMPLEMENTATION NOTES This patch updates the Linker API to be shardable. It updates the compiler to deal with this new API, and it updates the built-in linkers to support it. The main API change is that Linker.link has an extra boolean argument. That method is to be called once per permutation

[gwt-contrib] Re: In-block Clinit Pruner

2009-10-16 Thread spoon
http://gwt-code-reviews.appspot.com/80801/diff/1/2 File dev/core/src/com/google/gwt/dev/jjs/impl/flow/ClInitPruner.java (right): http://gwt-code-reviews.appspot.com/80801/diff/1/2#newcode63 Line 63: tracer.inc(Tracing.COUNT_PRUNABLE_CLINIT); Everything looks beautiful except here. Where is the

[gwt-contrib] Re: RR : GWT 2.0: Initial thoughts on $entry

2009-10-16 Thread spoon
http://gwt-code-reviews.appspot.com/77810/diff/1007/30 File user/src/com/google/gwt/core/client/impl/Impl.java (right): http://gwt-code-reviews.appspot.com/77810/diff/1007/30#newcode166 Line 166: // What is the correct return value here or should we re-throw? Additionally, this could be a

[gwt-contrib] fixes an overly long filename in a compile report

2009-10-29 Thread spoon
Reviewers: kathrin, Description: One-liner. Please review this at http://gwt-code-reviews.appspot.com/90801 Affected files: dev/core/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java Index: dev/core/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java

[gwt-contrib] Re: fixes an overly long filename in a compile report

2009-10-29 Thread spoon
Thanks! http://gwt-code-reviews.appspot.com/90801 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---

[gwt-contrib] Re: Yet another SOYC dashboard styling pass

2009-11-02 Thread spoon
LGTM http://gwt-code-reviews.appspot.com/91805 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---

[gwt-contrib] Re: Inline Polymorphic Function Declarations

2009-11-03 Thread spoon
LGTM, but with a few small changes requested. No need for rereview. http://gwt-code-reviews.appspot.com/89810/diff/1/2 File dev/core/src/com/google/gwt/dev/jjs/impl/FragmentExtractor.java (right): http://gwt-code-reviews.appspot.com/89810/diff/1/2#newcode175 Line 175: * the call to

[gwt-contrib] Re: RR : GWT 2.0 : Fix SingleJsoImpl and method overloads

2009-11-04 Thread spoon
LGTM http://gwt-code-reviews.appspot.com/92801 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---

[gwt-contrib] Re: RR : GWT 2.0 : Make SingleJsoImpl play nicely with generic parametrs in hosted mode

2009-11-04 Thread spoon
LGTM. Going forward, this comment makes me nervous: /* * Handle virtual overrides by finding the method that we would * normally invoke and using its declaring class as the dispatch * target. */ In general, there are *multiple*

[gwt-contrib] allow prefetching of runAsync code

2009-11-10 Thread spoon
Reviewers: bobv, Message: Ready for review. Description: Add a prefetch queue to AsyncFragmentLoader. User code can add split points to the queue, and AsyncFragmentLoader will load the code for them whenever it has nothing else to do. Also updates the Showcase sample to prefetch code within

[gwt-contrib] Re: FixCompoundAssignmentNarrowing

2009-11-12 Thread spoon
http://gwt-code-reviews.appspot.com/102811/diff/1/3 File dev/core/src/com/google/gwt/dev/jjs/impl/CompoundAssignmentNormalizer.java (right): http://gwt-code-reviews.appspot.com/102811/diff/1/3#newcode409 Line 409: protected JExpression operationToReturn(JBinaryOperation op) { Also, I think you

[gwt-contrib] Re: FixCompoundAssignmentNarrowing

2009-11-12 Thread spoon
Thanks, Ray! A couple of things look like they should be changed, as indicated in the line-by-lines. Also, it should have a test. CompilerTest would be a decent place to put it. http://gwt-code-reviews.appspot.com/102811/diff/1/2 File

[gwt-contrib] Re: allow prefetching of runAsync code

2009-11-12 Thread spoon
[ERROR] One call is in com.google.gwt.sample.showcase.client.content.panels.CwAbsolutePanel.asyncOnInitialize (file:/Users/spoon/gwt/git/samples/showcase/src/com/google/gwt/sample/showcase/client/content/panels/CwAbsolutePanel.java:201) [ERROR] One call

[gwt-contrib] Re: Fix a nasty attribute parsing bug in UiBinder

2009-11-16 Thread spoon
Sure thing, looking now. And here I'd hoped never to have to think too much about XML namespaces. http://gwt-code-reviews.appspot.com/102818 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Fix a nasty attribute parsing bug in UiBinder

2009-11-16 Thread spoon
LGTM. http://gwt-code-reviews.appspot.com/102818 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: allow prefetching of runAsync code

2009-11-18 Thread spoon
Committed at r6992. http://gwt-code-reviews.appspot.com/102801 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Add deprecation warning to SoycDashboard

2009-11-23 Thread spoon
LGTM http://gwt-code-reviews.appspot.com/111803/diff/1/2 File dev/core/src/com/google/gwt/soyc/SoycDashboard.java (right): http://gwt-code-reviews.appspot.com/111803/diff/1/2#newcode74 Line 74: Thread.sleep(1000); One second is very short. Maybe 5? http://gwt-code-reviews.appspot.com/111803

[gwt-contrib] get RPC policy name from Java code

2009-12-01 Thread spoon
Reviewers: robertvawter_google.com, Description: Some GWT users want to build GWT RPC requests themselves, e.g. via building a raw HTML form. To do this, they need to supply the RPC policy name as part of the request, just like GWT RPC normally does. How shall they get the policy name? This

[gwt-contrib] Flow analysis framework definition and solver.

2009-12-01 Thread spoon
LGTM. Aside from documentation and naming, there is one question about which nodes are re-added to the worklist in the solver. Can you post a large enough set of code to include an actual analysis and optimization? I don't think we should commit any of this code until there it is causing at

[gwt-contrib] ExpressionAnalyzer fix for clinit field refs

2009-12-14 Thread spoon
LGTM http://gwt-code-reviews.appspot.com/124802/diff/1/2 File dev/core/src/com/google/gwt/dev/jjs/impl/ExpressionAnalyzer.java (right): http://gwt-code-reviews.appspot.com/124802/diff/1/2#newcode304 Line 304: private void recordMethodCall() { LG, but it's not really about method calls any

[gwt-contrib] Re: Control flow graph abstraction builder for CFG optimizations.

2009-12-15 Thread spoon
LGTM. But gee, Mike, what can the framework actually DO? http://gwt-code-reviews.appspot.com/117805/diff/2047/1046 File dev/core/src/com/google/gwt/dev/util/Either.java (right): http://gwt-code-reviews.appspot.com/117805/diff/2047/1046#newcode28 Line 28: throw new IllegalArgumentException();

[gwt-contrib] Arrays implement Serializable and Cloneable

2009-12-17 Thread spoon
Reviewers: bobv, Description: Arrays implement Serializable and Cloneable. See: http://code.google.com/p/google-web-toolkit/issues/detail?id=1822 Please review this at http://gwt-code-reviews.appspot.com/125803 Affected files: dev/core/src/com/google/gwt/dev/jjs/ast/JProgram.java

[gwt-contrib] Re: Fix two CompileReport XML issues

2009-12-18 Thread spoon
LGTM. If we work much more on this area, though, here's an alternative approach. Instead of working around Unicode obscurities, we could encode strings using a Java-like encoding. For example, 0 would become \u, and \ would become \\. Going that way, reading and writing these XML files

[gwt-contrib] Re: Arrays implement Serializable and Cloneable

2009-12-18 Thread spoon
http://gwt-code-reviews.appspot.com/125803/diff/1/3 File dev/core/src/com/google/gwt/dev/jjs/ast/JProgram.java (right): http://gwt-code-reviews.appspot.com/125803/diff/1/3#newcode480 Line 480: } Will do. http://gwt-code-reviews.appspot.com/125803/diff/1/3#newcode1032 Line 1032:

[gwt-contrib] SameParameterValueOptimizer needs a cast for numeric literals

2009-12-18 Thread spoon
Reviewers: bobv, Description: Otherwise, it can end up treating, for example, an integer as a long. Please review this at http://gwt-code-reviews.appspot.com/126814 Affected files: dev/core/src/com/google/gwt/dev/jjs/impl/SameParameterValueOptimizer.java

[gwt-contrib] Re: Arrays implement Serializable and Cloneable

2009-12-21 Thread spoon
On 2009/12/18 17:37:06, scottb wrote: http://gwt-code-reviews.appspot.com/125803/diff/1/3 File dev/core/src/com/google/gwt/dev/jjs/ast/JProgram.java (right): http://gwt-code-reviews.appspot.com/125803/diff/1/3#newcode1032 Line 1032: allArrayTypes.add(arrayType); LGTM then. Maybe we could

[gwt-contrib] JSNI references with ?? as the parameter list

2009-12-22 Thread spoon
Reviewers: bobv, Description: This patch allows JSNI references to non-overloaded methods to use ?? as the parameter list. This saves a typing for the most common references to methods. Implementation notes: The best place to look first is JavaAccessFromJavaScriptTest.java . This adds tests

  1   2   3   4   5   6   >