370400 - Rhino should support const keyword

Status: RESOLVED FIXED

(Rhino Graveyard :: Core, defect)

Description

[Bob Jervis]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.7) Gecko/20060926 Ubuntu/breezy-security Firefox/1.5.0.7
Build Identifier: Rhino 1.6R5

Another JS 1.5 feature that RHino needs.

Reproducible: Always

Steps to Reproduce:
1. Start Rhino shell
2. enter 'const foo = 5;'
3.
Actual Results:  
Parse error.

Expected Results:  
Should work and protect foo from assignment.

Comment 1

[Bob Jervis]

I'm working on a fix for this.

Comment 2

[Bob Jervis]

Attachment: This is a PARTIAL patch to this bug.

I will lay odds that this patch will crash if opt level is > -1.  Do not integrate this patch.  I just want comments on the semantics and the changes in interfaces.

Comment 3

[Brendan Eich [:brendan]]

FYI, bug 229756 reports the lack of write-once semantics in SpiderMonkey's const.

/be

Comment 4

[Bob Jervis]

Attachment: This is a full patch

This completes the implementation of const.  There are a couple of validation tests that were failing always before (because they used const in them).  These tests (js1_5/String/regress-354541-02.js and js1_5/String/regress-354541-04.js) now fail only on -opt -1 but pass on opt >= 0.  Modifying the source to use 'var' instead of const exhibits the same behavior even without my changes.  I therefore conclude that these failures are unrelated to my changes.

Comment 5

[Bob Jervis]

Attachment: ConstProperties.java

The diff stuff didn't includes this new file (it goes in src/org/mozilla/javascript).  Sorry I missed this earlier.

Comment 6

[Brendan Eich [:brendan]]

Bob, please see comment 4.

/be

Comment 7

[Bob Clary [:bc] (inactive)]

The referenced tests are now in js1_5/extensions/ due to the use of Script. Does Rhino even support Script? Brendan, I can change the use of const in the tests to var if that would fix the problem.

Comment 8

[Bob Jervis]

Changing const to var does not change the test outcome (I tried that to isolate the cause).  Rhino does support Script, but these tests only work correctly on the compiled-thru-Java cde path.  The stand-alone interpreter gets theanswer wrong.  I traced the interpreted code path thru to the Script constructor and it appeared that it used the global scope as it's scope, and not the local scope of the caller (which is what the tests assume).

Comment 9

[Bob Jervis]

I just confirmed a bug in the existing patches.  For compiled code path, assigning to const declared variables at function scope does not seem to work.  The compile itself will crash in org.mozilla.javascript.optimizer.BodyCodegen.generateExpression with an Unexpected node type 151 error.  I'm working on an updated patch that fixes this case.

Comment 10

[Bob Jervis]

Attachment: Updated const patch that fixes problems with const in functions.

This patch fixes the previously noted bug in the original patches.

Comment 11

[Norris Boyd]

Checked in patch:

RCS file: /cvsroot/mozilla/js/rhino/src/org/mozilla/javascript/ConstProperties.java,v
done
Checking in ConstProperties.java;
/cvsroot/mozilla/js/rhino/src/org/mozilla/javascript/ConstProperties.java,v  <--  ConstProperties.java
initial revision: 1.1
done
Checking in Decompiler.java;
/cvsroot/mozilla/js/rhino/src/org/mozilla/javascript/Decompiler.java,v  <--  Decompiler.java
new revision: 1.23; previous revision: 1.22
done
Checking in IRFactory.java;
/cvsroot/mozilla/js/rhino/src/org/mozilla/javascript/IRFactory.java,v  <--  IRFactory.java
new revision: 1.103; previous revision: 1.102
done
Checking in InterpretedFunction.java;
/cvsroot/mozilla/js/rhino/src/org/mozilla/javascript/InterpretedFunction.java,v  <--  InterpretedFunction.java
new revision: 1.54; previous revision: 1.53
done
Checking in Interpreter.java;
/cvsroot/mozilla/js/rhino/src/org/mozilla/javascript/Interpreter.java,v  <--  Interpreter.java
new revision: 1.320; previous revision: 1.319
done
Checking in InterpreterData.java;
/cvsroot/mozilla/js/rhino/src/org/mozilla/javascript/InterpreterData.java,v  <--  InterpreterData.java
new revision: 1.55; previous revision: 1.54
done
Checking in NativeCall.java;
/cvsroot/mozilla/js/rhino/src/org/mozilla/javascript/NativeCall.java,v  <--  NativeCall.java
new revision: 1.35; previous revision: 1.34
done
Checking in NativeFunction.java;
/cvsroot/mozilla/js/rhino/src/org/mozilla/javascript/NativeFunction.java,v  <--  NativeFunction.java
new revision: 1.64; previous revision: 1.63
done
Checking in NodeTransformer.java;
/cvsroot/mozilla/js/rhino/src/org/mozilla/javascript/NodeTransformer.java,v  <--  NodeTransformer.java
new revision: 1.69; previous revision: 1.68
done
Checking in Parser.java;
/cvsroot/mozilla/js/rhino/src/org/mozilla/javascript/Parser.java,v  <--  Parser.java
new revision: 1.110; previous revision: 1.109
done
Checking in ScriptOrFnNode.java;
/cvsroot/mozilla/js/rhino/src/org/mozilla/javascript/ScriptOrFnNode.java,v  <--  ScriptOrFnNode.java
new revision: 1.20; previous revision: 1.19
done
Checking in ScriptRuntime.java;
/cvsroot/mozilla/js/rhino/src/org/mozilla/javascript/ScriptRuntime.java,v  <-- ScriptRuntime.java
new revision: 1.259; previous revision: 1.258
done
Checking in ScriptableObject.java;
/cvsroot/mozilla/js/rhino/src/org/mozilla/javascript/ScriptableObject.java,v  <--  ScriptableObject.java
new revision: 1.119; previous revision: 1.118
done
Checking in Token.java;
/cvsroot/mozilla/js/rhino/src/org/mozilla/javascript/Token.java,v  <--  Token.java
new revision: 1.38; previous revision: 1.37
done
Checking in TokenStream.java;
/cvsroot/mozilla/js/rhino/src/org/mozilla/javascript/TokenStream.java,v  <--  TokenStream.java
new revision: 1.66; previous revision: 1.65
done
Checking in optimizer/Codegen.java;
/cvsroot/mozilla/js/rhino/src/org/mozilla/javascript/optimizer/Codegen.java,v  <--  Codegen.java
new revision: 1.245; previous revision: 1.244
done
Checking in optimizer/OptFunctionNode.java;
/cvsroot/mozilla/js/rhino/src/org/mozilla/javascript/optimizer/OptFunctionNode.java,v  <--  OptFunctionNode.java
new revision: 1.33; previous revision: 1.32
done
Checking in resources/Messages.properties;
/cvsroot/mozilla/js/rhino/src/org/mozilla/javascript/resources/Messages.properties,v  <--  Messages.properties
new revision: 1.68; previous revision: 1.67
done

Comment 12

[Norris Boyd]

Adding target milestone of 1.6R6 based on the date this bug was resolved FIXED.