[v8-dev] Re: Replace C++ bitfields with our own BitFields (issue 700963002 by jkumme...@chromium.org)

2015-10-23 Thread brucedawson
Yup, 1-bit enum bitfields are a problem because enums are signed in VC++. 1-bit int bitfields would show the same problem in gcc - a bitfield that can't store '1' (0 and -1 only) is confusing. Additionally the packing rules for bitfields can vary between compilers. In particular, in VC++ diff

[v8-dev] Re: [Interpreter] Skeleton bytecode graph builder [NOT FOR COMMIT] (issue 1291693004 by o...@chromium.org)

2015-09-14 Thread brucedawson
This mornings 'new warning' report from the /analyzer builder pointed out some variable shadowing. Not bugs, but worth mentioning. https://codereview.chromium.org/1291693004/diff/340001/src/compiler/bytecode-graph-builder.cc File src/compiler/bytecode-graph-builder.cc (right): https://coderev

[v8-dev] Re: Debugger: refactor and move ScopeIterator and FrameInspector. (issue 1264993002 by yang...@chromium.org)

2015-08-10 Thread brucedawson
https://codereview.chromium.org/1264993002/diff/60001/src/debug/debug-scopes.cc File src/debug/debug-scopes.cc (right): https://codereview.chromium.org/1264993002/diff/60001/src/debug/debug-scopes.cc#newcode86 src/debug/debug-scopes.cc:86: Handle scope_info(shared_info->scope_info()); This varia

[v8-dev] Re: Fix runtime-atomics for Win 10 SDK and remove volatile (issue 1228063005 by brucedaw...@chromium.org)

2015-07-16 Thread brucedawson
What is the use-case for building V8 with the v120_xp toolset? Is that something that we need to support? When I wrote my fix I was unaware of how to detect which Windows SDK was being used. I have since learned how. You need to #include and then test VER_PRODUCTBUILD, as shown here: htt

[v8-dev] Fix runtime-atomics for Win 10 SDK and remove volatile (issue 1228063005 by brucedaw...@chromium.org)

2015-07-14 Thread brucedawson
Reviewers: jarin, Description: Fix runtime-atomics for Win 10 SDK and remove volatile For unclear and probably accidental reasons the Windows 10 SDK renamed some _Interlocked* functions to _InlineInterlocked. This leads to these errors: runtime-atomics.cc(159): error C3861: '_InterlockedExchang

[v8-dev] Re: Debugger: use list to find shared function info in a script. (issue 1206573004 by yang...@chromium.org)

2015-06-29 Thread brucedawson
Three instances of potentially-confusing variable shadowing. Consider fixing? https://codereview.chromium.org/1206573004/diff/40001/src/debug.cc File src/debug.cc (right): https://codereview.chromium.org/1206573004/diff/40001/src/debug.cc#newcode2064 src/debug.cc:2064: SharedFunctionInfo* sha

[v8-dev] Re: Remove duplicate isolate (issue 1196533004 by verwa...@chromium.org)

2015-06-22 Thread brucedawson
I'm not an OWNER but lgtm. https://codereview.chromium.org/1196533004/ -- -- 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 r

[v8-dev] Re: Inline SetLengthWithoutNormalize into its callers (issue 1194943002 by verwa...@chromium.org)

2015-06-22 Thread brucedawson
https://codereview.chromium.org/1194943002/diff/20001/src/elements.cc File src/elements.cc (right): https://codereview.chromium.org/1194943002/diff/20001/src/elements.cc#newcode1323 src/elements.cc:1323: Isolate* isolate = array->GetIsolate(); This declaration of 'isolate' shadows a seemingly id

[v8-dev] Re: Replace ad-hoc weakness in transition array with WeakCell. (issue 1157943003 by u...@chromium.org)

