Roberto Lublinerman has submitted this change and it was merged.

Change subject: Unary plus in Jsni code was always discarded.
......................................................................


Unary plus in Jsni code was always discarded.

Unary plus in JavaScript is only noop when applied to a
numeric value otherwise it behaves as a conversion to double
operator. The compiler incorrectly assumed that is was a noop
in all cases.

The fix was already made in r5960 by cromwellian but was lost
in a subsequent rollback (r5961).

Fixes issue 6373 and issue 3942.

Change-Id: I4d260d7fdc08b942f44b0275fb1c842be1c807fd
---
M dev/core/src/com/google/gwt/dev/js/JsParser.java
M dev/core/src/com/google/gwt/dev/js/JsToStringGenerationVisitor.java
M dev/core/test/com/google/gwt/dev/js/JsToStringGenerationVisitorAccuracyTest.java
M user/test/com/google/gwt/dev/jjs/CompilerSuite.java
A user/test/com/google/gwt/dev/jjs/test/CompilerMiscRegressionTest.java
5 files changed, 82 insertions(+), 13 deletions(-)

Approvals:
  Ray Cromwell: Looks good to me, approved
  Leeroy Jenkins: Verified



diff --git a/dev/core/src/com/google/gwt/dev/js/JsParser.java b/dev/core/src/com/google/gwt/dev/js/JsParser.java
index 2a7b218..a4e35dd 100644
--- a/dev/core/src/com/google/gwt/dev/js/JsParser.java
+++ b/dev/core/src/com/google/gwt/dev/js/JsParser.java
@@ -1,12 +1,12 @@
 /*
  * 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
@@ -1204,8 +1204,13 @@
         return mapPrefixOperation(JsUnaryOperator.TYPEOF, unOp);

       case TokenStream.ADD:
-        // Pretend we didn't see it.
-        return mapExpression(unOp.getFirstChild());
+        if (unOp.getFirstChild().getType() != TokenStream.NUMBER) {
+          return mapPrefixOperation(JsUnaryOperator.POS, unOp);
+        } else {
+          // Pretend we didn't see it.
+          return mapExpression(unOp.getFirstChild());
+        }
+
       case TokenStream.VOID:
         return mapPrefixOperation(JsUnaryOperator.VOID, unOp);

diff --git a/dev/core/src/com/google/gwt/dev/js/JsToStringGenerationVisitor.java b/dev/core/src/com/google/gwt/dev/js/JsToStringGenerationVisitor.java
index ed1f569..55fab01 100644
--- a/dev/core/src/com/google/gwt/dev/js/JsToStringGenerationVisitor.java
+++ b/dev/core/src/com/google/gwt/dev/js/JsToStringGenerationVisitor.java
@@ -1,12 +1,12 @@
 /*
  * 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
@@ -1340,7 +1340,7 @@
   /**
* Decide whether, if <code>op</code> is printed followed by <code>arg</code>,
    * there needs to be a space between the operator and expression.
-   *
+   *
    * @return <code>true</code> if a space needs to be printed
    */
   private boolean _spaceCalc(JsOperator op, JsExpression arg) {
@@ -1362,7 +1362,8 @@
       JsOperator op2 = ((JsPrefixOperation) arg).getOperator();
       return (op == JsBinaryOperator.SUB || op == JsUnaryOperator.NEG)
           && (op2 == JsUnaryOperator.DEC || op2 == JsUnaryOperator.NEG)
-          || (op == JsBinaryOperator.ADD && op2 == JsUnaryOperator.INC);
+          || (op == JsBinaryOperator.ADD || op == JsUnaryOperator.POS)
+          && (op2 == JsUnaryOperator.INC || op2 == JsUnaryOperator.POS);
     }
     if (arg instanceof JsNumberLiteral) {
       JsNumberLiteral literal = (JsNumberLiteral) arg;
diff --git a/dev/core/test/com/google/gwt/dev/js/JsToStringGenerationVisitorAccuracyTest.java b/dev/core/test/com/google/gwt/dev/js/JsToStringGenerationVisitorAccuracyTest.java
index 5a99ddd..117ead1 100644
--- a/dev/core/test/com/google/gwt/dev/js/JsToStringGenerationVisitorAccuracyTest.java +++ b/dev/core/test/com/google/gwt/dev/js/JsToStringGenerationVisitorAccuracyTest.java
@@ -140,6 +140,12 @@
     doTest("var x = +y", "var x = +y");
     // + prefix stripped when operand is literal number
     doTest("var x = +42", "var x = 42");
+    // + prefix not stripped when operand is not literal number
+    doTest("var x = +y", "var x = +y");
+    // + prefix stripped when operand is literal number
+    doTest("var x = +42","var x = 42");
+    // + <blank> + should not become ++
+    doTest("var x = 10+ +\"2\"", "var x = 10+ +\"2\"");
   }

   public void testEscapes() {
diff --git a/user/test/com/google/gwt/dev/jjs/CompilerSuite.java b/user/test/com/google/gwt/dev/jjs/CompilerSuite.java
index 40a30df..1e35dd0 100644
--- a/user/test/com/google/gwt/dev/jjs/CompilerSuite.java
+++ b/user/test/com/google/gwt/dev/jjs/CompilerSuite.java
@@ -1,12 +1,12 @@
 /*
  * 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
@@ -23,6 +23,7 @@
 import com.google.gwt.dev.jjs.test.ClassCastTest;
 import com.google.gwt.dev.jjs.test.ClassObjectTest;
 import com.google.gwt.dev.jjs.test.CompilerTest;
+import com.google.gwt.dev.jjs.test.CompilerMiscRegressionTest;
 import com.google.gwt.dev.jjs.test.CoverageTest;
 import com.google.gwt.dev.jjs.test.EnhancedForLoopTest;
 import com.google.gwt.dev.jjs.test.EnumsTest;
@@ -72,6 +73,7 @@
     suite.addTestSuite(ClassCastTest.class);
     suite.addTestSuite(ClassObjectTest.class);
     suite.addTestSuite(CompilerTest.class);
+    suite.addTestSuite(CompilerMiscRegressionTest.class);
     suite.addTestSuite(CoverageTest.class);
     suite.addTestSuite(EnhancedForLoopTest.class);
     suite.addTestSuite(EnumsTest.class);
diff --git a/user/test/com/google/gwt/dev/jjs/test/CompilerMiscRegressionTest.java b/user/test/com/google/gwt/dev/jjs/test/CompilerMiscRegressionTest.java
new file mode 100644
index 0000000..841eec3
--- /dev/null
+++ b/user/test/com/google/gwt/dev/jjs/test/CompilerMiscRegressionTest.java
@@ -0,0 +1,55 @@
+/*
+ * Copyright 2013 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.google.gwt.dev.jjs.test;
+
+import com.google.gwt.junit.client.GWTTestCase;
+
+/**
+ * Tests Miscelaneous fixes.
+ */
+public class CompilerMiscRegressionTest extends GWTTestCase {
+
+  @Override
+  public String getModuleName() {
+    return "com.google.gwt.dev.jjs.CompilerSuite";
+  }
+
+  native double toNumber(String value) /*-{
+    return +value;
+  }-*/;
+
+  native double addAndConvert(double v1, String v2) /*-{
+    return v1 + +v2;
+  }-*/;
+
+  native double minusAndDecrement(double val) /*-{
+    var lhs = val;
+    return - --lhs;
+  }-*/;
+
+  /**
+   * Test for issues 6373 and 3942.
+   */
+  public void testUnaryPlus() {
+    // With the unary + operator stripped the first assertion only fails in
+    // dev mode, in web mode the  comparison made by assertEquals masks
+    // the error; whereas the second fails in both dev and web modes.
+    assertEquals(11.0, toNumber("11"));
+    assertEquals(12.0, toNumber("10") + toNumber("2"));
+    assertEquals(12.0, addAndConvert(10, "2"));
+    assertEquals(-10.0, minusAndDecrement(11));
+  }
+}

--
To view, visit https://gwt-review.googlesource.com/2990
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I4d260d7fdc08b942f44b0275fb1c842be1c807fd
Gerrit-PatchSet: 2
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Roberto Lublinerman <rlu...@google.com>
Gerrit-Reviewer: Daniel Kurka <danku...@google.com>
Gerrit-Reviewer: Leeroy Jenkins <jenk...@gwtproject.org>
Gerrit-Reviewer: Matthew Dempsky <mdemp...@google.com>
Gerrit-Reviewer: Ray Cromwell <cromwell...@google.com>
Gerrit-Reviewer: Roberto Lublinerman <rlu...@google.com>

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- You received this message because you are subscribed to the Google Groups "GWT Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.


Reply via email to