[gwt-contrib] Re: Fixes a bug where temp local declarations can sometimes end up in a for statement's increments list. (issue677801)

[bowdidge]

http://gwt-code-reviews.appspot.com/677801/diff/1/9
File dev/core/test/com/google/gwt/dev/jjs/impl/TempLocalVisitorTest.java
(right):

http://gwt-code-reviews.appspot.com/677801/diff/1/9#newcode87
dev/core/test/com/google/gwt/dev/jjs/impl/TempLocalVisitorTest.java:87:
public void testForStatement() throws Exception {
Should you add a test with a loop over longs because of all the extra
manipulation that goes on there?

Think it's likely that anyone's going to try to increment a long in the
test clause of a for statement?

http://gwt-code-reviews.appspot.com/677801/show

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Fixes a bug where temp local declarations can sometimes end up in a for statement's increments list. (issue677801)

[bowdidge]

LGTM.


http://gwt-code-reviews.appspot.com/677801/diff/1/9
File dev/core/test/com/google/gwt/dev/jjs/impl/TempLocalVisitorTest.java
(right):

http://gwt-code-reviews.appspot.com/677801/diff/1/9#newcode87
dev/core/test/com/google/gwt/dev/jjs/impl/TempLocalVisitorTest.java:87:
public void testForStatement() throws Exception {
Sounds ok.

http://gwt-code-reviews.appspot.com/677801/show

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Add useful hooks into GWT to allow other tools to parse and analyze Java code. (issue631801)

[bowdidge]

Reviewers: Lex,

Description:
Add useful hooks into GWT to allow other tools to parse and analyze Java
code.

Please review this at http://gwt-code-reviews.appspot.com/631801/show

Affected files:
  M dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java
  M dev/core/src/com/google/gwt/dev/javac/JdtCompiler.java
  M dev/core/src/com/google/gwt/dev/jjs/UnifiedAst.java
  A  
dev/core/test/com/google/gwt/dev/jjs/impl/AdditionalTypeProviderDelegateTest.java

  M dev/core/test/com/google/gwt/dev/jjs/impl/JJSTestBase.java


--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Replace calls to printInt(num) with printInt(num, 10) to make sure

[bowdidge]

It's completely a style thing.  Closure Compiler's externs files only
allow printInt to take two parameters.  This code wouldn't be able to
trigger the incorrect behavior as far as I know.


http://gwt-code-reviews.appspot.com/225802

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Replace calls to printInt(num) with printInt(num, 10) to make sure

[Robert Bowdidge]

On Mar 17, 2010, at 1:54 PM, j...@google.com wrote:
It's completely a style thing.  Closure Compiler's externs files  
only allow
printInt to take two parameters.  This code wouldn't be able to  
trigger the

incorrect behavior as far as I know.


Ok, LGTM, though perhaps file a bug against Closure Compiler to  
remove a

restriction that may not be accurate.


The restriction's accurate - not providing a base can be buggy, and  
has different behavior depending on the browser (such as whether 08 is  
8 or 0). Calling parseInt() without a base is banned in Google's  
JavaScript, and similarly counts as an error in Closure Compiler.  See  
the es3.js externs file for the words strictly banned:


http://code.google.com/p/closure-compiler/source/browse/trunk/externs/es3.js

and this thread on the Closure Compiler open source list for another  
one of the discussions on the topic.


http://code.google.com/p/closure-compiler/issues/detail?id=111

Robert

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors