Skip to content

Conversation

@codebytere
Copy link
Member

@codebytere codebytere commented Jul 17, 2024

In utils.h's ERROR_AND_ABORT macro, rename the static local variable args to avoid -Wshadow warnings in code that calls ERROR_AND_ABORT(). This allows Electron to remove a patch.

Relevant console output:

In file included from ../../third_party/electron_node/src/inspector/runtime_agent.cc:3:
In file included from ../../third_party/electron_node/src/env-inl.h:32:
../../third_party/electron_node/src/node_internals.h:72:3: error: declaration shadows a local variable [-Werror,-Wshadow]
   72 |   CHECK(args[0]->IsObject());
      |   ^
../../third_party/electron_node/src/util.h:154:7: note: expanded from macro 'CHECK'
  154 |       ERROR_AND_ABORT(expr);                                                  \
      |       ^
../../third_party/electron_node/src/util.h:132:38: note: expanded from macro 'ERROR_AND_ABORT'
  132 |     static const node::AssertionInfo args = {                                 \
      |                                      ^
../../third_party/electron_node/src/node_internals.h:67:67: note: previous declaration is here
   67 | void GetSockOrPeerName(const v8::FunctionCallbackInfo<v8::Value>& args) {
      |                                                                   ^
1 error generated.

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Jul 17, 2024
@codebytere codebytere force-pushed the remove-shadowed-variable branch from 5d94fd5 to adc2a57 Compare July 17, 2024 13:09
@lpinca lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 18, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 18, 2024
@nodejs-github-bot
Copy link
Collaborator

@codebytere codebytere removed the needs-ci PRs that need a full CI run. label Jul 23, 2024
@codebytere
Copy link
Member Author

Haven't landed anything in a while so double checking - is this author-ready and ready to land or is there anything else I need to do?

@tniessen tniessen added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. labels Jul 24, 2024
@tniessen
Copy link
Member

Yes, indeed :)

@targos
Copy link
Member

targos commented Jul 24, 2024

It looks ready to me! You can add the commit-queue label to land it.

@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 24, 2024
@nodejs-github-bot nodejs-github-bot merged commit e440540 into nodejs:main Jul 24, 2024
@nodejs-github-bot
Copy link
Collaborator

Landed in e440540

@codebytere codebytere deleted the remove-shadowed-variable branch July 24, 2024 09:53
@RafaelGSS RafaelGSS mentioned this pull request Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants