GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||||||||||
GeezerVison is showing github.com — src: fix type mismatch warnings from missing priv by sam-github · Pull Request #24737 · nodejs/node Conversation
Contributor
Registration initialization functions are expected to have a 4th Checklist
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Registration initialization functions are expected to have a 4th argument, a void*, so add them where necessary to fix the warnings.
Collaborator
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
addaleax
approved these changes
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. LGTM, but I’m curious, how/where does this generate warnings? I’ve never seen any…
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
|
There was an error while loading. Please reload this page.
|
I guess that's why nobody has fixed them, then. When I'm working in src/, I can't see errors or warnings in my code, they are lost behind these: |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
It might just be a GCC 8 thing then. :) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
@addaleax What about the above, do you not see them, either? I see screen after screen of them. Its not clear to me how or even if they can be fixed. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@sam-github I don’t, no. I think we should maybe turn off this type of warning, or at least use pragmas to silence them? I’m not sure we could fix that one, either. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
binary propogation failed on linux & arm: resumed the build |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Hmmm, let's try a full-on rebuild CI: https://ci.nodejs.org/job/node-test-pull-request/19126/ |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Given that this makes no changes that anything other than the compiler should notice, I don't think these are related:
I'm not sure, can this land? @Trott do we absolutely require green in CI? I'll rekick the CI for now. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
ci: https://ci.nodejs.org/job/node-test-pull-request/19157/ (rebuild of 19126) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
ci: https://ci.nodejs.org/job/node-test-pull-request/19161/ (new) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| } | ||
|
|
||
| static void GetPromiseDetails(const FunctionCallbackInfo<Value>& args) { | ||
| Environment* env = Environment::GetCurrent(args); |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
@addaleax Is it possible this call and the similar one below had side-effects? Its suspicious that https://ci.nodejs.org/job/node-test-commit-linux/nodes=alpine-last-latest-x64/23666/testReport/junit/(root)/test/parallel_test_util_callbackify/ is happening regularly, and depends on promise behaviours.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
@sam-github No, that seems very unlikely – the only side effect I could possibly think of is allocating a Local<> under the hood (which leaks into the calling HandleScope), but since these are JS bindings, there’s always a HandleScope here, and V8 doesn’t generally rely on them being used in a specific way
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
resume ci: https://ci.nodejs.org/job/node-test-commit/23915/ |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Rebuild Windows (only part that failed last time): https://ci.nodejs.org/job/node-test-commit-windows-fanned/22961/ |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
^--- the Windows failures all look like that. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
tried to restart node-test-commit: https://ci.nodejs.org/job/node-test-commit/23921/ |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
The Windows rebuild I did an hour ago came back yellow, so I think this can land. https://ci.nodejs.org/job/node-test-commit-windows-fanned/22961/ |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Registration initialization functions are expected to have a 4th argument, a void*, so add them where necessary to fix the warnings. PR-URL: #24737 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
Registration initialization functions are expected to have a 4th argument, a void*, so add them where necessary to fix the warnings. PR-URL: #24737 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
Registration initialization functions are expected to have a 4th argument, a void*, so add them where necessary to fix the warnings. PR-URL: nodejs#24737 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.