LGTM
http://codereview.chromium.org/40322
--~--~-~--~~~---~--~~
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
-~--~~~~--~~--~--~---
Reviewers: Kasper Lund, William Hesse,
Message:
Bill, feel free to add back another test case for this if this is not
sufficient. The old test case took far too long to run.
Description:
Simplify the map collection regression test. This test is the minimal
example that would trigger the bug.
LGTM.
http://codereview.chromium.org/43004
--~--~-~--~~~---~--~~
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
-~--~~~~--~~--~--~---
LGTM.
http://codereview.chromium.org/40300/diff/1/2
File src/regexp-delay.js (right):
http://codereview.chromium.org/40300/diff/1/2#newcode304
Line 304:
Was the comment wrong, or has Firefox changed to 1?
http://codereview.chromium.org/40300/diff/1/3
File src/uri.js (right):
http://codereview
Author: a...@chromium.org
Date: Tue Mar 10 00:23:22 2009
New Revision: 1467
Modified:
branches/bleeding_edge/test/mjsunit/regress/regress-1493017.js
Log:
Simplify the map collection regression test. This test is the minimal
example that would trigger the bug.
Review URL: http://codereview.c
Status: Accepted
Owner: l...@chromium.org
Labels: Type-Bug Priority-Medium
New issue 267 by l...@chromium.org: Indirect eval fails in eval-tainted
scope.
http://code.google.com/p/v8/issues/detail?id=267
The script
var foo = "outer";
(function(fn){var foo="inner"; return [fn("foo")];})(eval
http://codereview.chromium.org/40300/diff/1/2
File src/regexp-delay.js (right):
http://codereview.chromium.org/40300/diff/1/2#newcode304
Line 304:
The comment was wrong in the sense that the spec does say something
about the length of exec and test. I don't anticipate any compatibility
issues.
LGTM
http://codereview.chromium.org/40317/diff/1/14
File src/debug-delay.js (right):
http://codereview.chromium.org/40317/diff/1/14#newcode1019
Line 1019: };
Is indentation off by one here?
http://codereview.chromium.org/40317
--~--~-~--~~~---~--~~
v8-dev maili
http://codereview.chromium.org/40317/diff/1/14
File src/debug-delay.js (right):
http://codereview.chromium.org/40317/diff/1/14#newcode1019
Line 1019: };
On 2009/03/10 07:53:14, Mads Ager wrote:
> Is indentation off by one here?
Done.
http://codereview.chromium.org/40317
--~--~-~--~---
Author: sgje...@chromium.org
Date: Tue Mar 10 01:10:50 2009
New Revision: 1468
Modified:
branches/bleeding_edge/include/v8.h
branches/bleeding_edge/src/accessors.cc
branches/bleeding_edge/src/accessors.h
branches/bleeding_edge/src/api.cc
branches/bleeding_edge/src/bootstrapper
Overall, this looks good. I agree with Kasper on the timing. We need
to get the new compiler pushed to trunk before landing this in
bleeding_edge. Once we land this in bleeding_edge, we need to make sure
that the real hiding of the property happens fairly quickly so we are
not blocked from push
It dawned on me that I needed to make the change for ARM as well, so
please take another look. Specific comments addressed below.
http://codereview.chromium.org/40295/diff/1/3
File src/ast.h (right):
http://codereview.chromium.org/40295/diff/1/3#newcode667
Line 667: CONSTANT, // Pro
Reviewers: Lasse Reichstein,
Description:
Fix probably harmless thinko in StringCharAt (causes
slow-case code to be run).
Please review this at http://codereview.chromium.org/43005
SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/
Affected files:
M src/string.js
Index: sr
LGTM.
http://codereview.chromium.org/40295
--~--~-~--~~~---~--~~
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
-~--~~~~--~~--~--~---
LGTM, if this is what you want.
http://codereview.chromium.org/43005/diff/1/2
File src/string.js (right):
http://codereview.chromium.org/43005/diff/1/2#newcode65
Line 65: var subject = ToString(this);
This changes the behavior on non-string receivers to potentially use the
fast case, but at the
Status: Accepted
Owner:
Labels: Type-Bug Priority-High
New issue 268 by christian.plesner.hansen: Valgrind reports leaks in code
generator
http://code.google.com/p/v8/issues/detail?id=268
Running
valgrind --leak-check=full --smc-check=all ./shell_g
../test/mjsunit/{mjsunit,bugs/bug-90
http://codereview.chromium.org/43005/diff/1/2
File src/string.js (right):
http://codereview.chromium.org/43005/diff/1/2#newcode65
Line 65: var subject = ToString(this);
My bad. The real problem was ofcourse that subject was used before it
was initialized.
http://codereview.chromium.org/43005
-
http://codereview.chromium.org/43005
--~--~-~--~~~---~--~~
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
-~--~~~~--~~--~--~---
Reviewers: Christian Plesner Hansen,
Description:
Fixed whitespace.
Please review this at http://codereview.chromium.org/43006
SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/
Affected files:
M src/parser.h
Index: src/parser.h
Lgtm
http://codereview.chromium.org/43006
--~--~-~--~~~---~--~~
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
-~--~~~~--~~--~--~---
Updates:
Status: Fixed
Comment #1 on issue 244 by christian.plesner.hansen: Uri encode/decode
don't handle malformed sequences correctly
http://code.google.com/p/v8/issues/detail?id=244
(No comment was entered for this change.)
--
You received this message because you are listed in th
Updates:
Owner: kmilli...@chromium.org
Comment #1 on issue 268 by kmilli...@chromium.org: Valgrind reports leaks
in code generator
http://code.google.com/p/v8/issues/detail?id=268
(No comment was entered for this change.)
--
You received this message because you are listed in the owne
This changelist has moved from experimental compiler branch to
bleeding-edge, with a new issue number,
http://codereview.chromium.org/42006.
http://codereview.chromium.org/21538
--~--~-~--~~~---~--~~
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google
Author: christian.plesner.han...@gmail.com
Date: Tue Mar 10 02:08:05 2009
New Revision: 1471
Added:
branches/bleeding_edge/test/mjsunit/regress/regress-244.js
Modified:
branches/bleeding_edge/src/regexp-delay.js
branches/bleeding_edge/src/uri.js
Log:
Implemented invalid UTF8 detectio
Reviewers: Kevin Millikin, iposva,
Message:
This changelist is a continuation of changelist
http://codereview.chromium.org/21538,
which is moved from the experimental compiler branch
to bleeding-edge to make this new issue.
This change has been reviewed extensively under the old number, and
pass
Author: olehougaard
Date: Tue Mar 10 02:30:10 2009
New Revision: 1472
Modified:
branches/bleeding_edge/src/parser.h
Log:
Fixed whitespace.
Review URL: http://codereview.chromium.org/43006
Modified: branches/bleeding_edge/src/parser.h
=
Author: olehougaard
Date: Tue Mar 10 01:59:00 2009
New Revision: 1469
Modified:
branches/bleeding_edge/src/ast.cc
branches/bleeding_edge/src/ast.h
branches/bleeding_edge/src/codegen-arm.cc
branches/bleeding_edge/src/codegen-ia32.cc
branches/bleeding_edge/src/heap.h
branche
Author: erik.co...@gmail.com
Date: Tue Mar 10 02:00:04 2009
New Revision: 1470
Modified:
branches/bleeding_edge/src/string.js
Log:
Fix probably harmless thinko in StringCharAt (causes
slow-case code to be run).
Review URL: http://codereview.chromium.org/43005
Modified: branches/bleeding_edg
Looks good. A few comments.
http://codereview.chromium.org/42006/diff/1/3
File src/codegen-ia32.cc (right):
http://codereview.chromium.org/42006/diff/1/3#newcode686
Line 686: // should be inlined, placed in the stub, or omitted entirely.
To parallel the order of the enum, "should be placed in
Reviewers: Mads Ager,
Message:
Should I remove Factory::NewNumberFromUint as well? It is not used
anymore.
Description:
Moved the storage of the last script id into the heap to make it part of
the serialized data when starting V8 on a snapshot.
Currently the script ids wrap when positive smi va
Reviewers: Lasse Reichstein,
Description:
Made the Error prototype into an error. Allow \c at the end of
regexps. Throw a type error when calling regexp methods on
non-regexps.
Please review this at http://codereview.chromium.org/42007
Affected files:
M src/messages.js
M src/parser.cc
LGTM
Yes, please remove the dead code as well.
http://codereview.chromium.org/43008
--~--~-~--~~~---~--~~
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
-~--~~~~--~~--~--~---
LGTM
http://codereview.chromium.org/42007
--~--~-~--~~~---~--~~
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
-~--~~~~--~~--~--~---
Author: christian.plesner.han...@gmail.com
Date: Tue Mar 10 04:32:19 2009
New Revision: 1473
Modified:
branches/bleeding_edge/src/messages.js
branches/bleeding_edge/src/parser.cc
branches/bleeding_edge/src/regexp-delay.js
branches/bleeding_edge/test/cctest/test-regexp.cc
Log:
Mad
买便宜五金请到《青远五金网 》
--~--~-~--~~~---~--~~
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
-~--~~~~--~~--~--~---
Reviewers: Kasper Lund,
Message:
Part I.
Description:
Introduce a BreakTarget subclass of JumpTarget used to represent the
blocks labeled by "break", "continue", and "return". BreakTargets are
the only jump targets that appear in the AST, the only ones that can
be uninitialized, and the only on
Reviewers: Mads Ager,
Message:
Codereview, please.
Description:
Issue 269: Calls to arguments in eval-tainted function scope uses global
object as receiver.
Please review this at http://codereview.chromium.org/42009
Affected files:
M src/ast.h
M src/codegen-arm.cc
M src/codegen-ia32.c
Author: sgje...@chromium.org
Date: Tue Mar 10 05:05:20 2009
New Revision: 1474
Modified:
branches/bleeding_edge/src/factory.cc
branches/bleeding_edge/src/heap-inl.h
branches/bleeding_edge/src/heap.cc
branches/bleeding_edge/src/heap.h
branches/bleeding_edge/test/mjsunit/debug-s
LGTM.
http://codereview.chromium.org/42008
--~--~-~--~~~---~--~~
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
-~--~~~~--~~--~--~---
LGTM
http://codereview.chromium.org/42009
--~--~-~--~~~---~--~~
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
-~--~~~~--~~--~--~---
http://codereview.chromium.org/42009/diff/10/13
File src/codegen-ia32.cc (right):
http://codereview.chromium.org/42009/diff/10/13#newcode3920
Line 3920: frame_->SpillAll();
I know it's not your code, but you could try getting rid of the SpillAll
here and changing the EmitPush to PushElementAt, j
http://codereview.chromium.org/42009/diff/10/13
File src/codegen-ia32.cc (right):
http://codereview.chromium.org/42009/diff/10/13#newcode3920
Line 3920: frame_->SpillAll();
Done.
http://codereview.chromium.org/42009
--~--~-~--~~~---~--~~
v8-dev mailing list
v8-d
Author: kmilli...@chromium.org
Date: Tue Mar 10 05:11:56 2009
New Revision: 1475
Modified:
branches/bleeding_edge/src/ast.cc
branches/bleeding_edge/src/ast.h
branches/bleeding_edge/src/codegen-arm.cc
branches/bleeding_edge/src/codegen-arm.h
branches/bleeding_edge/src/codegen-i
Updates:
Status: Fixed
Comment #1 on issue 267 by l...@chromium.org: Indirect eval fails in
eval-tainted scope.
http://code.google.com/p/v8/issues/detail?id=267
Fixed on bleeding edge in release 1476.
--
You received this message because you are listed in the owner
or CC fields of thi
Reviewers: Mads Ager,
Description:
Remove spaces to satisfy linter
tbr=a...@chromium.org
Please review this at http://codereview.chromium.org/43012
SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/
Affected files:
M src/factory.cc
Index: src/factory.cc
==
Author: l...@chromium.org
Date: Tue Mar 10 05:28:34 2009
New Revision: 1476
Added:
branches/bleeding_edge/test/mjsunit/regress/regress-267.js
Modified:
branches/bleeding_edge/src/ast.h
branches/bleeding_edge/src/codegen-arm.cc
branches/bleeding_edge/src/codegen-ia32.cc
branche
Author: sgje...@chromium.org
Date: Tue Mar 10 05:41:11 2009
New Revision: 1477
Modified:
branches/bleeding_edge/src/factory.cc
Log:
Remove spaces to satisfy linter
tbr=a...@chromium.org
Review URL: http://codereview.chromium.org/43012
Modified: branches/bleeding_edge/src/factory.cc
===
OK
On Tue, Mar 10, 2009 at 1:40 PM, wrote:
> Reviewers: Mads Ager,
>
> Description:
> Remove spaces to satisfy linter
>
> tbr=a...@chromium.org
>
> Please review this at http://codereview.chromium.org/43012
>
> SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/
>
> Affected files:
>
Reviewers: Lasse Reichstein,
Description:
Tweaked semantics of error prototype.
Please review this at http://codereview.chromium.org/43015
Affected files:
M src/messages.js
A test/mjsunit/error-constructors.js
Index: src/messages.js
diff --git a/src/messages.js b/src/messages.js
index
LGTM
http://codereview.chromium.org/43015
--~--~-~--~~~---~--~~
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
-~--~~~~--~~--~--~---
Author: christian.plesner.han...@gmail.com
Date: Tue Mar 10 06:15:24 2009
New Revision: 1478
Added:
branches/bleeding_edge/test/mjsunit/error-constructors.js
Modified:
branches/bleeding_edge/src/messages.js
Log:
Tweaked semantics of error prototype.
Modified: branches/bleeding_edge/src
Author: kmilli...@chromium.org
Date: Tue Mar 10 06:23:43 2009
New Revision: 1479
Modified:
branches/bleeding_edge/test/mjsunit/mjsunit.status
Log:
Change expectation for the bug-265 test to allow crashing in release
builds.
Modified: branches/bleeding_edge/test/mjsunit/mjsunit.status
=
Author: kasp...@chromium.org
Date: Tue Mar 10 06:35:28 2009
New Revision: 1480
Modified:
branches/bleeding_edge/test/mjsunit/mjsunit.status
Log:
Fix typo.
Modified: branches/bleeding_edge/test/mjsunit/mjsunit.status
===
LGTM when Kevin's comments are addressed plus some more comments.
Also what is the impact on compilation time and benchmark scores on top
of the code bloat question in the comments?
Thanks,
-Ivan
http://codereview.chromium.org/42006/diff/1/3
File src/codegen-ia32.cc (right):
http://coderevi
Reviewers: Mikhail Naganov, Erik Corry, Mads Ager,
Description:
Move InitLineEnds and GetLineNumber to handles.cc to avoid
issues with a raw this pointer and GC.
Fix issue with FlattenString where we may keep a StringShape
alive across a GC.
Please review this at http://codereview.chromium.org/
Comment #2 on issue 268 by kasp...@chromium.org: Valgrind reports leaks in
code generator
http://code.google.com/p/v8/issues/detail?id=268
This is probably because all the JumpTargets embedded in AST nodes never
gets their
deconstructor called (because they are ZoneObjects).
--
You received
LGTM
http://codereview.chromium.org/43020
--~--~-~--~~~---~--~~
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
-~--~~~~--~~--~--~---
Status: Accepted
Owner: sgje...@chromium.org
Labels: Type-Bug Priority-High
New issue 269 by sgje...@chromium.org: Debugger crashes when stepping into
apply()
http://code.google.com/p/v8/issues/detail?id=269
Running the following code with active debugger (e.g. in D8 or Chrome with
debugger op
Author: kasp...@chromium.org
Date: Tue Mar 10 08:08:45 2009
New Revision: 1481
Modified:
branches/bleeding_edge/src/accessors.cc
branches/bleeding_edge/src/compiler.cc
branches/bleeding_edge/src/handles.cc
branches/bleeding_edge/src/handles.h
branches/bleeding_edge/src/objects
Reviewers: Mads Ager,
Description:
See http://code.google.com/p/v8/issues/detail?id=269.
BUG=269
Please review this at http://codereview.chromium.org/43023
SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/
Affected files:
A test/mjsunit/bugs/bug-269.js
M test/mjsuni
Reviewers: iposva,
Message:
My last changeset got messed up on my drive (bad svn entries file). So
I've made the changes suggested in
http://codereview.chromium.org/39256/show and created a new one.
Description:
- Added ability to call histograms from within v8
- Changed the StatsRates to use th
LGTM
http://codereview.chromium.org/43020/diff/1/3
File src/handles.cc (right):
http://codereview.chromium.org/43020/diff/1/3#newcode189
Line 189:
CALL_HEAP_FUNCTION_VOID(string->TryFlattenIfNotFlat(StringShape(*string)));
I think this change makes the code much clearer (and I think we should
g
LGTM
http://codereview.chromium.org/43023/diff/1/3
File test/mjsunit/mjsunit.status (right):
http://codereview.chromium.org/43023/diff/1/3#newcode42
Line 42: bugs/bug-269: CRASH, FAIL if $mode == debug
Do you need this line? All tests in the bugs directory are expected to
fail. Maybe they are
Many comments about comments, but otherwise LGTM.
-Ivan
http://codereview.chromium.org/42020/diff/1/2
File include/v8.h (right):
http://codereview.chromium.org/42020/diff/1/2#newcode1916
Line 1916: static void
SetAddHistogramSampleFunction(AddHistogramSampleCallback);
Please add comments doc
Reviewers: Kasper Lund,
Message:
Now ported to ARM (as much as was necessary).
Description:
Fix issue 265 by handling extra statement state on the frame based on
the expectation at the break, continue, and return labels (including
shadowed ones) instead of based on the AST nodes.
See http://cod
If you deal with the virtual Initialize invocation in the JumpTarget
constructor, this LGTM.
Cheers,
Kasper
On Wed, Mar 11, 2009 at 6:50 AM, wrote:
> Reviewers: Kasper Lund,
>
> Message:
> Now ported to ARM (as much as was necessary).
>
> Description:
> Fix issue 265 by handling extra statemen
Author: kmilli...@chromium.org
Date: Tue Mar 10 23:17:19 2009
New Revision: 1482
Added:
branches/bleeding_edge/test/mjsunit/regress/regress-265.js
Removed:
branches/bleeding_edge/test/mjsunit/bugs/bug-265.js
Modified:
branches/bleeding_edge/src/assembler-arm.h
branches/bleeding_ed
Author: kmilli...@chromium.org
Date: Tue Mar 10 23:31:21 2009
New Revision: 1483
Modified:
branches/bleeding_edge/src/jump-target.cc
Log:
Fix formatting problem.
Modified: branches/bleeding_edge/src/jump-target.cc
68 matches
Mail list logo