2015-06-19 Thread brucedawson
One post-commit comment, based on a /analyze warning. Just an FYI. https://codereview.chromium.org/1157943003/diff/80001/src/heap/mark-compact.cc File src/heap/mark-compact.cc (right): https://codereview.chromium.org/1157943003/diff/80001/src/heap/mark-compact.cc#newcode2490 src/heap/mark-compa

[v8-dev] Re: Cleanup after TF super.prop (issue 1151303008 by a...@chromium.org)

2015-06-08 Thread brucedawson
I'm a full Chromium committer, but not a v8 committer. https://codereview.chromium.org/1151303008/ -- -- 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 unsubscrib

[v8-dev] Re: Cleanup after TF super.prop (issue 1151303008 by a...@chromium.org)

2015-06-08 Thread brucedawson
lgtm. Thanks for addressing this. The impetus for this, BTW, came from the /analyze builder. I check its results every morning and it reported this new warning today: src\v8\src\compiler\ast-graph-builder.cc(2270) : warning C6001: Using uninitialized memory 'value'. Setting up to check its r

[v8-dev] Re: [es6] Add TF for super.prop (issue 1149133005 by a...@chromium.org)

2015-06-08 Thread brucedawson
https://codereview.chromium.org/1149133005/diff/11/src/compiler/ast-graph-builder.cc File src/compiler/ast-graph-builder.cc (right): https://codereview.chromium.org/1149133005/diff/11/src/compiler/ast-graph-builder.cc#newcode2219 src/compiler/ast-graph-builder.cc:2219: Node* value; It is

[v8-dev] Re: Reland: track global accesses to constant types (issue 1102543002 by dcar...@chromium.org)

2015-04-29 Thread brucedawson
This change makes kUninitialized and kUndefined have the same value, where they used to be distinct. I noticed this because the /analyze builder gave a new warning pointing out that there was some redundant code. The warning is: src\v8\src\objects.cc(17173) : warning C6287: Redundant code: the

[v8-dev] get_stack_trace_line_fun consuming 98.6% of CPU time in new bookmark manager

