-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
[v12.x backport] src: ignore GCC -Wcast-function-type for v8.h 31475 31524 #31646
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Compiling a library with -fPIE won't do, and on Android libraries are not versioned. PR-URL: #29388 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Christian Clauss <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Rich Trott <[email protected]>
And other errors like lost promises PR-URL: #29388 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Christian Clauss <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Rich Trott <[email protected]>
PR-URL: #30863 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: #30840 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Yongsheng Zhang <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Minwoo Jung <[email protected]>
Disable colorMode in test-console-group so that the test will succeed if run without the test runner from the command line. PR-URL: #30886 Reviewed-By: Shelley Vohr <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Add information about what it means when colorMode is set to false. PR-URL: #30887 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
This commit updates two old links to Google's C++ styleguide which currently result in a 404 when accessed. PR-URL: #30876 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Rich Trott <[email protected]>
Revise compilation/execution support text to keep it shorter, simpler, and hopefully easier to read/understand. PR-URL: #30899 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Jiawen Geng <[email protected]>
doc change for stream.md that 'finish' event should be before writer.end fixes: #30759 PR-URL: #30889 Fixes: #30759 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Rich Trott <[email protected]>
PR-URL: #30877 Reviewed-By: David Carlier <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Rich Trott <[email protected]>
PR-URL: #30852 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Rich Trott <[email protected]>
Change '==' to '===' in v8_prof_polyfill.js, punycode.js. PR-URL: #30898 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: Rich Trott <[email protected]>
process.nextTick accepts additional parameters which are passed through to the callback. Use that instead of binding the function to a context. PR-URL: #28131 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
Pass through parameters using setImmediate rather than using Function.prototype.bind to bind the provided context. PR-URL: #28131 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
Don't use Function.prototype.bind where it isn't necessary. Rely on event emitter context instead and on arrow function as class property. PR-URL: #28131 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
PR-URL: #28131 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
PR-URL: #28131 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
This simplifies requires for those using DebugOptions, since debug_options was defined in src/node_options-inl.h and thus embedders would need to require an extra file to do what could trivially be consolidated into one. PR-URL: #30494 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: David Carlier <[email protected]>
OpenSSL uses a macro without typechecking; since C++ does not implicitly cast void* this is needed to conform Node.js to the OpenSSL documentation. PR-URL: #30917 Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]>
PR-URL: #30913 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
The return value is not a boolean and even if interpreted as one, it does not indicate whether an exception is pending. For napi_is_exception_pending, the description of the result parameter already explains how to check whether an exception is pending. PR-URL: #30893 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Gabriel Schulhof <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
Prior to this commit, running `node tools/lint-md --help` and `node tools/lint-md --version` resulted in an error. * Use `unified-args` to start processor * Use `unified-args`'s error handler Fixes: #30156 PR-URL: #30216 Reviewed-By: Rich Trott <[email protected]>
PR-URL: #30931 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
test-windows-failed-heap-allocation forces a out of mem crash resulting in a report file. To avoid a leftover in repo the child is started in a tmp folder like in test-report-fatal-error. PR-URL: #30925 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Prior to this commit, new contributors were suggested to use a utility to validate commit messages. Although not inaccurate, this utility produces misleading results. * Remove reference to `core-validate-commit` PR-URL: #30922 Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Rich Trott <[email protected]>
Adding tests covering promises-related code paths. PR-URL: #30777 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Ben Coe <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
I've been doing a lot of work on-and-off to reduce unnecessary wordiness in our docs. Codify it in the style guide. PR-URL: #30935 Reviewed-By: David Carlier <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: James M Snell <[email protected]>
Fixes: #30953 PR-URL: #30957 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: #30945 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]>
PR-URL: #30918 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
Original commit message: [coverage] Fix coverage with default arguments In the presence of default arguments, the body of the function gets wrapped into another block. This caused our trailing-range-after-return optimization to not apply, because the wrapper block had no source range assigned. This CL correctly assignes a source range to that block, which allows already present code to handle it correctly. Note that this is not a real coverage bug; we've just been reporting whitespace as uncovered. We're fixing it for consistency. Originally reported on github.com/bcoe/c8/issues/66 Bug: v8:9952 Change-Id: Iab3905f558eb99126e0dad8072d03d0a312fdcd3 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1903430 Commit-Queue: Sigurd Schneider <[email protected]> Reviewed-by: Toon Verwaest <[email protected]> Reviewed-by: Jakob Gruber <[email protected]> Cr-Commit-Position: refs/heads/master@{#64836} Refs: v8/v8@0dfd9ea Backport-PR-URL: #31412 PR-URL: #30713 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Rich Trott <[email protected]>
HTTP2ServerResponse.write would behave differently than both http1 and streams. This PR makes it more compliant with stream.Writable behaviour. Backport-PR-URL: #31444 PR-URL: #30964 Refs: #29529 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Rich Trott <[email protected]>
This implements ArrayBuffer#IsDetachedBuffer operation as per ECMAScript specification Section 24.1.1.2 https://tc39.es/ecma262/#sec-isdetachedbuffer Closes: #29955 Backport-PR-URL: #31422 PR-URL: #30613 Fixes: #29955 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
This makes sure that `util.format('%s', object)` will always call a user defined `toString` function. It was formerly not the case when the object had the function declared on the super class. At the same time this also makes sure that getters won't be triggered accessing the `constructor` property. Backport-PR-URL: #31431 PR-URL: #30343 Fixes: #30333 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>
This adds a couple of benchmarks to check different options and code paths. Backport-PR-URL: #31431 PR-URL: #30767 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Reviewed-By: Rich Trott <[email protected]>
This makes sure we do not retrieve the handler in case it's not required. This improves the performance a tiny bit for these cases. Backport-PR-URL: #31431 PR-URL: #30767 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Reviewed-By: Rich Trott <[email protected]>
Backport-PR-URL: #31431 PR-URL: #30767 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Reviewed-By: Rich Trott <[email protected]>
This makes sure that the regular expression matches all built-in objects properly. So far a couple where missed. Backport-PR-URL: #31431 PR-URL: #30768 Fixes: #30183 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
This is only active if the `showHidden` option is truthy. The implementation is a trade-off between accuracy and performance. This will miss properties such as properties added to built-in data types. The goal is mainly to visualize prototype getters and setters such as: class Foo { ownProperty = true get bar() { return 'Hello world!' } } const a = new Foo() The `bar` property is a non-enumerable property on the prototype while `ownProperty` will be set directly on the created instance. The output is similar to the one of Chromium when inspecting objects closer. The output from Firefox is difficult to compare, since it's always a structured interactive output and was therefore not taken into account. Backport-PR-URL: #31431 PR-URL: #30768 Fixes: #30183 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
This removes the special handling to inspect iterable objects with a null prototype. It is now handled together with the regular prototype. Backport-PR-URL: #31431 PR-URL: #30225 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Rich Trott <[email protected]>
Align the inspect output with the one used in the Chrome dev tools. A recent survey outlined that most users prefer to see the number of set and map entries. This should count as well for array sizes. The size is only added to regular arrays in case the constructor is not the default constructor. Typed arrays always indicate their size. Backport-PR-URL: #31431 PR-URL: #31027 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Anto Aravinth <[email protected]>
This makes sure this function stays backwards compatible in case it's accessed through the binding directly. Refs: #29947 (comment) Backport-PR-URL: #31431 PR-URL: #30858 Refs: #30767 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: Yongsheng Zhang <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
The fast path for the prototype inspection had a bug that caused some prototype properties to be skipped that should in fact be inspected. Backport-PR-URL: #31431 PR-URL: #31113 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rich Trott <[email protected]>
This makes sure the `generatedMessage` property is always set as expected. This was not the case some `assert.throws` and `assert.rejects` calls. Backport-PR-URL: #31431 PR-URL: #28263 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]>
This refactors some code for less duplication. Backport-PR-URL: #31431 PR-URL: #28263 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]>
This updates two outdated examples to the current implementation. Backport-PR-URL: #31431 PR-URL: #28263 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]>
This makes sure that `AssertionError` links to the correct place in the assert documentation. Backport-PR-URL: #31431 PR-URL: #28263 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]>
This adds a new functionality to the assertion module: a dedicated check for regular expressions. So far it's possible to use `assert.ok(regexp.test(string))`. This is not ideal though when it comes to the error message, since it's not possible to know how either of the input values look like. It's just known that the assertion failed. This allows to pass through the regular expression and the input string. The string is then matched against the regular expression and reports a expressive error message in case of a failure. Backport-PR-URL: #31431 PR-URL: #30929 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Stephen Belanger <[email protected]>
This commit suggests that cast-function-type warnings be ignored from v8.h. Currently, GCC reports a number of warnings like this: In file included from ../src/util.h:27, from ../src/aliased_buffer.h:7, from ../src/memory_tracker.h:5, from ../src/base_object.h:27, from ../src/async_wrap.h:27, from ../src/req_wrap.h:6, from ../src/req_wrap-inl.h:6, from ../src/connect_wrap.h:6, from ../src/connect_wrap.cc:1: ../deps/v8/include/v8.h: In instantiation of ‘void v8::PersistentBase<T>::SetWeak( P*, typename v8::WeakCallbackInfo<P>::Callback, v8::WeakCallbackType) [with P = node::BaseObject; T = v8::Object; typename v8::WeakCallbackInfo<P>::Callback = void (*)(const v8::WeakCallbackInfo<node::BaseObject>&)]’: ../src/base_object-inl.h:123:42: required from here ../deps/v8/include/v8.h:10374:16: warning: cast between incompatible function types from ‘v8::WeakCallbackInfo<node::BaseObject>::Callback’ {aka ‘void (*)(const v8::WeakCallbackInfo<node::BaseObject>&)’} to ‘Callback’ {aka ‘void (*)(const v8::WeakCallbackInfo<void>&)’} [-Wcast-function-type] reinterpret_cast<Callback>(callback), type); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ The motivation for doing this that it makes it difficult to spot other warnings that might be important. Since it is v8 that performs this cast I was not able to find a way around it. PR-URL: #31475 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Rich Trott <[email protected]>
Fixes: #31517 PR-URL: #31524 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Rich Trott <[email protected]>
addaleax
approved these changes
Feb 5, 2020
67ec97a
to
fc7b27e
Compare
@kasicka This would require a rebase against |
a01fb4a
to
e5dc2fb
Compare
63a03d2
to
d577190
Compare
The commits that were backported here are now cleanly landed on the v12.x-staging branch and included in the next 12.16.x release. Closing as this PR is no longer neccessary |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Checklist
Backport of #31475 and #31524
Builds fine with GCC 9.2.1 and 7.3.1
make -j4 test
(UNIX), orvcbuild test
(Windows) passes