added --stress-opt option to d8

added --stress-opt option to d8

Committed: http://code.google.com/p/v8/source/detail?r=8241

[Yang, 2011-06-09 09:50:10]

please take a look

[Søren Thygesen Gjesse, 2011-06-09 10:05:48]

This change contains a lot of reformatting - automatically done by Eclipse I
suppose. Some of it does not adhere to the style guide, and the rest clutters
the change.

Please revert the reformatting so the real change stands out.

If you think some of the style changes are needed please do that as a separate
change.

http://codereview.chromium.org/7134040/diff/1/src/d8.cc
File src/d8.cc (left):

http://codereview.chromium.org/7134040/diff/1/src/d8.cc#oldcode161
src/d8.cc:161: 
We always try to keep two blank lines between functions.

http://codereview.chromium.org/7134040/diff/1/src/d8.h
File src/d8.h (right):

http://codereview.chromium.org/7134040/diff/1/src/d8.h#newcode41
src/d8.h:41: public:
One space indent for public/private/protected (several times below), see
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Class_....

http://codereview.chromium.org/7134040/diff/1/src/d8.h#newcode44
src/d8.h:44: int32_t* ptr() {
Why did you re-format all these accessors?

http://codereview.chromium.org/7134040/diff/1/src/d8.h#newcode82
src/d8.h:82: CounterMap() :
If you want to format this on several lines the ':' needs to be on the next line
with a four space indent. See
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Constr...

http://codereview.chromium.org/7134040/diff/1/src/d8.h#newcode128
src/d8.h:128: static bool ExecuteString(Handle<String> source, Handle<Value>
name,
I think you should keep the original formatting of the function arguments, see
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Functi....

[Yang, 2011-06-09 11:19:15]

I reformatted the whole thing. Should be fine now. In the meantime I'll take a
look at the style guide.

[Søren Thygesen Gjesse, 2011-06-09 12:36:04]

LGTM with comments addressed

http://codereview.chromium.org/7134040/diff/1003/src/d8.cc
File src/d8.cc (left):

http://codereview.chromium.org/7134040/diff/1003/src/d8.cc#oldcode619
src/d8.cc:619: // Mark the d8 shell script as native to avoid it showing up as
normal source
Is this last part of the installation of the utility script not missing in the
refactored version?

http://codereview.chromium.org/7134040/diff/1003/src/d8.cc
File src/d8.cc (right):

http://codereview.chromium.org/7134040/diff/1003/src/d8.cc#newcode596
src/d8.cc:596: 
Can the adding of the "os" object be part of the CreateGlobalTemplate?

http://codereview.chromium.org/7134040/diff/1003/src/d8.cc#newcode604
src/d8.cc:604: 
Also can adding the arguments be part of CreateGlobalTemplate? I know that they
would then also be available in the utility context, but that will probably be
ok.

http://codereview.chromium.org/7134040/diff/1003/src/d8.h
File src/d8.h (right):

http://codereview.chromium.org/7134040/diff/1003/src/d8.h#newcode132
src/d8.h:132: static void Initialize();
Please rename to RenewEvaluationContext as using the name "eval" might cause
confusion with the JavaScript eval function.

http://codereview.chromium.org/7134040/diff/1003/src/d8.h#newcode133
src/d8.h:133: static void RenewEvalContext();
Rename Util ->Utility

[Yang, 2011-06-09 13:00:40]

I included your suggestions. Including those JS arguments into
CreateGlobalTemplate leads to crashes so I didn't do this. So is this good for
commit?

http://codereview.chromium.org/7134040/diff/1003/src/d8.cc
File src/d8.cc (left):

http://codereview.chromium.org/7134040/diff/1003/src/d8.cc#oldcode619
src/d8.cc:619: // Mark the d8 shell script as native to avoid it showing up as
normal source
On 2011/06/09 12:36:04, Søren Gjesse wrote:
> Is this last part of the installation of the utility script not missing in the
> refactored version?

Indeed. Must have gotten lost when I moved the code around.

http://codereview.chromium.org/7134040/diff/1003/src/d8.cc
File src/d8.cc (right):

http://codereview.chromium.org/7134040/diff/1003/src/d8.cc#newcode596
src/d8.cc:596: 
On 2011/06/09 12:36:04, Søren Gjesse wrote:
> Can the adding of the "os" object be part of the CreateGlobalTemplate?

yes

http://codereview.chromium.org/7134040/diff/1003/src/d8.cc#newcode604
src/d8.cc:604: 
On 2011/06/09 12:36:04, Søren Gjesse wrote:
> Also can adding the arguments be part of CreateGlobalTemplate? I know that
they
> would then also be available in the utility context, but that will probably be
> ok.

I tried that. this would lead to crashes somehow.

http://codereview.chromium.org/7134040/diff/1003/src/d8.h
File src/d8.h (right):

http://codereview.chromium.org/7134040/diff/1003/src/d8.h#newcode132
src/d8.h:132: static void Initialize();
On 2011/06/09 12:36:04, Søren Gjesse wrote:
> Please rename to RenewEvaluationContext as using the name "eval" might cause
> confusion with the JavaScript eval function.

Done.

http://codereview.chromium.org/7134040/diff/1003/src/d8.h#newcode133
src/d8.h:133: static void RenewEvalContext();
On 2011/06/09 12:36:04, Søren Gjesse wrote:
> Rename Util ->Utility

Done.

[Søren Thygesen Gjesse, 2011-06-09 13:16:18]

LGTM

Please go ahead an commit when the accidental indentation is reverted.

http://codereview.chromium.org/7134040/diff/1004/src/d8.cc
File src/d8.cc (right):

http://codereview.chromium.org/7134040/diff/1004/src/d8.cc#newcode619
src/d8.cc:619: for (int j = 0; j < js_args.argc(); j++) {
Something happened to the intention here.

http://codereview.chromium.org/7134040/diff/1004/src/d8.cc#newcode624
src/d8.cc:624: i::Handle<i::JSArray> arguments_jsarray =
Ditto.

http://codereview.chromium.org/7134040/diff/1004/src/d8.cc#newcode626
src/d8.cc:626: global_template->Set(String::New("arguments"),
Ditto.