2015-04-10 Thread brucedawson
I just verified a customer reported bug - a slowdown in the new bookmark manager when deleting folders containing many (~2,500) bookmarks. Deleting them takes over a minute. Profiling with ETW (Event Tracing for Windows -- this is on a Windows machine) shows that 98.6% of the CPU time (it is CPU

[v8-dev] Don't #define snprintf in VS2015 - it's illegal and unneeded. (issue 1078453002 by brucedaw...@chromium.org)

2015-04-08 Thread brucedawson
Reviewers: jarin, Message: One small step towards building Chrome with VS 2015. Description: Don't #define snprintf in VS2015 - it's illegal and unneeded. VS 2015 supplies a conforming snprintf implementation, so #define snprintf is no longer needed. Also, VS 2015 checks for #define of snprintf

[v8-dev] Re: [turbofan] optimize moves into merges (issue 755323011 by dcar...@chromium.org)

2015-04-07 Thread brucedawson
We do have a /analyze bot at http://build.chromium.org/p/chromium.fyi/builders/Chromium%20Windows%20Analyze/. However, it displays the raw results which are a bit daunting: 6.5 MB of raw text. After running through a script that throws away duplicates and organizes the warnings it is still 733

[v8-dev] Re: [turbofan] optimize moves into merges (issue 755323011 by dcar...@chromium.org)

2015-04-06 Thread brucedawson
Please take a look at variable shadowing comment? https://codereview.chromium.org/755323011/diff/340001/src/compiler/move-optimizer.cc File src/compiler/move-optimizer.cc (right): https://codereview.chromium.org/755323011/diff/340001/src/compiler/move-optimizer.cc#newcode220 src/compiler/move-o

[v8-dev] Re: [turbofan] Remove unused diamonds during control reduction. (issue 1000883003 by bmeu...@chromium.org)

2015-03-13 Thread brucedawson
I opened a bug for this issue: crbug/467099 Also, but all-purpose bug for /analyze is 427616 so any fix should be annotated with that bug also. https://codereview.chromium.org/1000883003/diff/1/src/compiler/control-reducer.cc File src/compiler/control-reducer.cc (right): https://codereview.

[v8-dev] Re: Implement parsing ES6 Template Literals (issue 663683006 by caitpotte...@gmail.com)

2014-12-09 Thread brucedawson
Renaming it would be nice, depending on the tradeoff you want between code clarity and avoiding churn. Glad to hear the code is correct. https://codereview.chromium.org/663683006/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this mess

[v8-dev] Re: Implement parsing ES6 Template Literals (issue 663683006 by caitpotte...@gmail.com)

2014-12-09 Thread brucedawson
https://codereview.chromium.org/663683006/diff/710001/src/preparser.h File src/preparser.h (right): https://codereview.chromium.org/663683006/diff/710001/src/preparser.h#newcode2816 src/preparser.h:2816: int pos = peek_position(); While running VC++'s /analyze on Chrome I got a warning for this

[v8-dev] Re: Replace C++ bitfields with our own BitFields (issue 700963002 by jkumme...@chromium.org)

2014-11-04 Thread brucedawson
I looked through all of the .h files and some of the .cc files. All of the changes appear good (although it would be very easy to miss an error). Some of the changes look like they will reduce the size of structures. Some of the changes are avoidable (I commented on some but not all of these

[v8-dev] Re: Don't use one-bit bit fields for enums -- they misbehave and are inefficient in VC++ (issue 694003002 by brucedaw...@chromium.org)

2014-11-04 Thread brucedawson
Okay, updates made. It's a shame that gcc and VC++ both make combining of enums and bit fields so messy. In this case eschewing the bit field helps. The behavioral change from this fix concerns me, but it should just make VC++ behave the same as the other platforms. https://codereview.chrom

[v8-dev] Re: Don't use one-bit bit fields for enums -- they misbehave and are inefficient in VC++ (issue 694003002 by brucedaw...@chromium.org)

2014-11-03 Thread brucedawson
I have an update that further improves things with VC++ by squishing the enum into an unsigned char, but I am having trouble uploading it. "git cl upload" gives me this error and I don't know what it means or where LOG=Y is supposed to go. ** Presubmit ERRORS ** An issue reference (BUG=) req

[v8-dev] Re: Don't use one-bit bit fields for enums -- they misbehave and are inefficient in VC++ (issue 694003002 by brucedaw...@chromium.org)

2014-11-03 Thread brucedawson
On 2014/11/03 17:23:37, cpu wrote: there are quite a few more of those in the file... not compiled for windows? Were the other ones references to the same enum? If so then they should all be fixed by this change. One of the other references to store_mode_ is through a store_mode() acces

[v8-dev] Don't use one-bit bit fields for enums -- they misbehave and are inefficient in VC++ (issue 694003002 by brucedaw...@chromium.org)

2014-10-31 Thread brucedawson
Reviewers: Hannes Payer (OOO), Description: Don't use one-bit bit fields for enums -- they misbehave in VC++, unless you force the enum's type to be unsigned. enum bit fields may also waste space. One-bit bit fields in VC++ default to being signed so their only possible values are zero and *n

[v8-dev] Changed free(buffer) to delete [] buffer (issue 686193004 by brucedaw...@chromium.org)

2014-10-29 Thread brucedawson
Reviewers: Yang, Description: Changed free(buffer) to delete [] buffer. This bug (mismatch between new[] and free) was found by running VC++'s /analyze on all of Chrome. BUG=427616 Please review this at https://codereview.chromium.org/686193004/ Base URL: https://v8.googlecode.com/svn/bran

[v8-dev] Certificate error when fetching v8 code

2014-10-28 Thread brucedawson
When fetching the v8 code with 'fetch v8' on Windows I always hit this problem: Running: 'C:\Users\brucedawson\Documents\depot_tools\git.bat' svn fetch Error validating server certificate for 'https://v8.googlecode.com:443': - The certificate is not issued b