https://codereview.chromium.org/967243002/diff/20001/include/v8.h
File include/v8.h (right):
https://codereview.chromium.org/967243002/diff/20001/include/v8.h#newcode5737
include/v8.h:5737: V8_INLINE bool IsJust() const { return has_value; }
I saw this since someone made a similar name change in
Zombie review comment! Can you expand on the "as discussed" bit a bit? The
BUG=
line is just some generic tracking bug, and the CL description doesn't
really
say what's happening here. I think it was because bool bitfields are
considered
signed and aren't packed tightly with neighboring unsig
lgtm, thanks!
https://codereview.chromium.org/1301093002/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emai
lgtm, this has a good chance of working I think.
https://codereview.chromium.org/1288723005/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from
lgtm, thanks!
https://codereview.chromium.org/1287543004/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emai
https://codereview.chromium.org/249183003/diff/1/src/base/macros.h
File src/base/macros.h (right):
https://codereview.chromium.org/249183003/diff/1/src/base/macros.h#newcode39
src/base/macros.h:39: // Here we simply use the non-zero value 4, which
seems to work.
Is this really still needed? Othe
On 2015/05/20 20:21:15, hans wrote:
New patch uploaded.
https://codereview.chromium.org/1145213004/diff/1/include/v8config.h
File include/v8config.h (right):
https://codereview.chromium.org/1145213004/diff/1/include/v8config.h#newcode273
include/v8config.h:273: #elif defined(_MSC_VER)
On 2
checked myself now that i'm on a laptop – I didn't change this as much as I
wanted.
In general, I think __clang__ checks should be kept to a minimum and the
gcc and
msvc macro checks should instead mostly do the right thing in clang mode
too. In
this case:
https://codereview.chromium.org/1
On 2015/05/20 18:18:39, Reid Kleckner wrote:
lgtm
Can you check where that explicit clang check for added? Iirc I removed
that a
while ago, but maybe I'm misremembering what exactly I did here.
https://codereview.chromium.org/1145213004/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
ht
lgtm, thanks!
https://codereview.chromium.org/1073943002/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emai
…and there it is. ptal.
https://codereview.chromium.org/1073903002/diff/1/include/v8-profiler.h
File include/v8-profiler.h (left):
https://codereview.chromium.org/1073903002/diff/1/include/v8-profiler.h#oldcode36
include/v8-profiler.h:36: std::vector stack;
On 2015/04/09 20:23:21, loislo wrote:
sorry, i meant to put sven into the reviewers box, not the cc box
https://codereview.chromium.org/1073903002/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To
Reviewers: loislo,
Description:
Fix C++ violation.
gcc rejects the following snippet, clang rejects it in -std=c++11 mode:
namespace A { template class C {}; }
namespace B { template class A::C; }
Indeed, the C++ standard says in 14.7.2p2 "An explicit instantiation shall
appear in an enclos
On 2015/03/15 04:22:03, Yang wrote:
On 2015/03/14 20:16:39, Nico (traveling) wrote:
> On 2015/03/14 18:41:49, I haz the power (commit-bot) wrote:
> > Patchset 1 (id:??) landed as
> > https://crrev.com/bab55d24aed533b7a86497bbc8f4a2b90d0921c3
> > Cr-Commit-Position: refs/heads/master@{#27200}
>
>
On 2015/03/14 18:41:49, I haz the power (commit-bot) wrote:
Patchset 1 (id:??) landed as
https://crrev.com/bab55d24aed533b7a86497bbc8f4a2b90d0921c3
Cr-Commit-Position: refs/heads/master@{#27200}
Do you not support building on Mac?
https://codereview.chromium.org/1005333002/
--
--
v8-dev maili
I don't know much about this, but !is_clang is still used by cros (which
uses
one toolchain for the whole system from what i understand)
https://codereview.chromium.org/993173003/diff/20001/BUILD.gn
File BUILD.gn (right):
https://codereview.chromium.org/993173003/diff/20001/BUILD.gn#newcode22
On 2015/03/09 17:02:52, Michael Achenbach wrote:
PTAL. Has only an effect when in target_defaults...
Ah that makes sense. Lgtm, sorry about missing that.
https://codereview.chromium.org/990253002/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You
lgtm
https://codereview.chromium.org/987063002/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it
https://codereview.chromium.org/978783002/diff/40001/build/standalone.gypi
File build/standalone.gypi (right):
https://codereview.chromium.org/978783002/diff/40001/build/standalone.gypi#newcode527
build/standalone.gypi:527: 'CLANG_CXX_LIBRARY': 'libc++', #
-stdlib=libc++
On 2015/03/04 18:52:37,
Thanks!
https://codereview.chromium.org/975383002/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from
https://codereview.chromium.org/975283002/diff/20001/src/base/bits.h
File src/base/bits.h (right):
https://codereview.chromium.org/975283002/diff/20001/src/base/bits.h#newcode151
src/base/bits.h:151: // Precondition: 0 <= shift < 32
On 2015/03/04 06:02:51, hans wrote:
Any reason the preconditio
Reviewers: jochen (traveling),
Description:
When using ninja and clang, make sure diagnostics are colored.
BUG=none
LOG=N
Please review this at https://codereview.chromium.org/969653004/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Affected files (+7, -0 lines):
M build/stan
Reviewers: jochen (traveling),
Message:
Small follow-up.
Description:
Use Rotate*() functions instead of doing this manually.
Shouldn't make a difference in practice, but it's a bit more readable and it
gets the case of a 0 shift correct without undefined behavior.
BUG=463436
LOG=N
Please rev
On 2015/03/04 01:21:41, jochen (traveling) wrote:
lgtm
Can someone land?
https://codereview.chromium.org/979633002/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" g
lgtm ;-)
https://codereview.chromium.org/792783006/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails fro
https://codereview.chromium.org/23248006/diff/11001/include/v8config.h
File include/v8config.h (right):
https://codereview.chromium.org/23248006/diff/11001/include/v8config.h#newcode118
include/v8config.h:118: #if defined(__clang__)
On 2014/12/01 18:02:00, Benedikt Meurer wrote:
So you're talki
Reviewers: Benedikt Meurer,
Message:
Useful cs links:
https://code.google.com/p/chromium/codesearch#chromium/src/v8/include/v8config.h&q=v8_cc_clang&sq=package:chromium&type=cs&l=196
https://code.google.com/p/chromium/codesearch#search/&q=v8_cc_gnu&sq=package:chromium&type=cs
https://code.google.
https://codereview.chromium.org/23248006/diff/11001/include/v8config.h
File include/v8config.h (right):
https://codereview.chromium.org/23248006/diff/11001/include/v8config.h#newcode118
include/v8config.h:118: #if defined(__clang__)
On 2014/12/01 18:02:00, Benedikt Meurer wrote:
So you're talki
https://codereview.chromium.org/23248006/diff/11001/include/v8config.h
File include/v8config.h (right):
https://codereview.chromium.org/23248006/diff/11001/include/v8config.h#newcode118
include/v8config.h:118: #if defined(__clang__)
This doesn't look like a good approach to me. Clang tries to lo
Thanks! Let me know if you want me to edit the 3 lines you pointed out and
the 1
I'm pointing out, or if you'll do that before landing. (I figured I'd miss
some;
http://build.chromium.org/p/chromium.fyi/builders/Cr%20Win%20Clang/builds/1226/steps/compile/logs/stdio
shows all these warnings and
jkummerow: If you like this, it'd be great if you could land it too. I'm
not a
v8 committer.
https://codereview.chromium.org/688533002/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Googl
Reviewers: Vyacheslav Egorov,
Description:
Add a few missing overrides found by a new clang warning.
Namely, -Winconsistent-missing-override. No behavior change.
BUG=v8:3658
LOG=N
Please review this at https://codereview.chromium.org/688533002/
Base URL: http://v8.googlecode.com/svn/branches/
Out of interest, what goes wrong if you don't explicitly check for nacl? I
believe we want all our toolchains to be c++11-capable, and the nacl irt
build
switched to building with the pnacl toolchain to achieve this. Which
configuration did we miss?
https://codereview.chromium.org/668303002/
https://codereview.chromium.org/560903002/diff/1/src/base/build_config.h
File src/base/build_config.h (right):
https://codereview.chromium.org/560903002/diff/1/src/base/build_config.h#newcode27
src/base/build_config.h:27: #if defined(__x86_64__) &&
__SIZEOF_POINTER__ == 32 // Check for x32.
On
(from the llvm bug: https://sourceware.org/glibc/wiki/x32 recommends to
check
__ILP32__ instead, no idea if that's better or any different in practice
from
what this patch does.)
https://codereview.chromium.org/560903002/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google
Reviewers: jochen,
Message:
(see also http://llvm.org/PR20896)
Description:
Improve x32 detection macro.
When targeting the Microsoft ABI in 64bit mode, clang defines __x86_64__ but
doesn't define __LP64__ (Microsoft uses LLP64), so it would fall down the
x32
path. cl.exe doesn't define __x8
https://codereview.chromium.org/18014003/diff/161001/src/base/build_config.h
File src/base/build_config.h (right):
https://codereview.chromium.org/18014003/diff/161001/src/base/build_config.h#newcode27
src/base/build_config.h:27: #if defined(__x86_64__) &&
!defined(__LP64__)
On 2014/09/10 16:54:
https://codereview.chromium.org/18014003/diff/161001/src/base/build_config.h
File src/base/build_config.h (right):
https://codereview.chromium.org/18014003/diff/161001/src/base/build_config.h#newcode27
src/base/build_config.h:27: #if defined(__x86_64__) &&
!defined(__LP64__)
Is this the best def
Reviewers: jochen,
Description:
Availability of sprintf_s is a C standard library thing, not a compiler
thing.
Our clang/win build currently uses MSVS's C library, so it doesn't have
snprintf but it does have sprintf_s.
BUG=chromium:82385
LOG=n
Please review this at https://codereview.chrom
Reviewers: jochen,
Description:
et host_arch to ia32 on machines with a 32bit userland but a 64bit kernel.
I don't know if there are any v8 bots with that configuration, but it seems
like a good idea to have v8 be consistent with chromium and nacl here, so
that
this works fine if such a bot i
Are you sure this does anything?
Nicos-MacBook-Pro:tmp thakis$ cat Makefile
foo:
PYTHONPATH="$(shell pwd)/tools/generate_shim_headers:$(PYTHONPATH)" \
PYTHONPATH="$(shell pwd)/build/gyp/pylib:$(PYTHONPATH)" \
bash -c 'echo $${PYTHONPATH}&#x
Reviewers: jochen traveling until Jun 23,
Message:
http://build.chromium.org/p/tryserver.v8/waterfall seems to ignore my `gcl
try`
invocation, but it seems to work locally at least.
Description:
Set host_arch to ia32 on machines with a 32bit userland but a 64bit kernel.
I don't know if there
I changed this to work with the x87 backend.
https://codereview.chromium.org/345443003/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this
Reviewers: jochen traveling until Jun 23,
Description:
v8 atomicops: Drop SSE2 detection, v8 always requires SSE2.
This ports Chromium's https://codereview.chromium.org/291993003/
BUG=chromium:348761, chromium:94925
LOG=N
Please review this at https://codereview.chromium.org/345443003/
SVN Ba
I'm pretty surprised by the duplicate flags too. I wonder why that doesn't
happen for chromium. I'd add a few print statments in
build/gyp/pylib/gyp/input.py ('configuration'-related stuff, __init__.py and
generators/ninja.py ) :-/
https://codereview.chromium.org/224443003/diff/40001/build.ninja
(happy side effect: if you use GYP_GENERATORS=ninja, it'll now also build
into
v8/out, not v8/out/out)
https://codereview.chromium.org/98753003/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to
Reviewers: machenbach,
Message:
With this, the global makefile wouldn't have to set `builddir` in the
`buildbot`
target either (but that target isn't used any more).
If this lgty, you probably have to land this yourself since this is
probably off
the wrong branch (?)
Description:
Pass -Go
fyi:
https://codereview.chromium.org/18516012/diff/19001/build/toolchain.gypi
File build/toolchain.gypi (right):
https://codereview.chromium.org/18516012/diff/19001/build/toolchain.gypi#newcode513
build/toolchain.gypi:513: 'GCC_STRICT_ALIASING': 'YES',
Note that strict aliasing is only done at
Reviewers: Michael Starzinger,
Message:
I'm not sure if this change is correct. The explicit cast suggests that a
real
floating division was desired here, but it probably doesn't matter too much
either way?
(Found by a compiler warning I'm prototyping, and I'm not sure if it's a
useful
one
Thanks!
The prerequisite landed in r11586 – could someone land this CL?
http://codereview.chromium.org/10399036/diff/1/include/v8.h
File include/v8.h (right):
http://codereview.chromium.org/10399036/diff/1/include/v8.h#newcode65
include/v8.h:65: // Setup for Linux shared library export. There
Reviewers: Mads Sig Ager, Søren Thygesen Gjesse,
Description:
Define V8_EXPORT to nothing for clients of v8.
This is to make sure that inline functions are only exported by
libv8.so and not also by all clients. This is the v8 version of
https://chromiumcodereview.appspot.com/10386108/
This CL d
On 2012/05/15 09:24:42, Yang wrote:
On 2012/05/14 23:48:16, Nico wrote:
> This is necessary for a future change I'd like to make (basically,
> http://codereview.chromium.org/10386108/ for v8).
>
> https://chromiumcodereview.appspot.com/10310156/diff/1/tools/gyp/v8.gyp
> File tools/gyp/v8.gyp (rig
Reviewers: fschneider,
Message:
This is necessary for a future change I'd like to make (basically,
http://codereview.chromium.org/10386108/ for v8).
https://chromiumcodereview.appspot.com/10310156/diff/1/tools/gyp/v8.gyp
File tools/gyp/v8.gyp (right):
https://chromiumcodereview.appspot.com/103
lgtm
http://codereview.chromium.org/10177004/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
54 matches
Mail list logo
