%X => %_X, %_X => %__X

%X => %_X, %_X => %__X

R=mstarzinger@chromium.org
BUG=

[Michael Starzinger, 2013-01-31 15:04:53]

First round of comments. There are missing adaptations in macros.py because the
IS_VAR macro is not actually expanded but is a runtime intrinsic.

https://chromiumcodereview.appspot.com/12095035/diff/1013/src/parser.cc
File src/parser.cc (right):

https://chromiumcodereview.appspot.com/12095035/diff/1013/src/parser.cc#newco...
src/parser.cc:4625: // %IS_VAR(x) evaluates to x if x is a variable,
Comment about IS_VAR needs adaptation as well.

https://chromiumcodereview.appspot.com/12095035/diff/1013/src/parser.cc#newco...
src/parser.cc:4654: name = isolate()->factory()->NewSubString(name, 1,
name->length());
This essentially merge the "%foo" and "%_foo" namespace at the moment, which is
probably not what we want. Instead the "%foo" namespace should be checked
against a white-list of allowed API-instrinsics. That white-list is empty for
now.

Also the AST node generated at the end of this function needs to know the
distinction between API-intrinsics and runtime-intrinsics, because otherwise
that distinction would be lost by the name mangling.

https://chromiumcodereview.appspot.com/12095035/diff/1013/test/mjsunit/regres...
File test/mjsunit/regress/regress-2286.js (right):

https://chromiumcodereview.appspot.com/12095035/diff/1013/test/mjsunit/regres...
test/mjsunit/regress/regress-2286.js:31: assertThrows("%_f()", TypeError);
We should add a test that a call of "%f()" still throws a TypeError.

[drcarney, 2013-01-31 15:20:47]

the IS_VAR thing is handled everywhere automatically

https://chromiumcodereview.appspot.com/12095035/diff/1013/src/parser.cc
File src/parser.cc (right):

https://chromiumcodereview.appspot.com/12095035/diff/1013/src/parser.cc#newco...
src/parser.cc:4625: // %IS_VAR(x) evaluates to x if x is a variable,
On 2013/01/31 15:04:53, Michael Starzinger wrote:
> Comment about IS_VAR needs adaptation as well.

This gets handled by the automatic replace

https://chromiumcodereview.appspot.com/12095035/diff/1013/src/parser.cc#newco...
src/parser.cc:4654: name = isolate()->factory()->NewSubString(name, 1,
name->length());
On 2013/01/31 15:04:53, Michael Starzinger wrote:
> This essentially merge the "%foo" and "%_foo" namespace at the moment, which
is
> probably not what we want. Instead the "%foo" namespace should be checked
> against a white-list of allowed API-instrinsics. That white-list is empty for
> now.
> 
> Also the AST node generated at the end of this function needs to know the
> distinction between API-intrinsics and runtime-intrinsics, because otherwise
> that distinction would be lost by the name mangling.

Yes, I was planning this for the patch that actually introduces the public
intrinsics.