- 28 Feb, 2018 1 commit
-
-
Nico Weber authored
gcc and clang (and the standard) don't allow implicit conversion of function pointers to object pointers. MSVC does allow that, and since system headers require this to work, clang-cl allows it too -- but it emits a -Wmicrosoft-cast warning (which we currently suppress in the Chromium build, but which we want to enable.) As a side effect, when printing a function pointer to a stream, MSVC (and clang-cl) will pick the operator<<(void*) overload, while gcc and clang will pick operator<<(bool) since the best allowed conversion they find is from function pointer to bool. To prevent the clang-cl warning, we need to make sure that we never directly print a function pointer to a stream. In v8, this requires two changes: 1. Give PrintCheckOperand() an explicit specialization for function pointers and explicitly cast to void* there. This ports https://codereview.chromium.org/2515283002/ to V8, and also fixes a bug on non-Windows where DCHECK() of function pointers would print "(1 vs 1)" instead of the function's addresses. (The bug remains with member function pointers, where it's not clear what to print instead of the 1.) 2. has_output_operator<T> must not use operator<< on its argument in an evaluated context if T is a function pointer. This patch modifies has_output_operator<> to use an unevaluated context instead, which is simpler than the current approach (and matches what Chromium's base does), but changes behavior in minor (boring) ways (see template-utils-unittest.cc), since operator<<() is now called with a temporary and only operator<<() implementations callable with a temporary are considered. A more complicated but behavior-preserving alternative would be to add an explicit specialization for function pointers. You can see this variant in patch set 1 on gerrit. Bug: chromium:550065 Change-Id: Idc2854d6c258b7fc0b959604006d8952a79eca3d Reviewed-on: https://chromium-review.googlesource.com/940004 Commit-Queue: Nico Weber <thakis@chromium.org> Reviewed-by: Clemens Hammacher <clemensh@chromium.org> Cr-Commit-Position: refs/heads/master@{#51636}
-
- 02 Dec, 2017 1 commit
-
-
Mathias Bynens authored
This patch normalizes the casing of hexadecimal digits in escape sequences of the form `\xNN` and integer literals of the form `0xNNNN`. Previously, the V8 code base used an inconsistent mixture of uppercase and lowercase. Google’s C++ style guide uses uppercase in its examples: https://google.github.io/styleguide/cppguide.html#Non-ASCII_Characters Moreover, uppercase letters more clearly stand out from the lowercase `x` (or `u`) characters at the start, as well as lowercase letters elsewhere in strings. BUG=v8:7109 TBR=marja@chromium.org,titzer@chromium.org,mtrofin@chromium.org,mstarzinger@chromium.org,rossberg@chromium.org,yangguo@chromium.org,mlippautz@chromium.org NOPRESUBMIT=true Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_chromium_rel_ng Change-Id: I790e21c25d96ad5d95c8229724eb45d2aa9e22d6 Reviewed-on: https://chromium-review.googlesource.com/804294 Commit-Queue: Mathias Bynens <mathias@chromium.org> Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> Cr-Commit-Position: refs/heads/master@{#49810}
-
- 28 Sep, 2017 1 commit
-
-
Mostyn Bramley-Moore authored
TBR=jkummerow@chromium.org Bug: chromium:746958 Change-Id: I7500b6206c4ceb087672de5b61b7e7ad234bb425 Reviewed-on: https://chromium-review.googlesource.com/690397 Commit-Queue: Jakob Kummerow <jkummerow@chromium.org> Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> Cr-Commit-Position: refs/heads/master@{#48213}
-
- 21 Sep, 2017 1 commit
-
-
Clemens Hammacher authored
In the current implementation, compilation would fail because operator<< is not defined for enum classes. For others, the compiler finds more than one operator<<, so it fails because it's ambiguous. This CL fixes this by printing the integer value for enums, uses the operator<< for all values that support it, and prints "<unprintable>" otherwise. Also, lots of unit tests. R=ishell@chromium.org Bug: v8:6837 Change-Id: I895ed226672aa07213f9605e094b87af186ec2e4 Reviewed-on: https://chromium-review.googlesource.com/671016 Commit-Queue: Clemens Hammacher <clemensh@chromium.org> Reviewed-by: Igor Sheludko <ishell@chromium.org> Cr-Commit-Position: refs/heads/master@{#48110}
-
- 15 Sep, 2017 1 commit
-
-
Sigurdur Asgeirsson authored
Bug: chromium:763010 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng Change-Id: I7d479f8abb16ffd7ffc19d3a6b58da01f5feddd0 Reviewed-on: https://chromium-review.googlesource.com/661054Reviewed-by: Camillo Bruni <cbruni@chromium.org> Commit-Queue: Sigurður Ásgeirsson <siggi@chromium.org> Cr-Commit-Position: refs/heads/master@{#48038}
-
- 12 May, 2017 1 commit
-
-
Clemens Hammacher authored
The current implementation failed when comparing an integral type to a reference to an integral type of different signedness (see updated unittest). This CL fixes the checks to actually test the std::decay<T>::type, i.e. with all references, const or volatile modifiers stripped. R=jochen@chromium.org, ishell@chromium.org TEST=unittests/LoggingTest.CompareWithReferenceType Change-Id: Ib0ac077a91e0409ada7a80b68150cb98cbdd32f1 Reviewed-on: https://chromium-review.googlesource.com/502814Reviewed-by: Jochen Eisinger <jochen@chromium.org> Reviewed-by: Igor Sheludko <ishell@chromium.org> Commit-Queue: Clemens Hammacher <clemensh@chromium.org> Cr-Commit-Position: refs/heads/master@{#45271}
-
- 20 Jan, 2017 1 commit
-
-
clemensh authored
In this particular case, we just did a (lhs)op(rhs), ignoring the case that lhs and rhs might have different signedness. This CL changes that to use the proper Cmp##op##Impl implementation, which does two comparisions for signed-vs-unsigned checks, avoiding compiler errors. R=ishell@chromium.org Review-Url: https://codereview.chromium.org/2642383002 Cr-Commit-Position: refs/heads/master@{#42566}
-
- 19 Dec, 2016 1 commit
-
-
yangguo authored
R=cbruni@chromium.org BUG=chromium:662388 Review-Url: https://codereview.chromium.org/2586203002 Cr-Commit-Position: refs/heads/master@{#41799}
-
- 01 Dec, 2016 1 commit
-
-
clemensh authored
The current CHECK/DCHECK implementation fails statically if a signed value is compared against an unsigned value. The common solution is to cast on each caller, which is tedious and error-prone (might hide bugs). This CL implements signed vs. unsigned comparisons by executing up to two comparisons. For example, if i is int32_t and u is uint_32_t, a DCHECK_LE(i, u) would create the check i <= 0 || static_cast<uint32_t>(i) <= u. For checks against constants, at least one of the checks can be removed by compiler optimizations. The tradeoff we have to make is to sometimes silently execute an additional comparison. And we increase code complexity of course, even though the usage is just as easy (or even easier) as before. The compile time impact seems to be minimal: I ran 3 full compilations for Optdebug on my local machine, one time on the current ToT, one time with this CL plus http://crrev.com/2524093002. Before: 143.72 +- 1.21 seconds Now: 144.18 +- 0.67 seconds In order to check that the new comparisons are working, I refactored some DCHECKs in wasm to use the new magic, and added unit test cases. R=ishell@chromium.org, titzer@chromium.org CC=ahaas@chromium.org, bmeurer@chromium.org Committed: https://crrev.com/5925074a9dab5a8577766545b91b62f2c531d3dc Review-Url: https://codereview.chromium.org/2526783002 Cr-Original-Commit-Position: refs/heads/master@{#41275} Cr-Commit-Position: refs/heads/master@{#41411}
-
- 29 Nov, 2016 1 commit
-
-
clemensh authored
This not only potentially improves performance, but also avoids weird linker errors, like the one below, where I used Smi::kMinValue in a DCHECK_LE. > [421/649] LINK ./mksnapshot > FAILED: mksnapshot > src/base/logging.h|178| error: undefined reference to 'v8::internal::Smi::kMinValue' R=bmeurer@chromium.org, ishell@chromium.org Committed: https://crrev.com/76723502528c5af003fdffc3520632ea2a13fef3 Review-Url: https://codereview.chromium.org/2524093002 Cr-Original-Commit-Position: refs/heads/master@{#41273} Cr-Commit-Position: refs/heads/master@{#41363}
-
- 24 Nov, 2016 2 commits
-
-
clemensh authored
Revert of [base] Pass scalar arguments by value in CHECK/DCHECK (patchset #3 id:40001 of https://codereview.chromium.org/2524093002/ ) Reason for revert: Seems to cause compile errors on Android. Will investigate on Monday. Original issue's description: > [base] Pass scalar arguments by value in CHECK/DCHECK > > This not only potentially improves performance, but also avoids weird > linker errors, like the one below, where I used Smi::kMinValue in a > DCHECK_EQ. > > > [421/649] LINK ./mksnapshot > > FAILED: mksnapshot > > src/base/logging.h|178| error: undefined reference to > 'v8::internal::Smi::kMinValue' > > R=bmeurer@chromium.org, ishell@chromium.org > > Committed: https://crrev.com/76723502528c5af003fdffc3520632ea2a13fef3 > Cr-Commit-Position: refs/heads/master@{#41273} TBR=bmeurer@chromium.org,ishell@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true Review-Url: https://codereview.chromium.org/2527883004 Cr-Commit-Position: refs/heads/master@{#41278}
-
clemensh authored
This not only potentially improves performance, but also avoids weird linker errors, like the one below, where I used Smi::kMinValue in a DCHECK_EQ. > [421/649] LINK ./mksnapshot > FAILED: mksnapshot > src/base/logging.h|178| error: undefined reference to 'v8::internal::Smi::kMinValue' R=bmeurer@chromium.org, ishell@chromium.org Review-Url: https://codereview.chromium.org/2524093002 Cr-Commit-Position: refs/heads/master@{#41273}
-
- 30 Jan, 2015 3 commits
-
-
bmeurer authored
R=svenpanne@chromium.org Review URL: https://codereview.chromium.org/877753007 Cr-Commit-Position: refs/heads/master@{#26346}
-
Benedikt Meurer authored
This reverts commit 6a4c0a3b and commit 0deaa4b6 for breaking GCC bots. TBR=svenpanne@chromium.org Review URL: https://codereview.chromium.org/893533003 Cr-Commit-Position: refs/heads/master@{#26342}
-
bmeurer authored
R=svenpanne@chromium.org Review URL: https://codereview.chromium.org/888613002 Cr-Commit-Position: refs/heads/master@{#26340}
-