Author: [EMAIL PROTECTED]
Date: Wed Nov 26 00:03:55 2008
New Revision: 845
Modified:
branches/bleeding_edge/src/SConscript
branches/bleeding_edge/src/ast.h
branches/bleeding_edge/src/jsregexp-inl.h
branches/bleeding_edge/src/jsregexp.cc
branches/bleeding_edge/src/jsregexp.h
Reviewers: Erik Corry, iposva,
Message:
Restructured ARM/IA32 division in new regexp code
(+ minor readability fixups).
Description:
Created separate include file that distinguishes between architectures.
* Removed #if defined ARM || defined __arm__ || defined __thumb__.
Only test against "AR
Reviewers: Christian Plesner Hansen,
Message:
Small review, please.
Description:
Fixed some outstanding formatting issues.
Please review this at http://codereview.chromium.org/12470
Affected files:
M src/assembler.h
src/ast.h
M src/ast.cc
M src/jsregexp-inl.h
M src/jsregexp.cc
On 2008/11/26 11:04:15, Lasse Reichstein wrote:
> Small review, please.
It's only addressing previous reviews, so I went ahead and committed it
directly.
http://codereview.chromium.org/12470
--~--~-~--~~~---~--~~
v8-dev mailing list
v8-dev@googlegroups.com
http:/
I have a couple of comments on top of Kasper's. I think we need to be
careful with evaluation order here.
http://codereview.chromium.org/12673/diff/1/15
File src/codegen-arm.cc (right):
http://codereview.chromium.org/12673/diff/1/15#newcode2323
Line 2323: Label after_call;
Move this new code b
I've gone through the comments for the code I feel responsible for.
http://codereview.chromium.org/12427/diff/1/23
File src/assembler-irregexp-inl.h (right):
http://codereview.chromium.org/12427/diff/1/23#newcode69
Line 69: void IrregexpAssembler::EmitOrLink(Label* l) {
On 2008/11/25 21:09:41,
Author: [EMAIL PROTECTED]
Date: Wed Nov 26 04:18:17 2008
New Revision: 847
Modified:
branches/bleeding_edge/src/assembler-irregexp-inl.h
branches/bleeding_edge/src/assembler-irregexp.cc
branches/bleeding_edge/src/assembler-irregexp.h
branches/bleeding_edge/src/bytecodes-irregexp.h
Oops, this comment should have been attached to
http://codereview.chromium.org/12469
On Wed, Nov 26, 2008 at 1:33 PM, <[EMAIL PROTECTED]> wrote:
> I'm afraid I don't like parts of this change. The new
> RegExpMacroAssemblerImpl typedef is only used in one place and that's
> inside an ifdef. It
Thanks for the comments. New patch set uploaded.
http://codereview.chromium.org/11396
--~--~-~--~~~---~--~~
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
-~--~~~~--~~--~--~---
Author: [EMAIL PROTECTED]
Date: Wed Nov 26 05:54:08 2008
New Revision: 848
Added:
branches/bleeding_edge/src/regexp-macro-assembler-arm.cc
- copied, changed from r847,
/branches/bleeding_edge/src/macro-assembler.h
branches/bleeding_edge/src/regexp-macro-assembler-arm.h
- c
No extra include file any more. A few more "#if defined(ARM)" fixed.
http://codereview.chromium.org/12469/diff/22/202
File src/execution.cc (right):
http://codereview.chromium.org/12469/diff/22/202#newcode35
Line 35: #if defined(ARM)
On 2008/11/26 10:05:59, Kevin Millikin wrote:
> #ifdef ARM
G
Overall the change looks sound, but I think you should change it so that
you only have one runtime call per possibly direct eval. Comments:
http://codereview.chromium.org/12673/diff/1/9
File src/ast.h (right):
http://codereview.chromium.org/12673/diff/1/9#newcode868
Line 868: enum EvalType {
Do
http://codereview.chromium.org/12469/diff/22/202
File src/execution.cc (right):
http://codereview.chromium.org/12469/diff/22/202#newcode35
Line 35: #if defined(ARM)
#ifdef ARM
http://codereview.chromium.org/12469
--~--~-~--~~~---~--~~
v8-dev mailing list
v8-dev@
Reviewers: Erik Corry,
Description:
Graph node attribute printing.
Please review this at http://codereview.chromium.org/12471
Affected files:
M src/jsregexp.cc
M test/cctest/test-regexp.cc
Index: src/jsregexp.cc
diff --git a/src/jsregexp.cc b/src/jsregexp.cc
index
e569365d03146a03ad86
Attached is an example.
On Wed, Nov 26, 2008 at 3:08 PM, <[EMAIL PROTECTED]> wrote:
> Reviewers: Erik Corry,
>
> Description:
> Graph node attribute printing.
>
> Please review this at http://codereview.chromium.org/12471
>
> Affected files:
> M src/jsregexp.cc
> M test/cctest/test-regexp.cc
>
Cool stuff.
The bytecode tracing code works with hexadecimal program counters, so if
the PC could be hex that would be ideal.
http://codereview.chromium.org/12471
--~--~-~--~~~---~--~~
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-
OK, I just tested evaluation order in a webkit nightly and in firefox.
Apparently, they are also treating eval in a special way, so the
evaluation order of your new code seems to be what they are doing for
calls to something that might be eval.
eval(eval('var eval = function f() { print("hep"
Reviewers: Mads Ager,
Description:
Change implementation of eval to make an exact distinction between
direct eval and aliased eval.
Please review this at http://codereview.chromium.org/12673
SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/
Affected files:
M src/ast.h
M
Author: [EMAIL PROTECTED]
Date: Wed Nov 26 03:29:26 2008
New Revision: 846
Modified:
branches/bleeding_edge/src/assembler.h
branches/bleeding_edge/src/ast.cc
branches/bleeding_edge/src/ast.h
branches/bleeding_edge/src/jsregexp-inl.h
branches/bleeding_edge/src/jsregexp.cc
b
I'm afraid I don't like parts of this change. The new
RegExpMacroAssemblerImpl typedef is only used in one place and that's
inside an ifdef. It only adds confusion. The new include file does
something very simple and is used twice where it could be inlined.
Modulo this and the other comments i
Reviewers: Mads Ager, Christian Plesner Hansen,
Description:
Added a debugger call to run a JavaScript function in the debugger. When
called the debugger will be entered and the JavaScript function will be
called with the debugger ExecutionState object as its first parameter.
This makes it possi
Reviewers: Christian Plesner Hansen,
Description:
* Complete case independent support in Irregexp.
Please review this at http://codereview.chromium.org/12473
SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/
Affected files:
M src/flag-definitions.h
M src/jsregexp.h
LGTM.
Maybe you can add a test that shows what happens if you pass in a
closure that uses local variables from its context:
var x = 3;
function (execstate) {
execstate.something == x;
}
This should just work, but a test would be nice.
http://codereview.chromium.org/12472/diff/1/4
File src/
I'm afraid I don't like parts of this change. The new
RegExpMacroAssemblerImpl typedef is only used in one place and that's
inside an ifdef. It only adds confusion. The new include file does
something very simple and is used twice where it could be inlined.
Modulo this and the other comments i
Added a function which captures a context variable as an additional
test.
http://codereview.chromium.org/12472/diff/1/4
File src/api.cc (right):
http://codereview.chromium.org/12472/diff/1/4#newcode2907
Line 2907: if (!i::V8::HasBeenSetup()) v8::Undefined();
On 2008/11/26 15:02:42, Mads Ager wr
Reviewers: iposva, Erik Kay,
Description:
Add v8::jscre namespace around jscre functions to avoid link errors with
jsc pcre files in Chrome.
Please review this at http://codereview.chromium.org/12496
SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/
Affected files:
M src/j
LGTM
-Ivan
http://codereview.chromium.org/12496
--~--~-~--~~~---~--~~
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
-~--~~~~--~~--~--~---
LGTM
http://codereview.chromium.org/12496
--~--~-~--~~~---~--~~
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
-~--~~~~--~~--~--~---
Author: [EMAIL PROTECTED]
Date: Wed Nov 26 14:45:21 2008
New Revision: 849
Modified:
branches/bleeding_edge/src/jsregexp.cc
branches/bleeding_edge/src/third_party/jscre/pcre.h
branches/bleeding_edge/src/third_party/jscre/pcre_compile.cpp
branches/bleeding_edge/src/third_party/jscr
Reviewers: Feng Qian, Erik Kay,
Description:
Prepare to push the v8::jscre namespace changes to trunk.
This does not change the working revision number as this
is marked as revision 0.4.4.1, since it is quick specific
patch.
Please review this at http://codereview.chromium.org/12509
SVN Base:
lgtm
On 2008/11/26 23:28:40, iposva wrote:
>
http://codereview.chromium.org/12509
--~--~-~--~~~---~--~~
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
-~--~~~~--~~--~--~---
Author: [EMAIL PROTECTED]
Date: Wed Nov 26 15:34:36 2008
New Revision: 850
Modified:
branches/bleeding_edge/ChangeLog
Log:
Prepare to push the v8::jscre namespace changes to trunk.
This does not change the working revision number as this
is marked as revision 0.4.4.1, since it is quick speci
Reviewers: Feng Qian, Erik Kay,
Description:
Added the v8::jscre namespace around the jscre functions to avoid link
errors (duplicate symbols) when building Google Chrome.
Please review this at http://codereview.chromium.org/12511
SVN Base: http://v8.googlecode.com/svn/trunk/
Affected files:
lgtm
On 2008/11/26 23:43:27, iposva wrote:
>
http://codereview.chromium.org/12511
--~--~-~--~~~---~--~~
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
-~--~~~~--~~--~--~---
Author: [EMAIL PROTECTED]
Date: Wed Nov 26 15:48:02 2008
New Revision: 851
Modified:
trunk/ChangeLog
trunk/src/api.cc
trunk/src/jsregexp.cc
trunk/src/third_party/jscre/pcre.h
trunk/src/third_party/jscre/pcre_compile.cpp
trunk/src/third_party/jscre/pcre_exec.cpp
trunk/s
LGTM
http://codereview.chromium.org/12511
--~--~-~--~~~---~--~~
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
-~--~~~~--~~--~--~---
LGTM
http://codereview.chromium.org/12509
--~--~-~--~~~---~--~~
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
-~--~~~~--~~--~--~---
Lgtm
http://codereview.chromium.org/12473/diff/1/4
File src/jsregexp.cc (right):
http://codereview.chromium.org/12473/diff/1/4#newcode476
Line 476: //PrintF("\n\nSubject string: '%s'\n\n",
*(two_byte_subject->ToCString()));
Either comment it back in or remove it.
http://codereview.chromium.org
Lgtm
http://codereview.chromium.org/12472/diff/9/210
File include/v8-debug.h (right):
http://codereview.chromium.org/12472/diff/9/210#newcode132
Line 132: // Run a JavaScript function in the debugger.
From this comment it's not obvious what this means, exactly. Is it just
a way to get your ha
Author: [EMAIL PROTECTED]
Date: Wed Nov 26 23:21:43 2008
New Revision: 852
Modified:
branches/bleeding_edge/src/jsregexp.cc
branches/bleeding_edge/test/cctest/test-regexp.cc
Log:
Graph node attribute printing.
Modified: branches/bleeding_edge/src/jsregexp.cc
===
Author: [EMAIL PROTECTED]
Date: Wed Nov 26 23:27:08 2008
New Revision: 853
Modified:
branches/bleeding_edge/src/flag-definitions.h
branches/bleeding_edge/src/jsregexp.cc
branches/bleeding_edge/src/jsregexp.h
branches/bleeding_edge/src/regexp-macro-assembler-ia32.cc
branches/bl
On Thu, Nov 27, 2008 at 7:49 AM, <[EMAIL PROTECTED]> wrote:
> Lgtm
>
>
> http://codereview.chromium.org/12473/diff/1/4
> File src/jsregexp.cc (right):
>
> http://codereview.chromium.org/12473/diff/1/4#newcode476
> Line 476: //PrintF("\n\nSubject string: '%s'\n\n",
> *(two_byte_subject->ToCString()
I've changed it to hex.
On Wed, Nov 26, 2008 at 3:13 PM, <[EMAIL PROTECTED]> wrote:
> Cool stuff.
>
> The bytecode tracing code works with hexadecimal program counters, so if
> the PC could be hex that would be ideal.
>
> http://codereview.chromium.org/12471
>
--~--~-~--~~--
43 matches
Mail list logo
