Skip to content
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.

Sign up for GitHub

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

Jump to bottom

deps: V8: fix debug build #44392

Closed
wants to merge 1 commit into from
Closed

deps: V8: fix debug build #44392

wants to merge 1 commit into from

Conversation

bnoordhuis
Copy link
Member

This is a (very) partial cherry-pick of upstream change v8/v8@f8fddd6b13
that fixes the following debug mode build error with clang:

../deps/v8/src/codegen/register.h:29:48: error: member reference
base type 'RegListBase' is not a structure or union
  int num_different_regs = RegListBase{regs...}.Count();

Fixes: #44371

@bnoordhuis
deps: V8: fix debug build
ce33a31 
This is a (very) partial cherry-pick of upstream change v8/v8@f8fddd6b13
that fixes the following debug mode build error with clang:

    ../deps/v8/src/codegen/register.h:29:48: error: member reference
    base type 'RegListBase' is not a structure or union
      int num_different_regs = RegListBase{regs...}.Count();

Fixes: nodejs#44371
@bnoordhuis bnoordhuis requested a review from targos August 25, 2022 11:19
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/v8-update

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. v18.x Issues that can be reproduced on v18.x or PRs targeting the v18.x-staging branch. v8 engine Issues and PRs related to the V8 dependency. labels Aug 25, 2022
@bnoordhuis bnoordhuis removed the needs-ci PRs that need a full CI run. label Aug 25, 2022
@bnoordhuis
Copy link
Member Author

I'm removing the needs-ci label because I don't think CI builds V8 in debug mode anywhere.

joyeecheung
joyeecheung approved these changes Aug 25, 2022
View reviewed changes
Copy link
Member

@joyeecheung joyeecheung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had the same patch locally before V8 was updated

@bnoordhuis bnoordhuis added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Aug 27, 2022
@bnoordhuis bnoordhuis added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 29, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 29, 2022
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/46305/

This was referenced Aug 31, 2022
Open
Open
Open
Open
Open
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/46421/

@github-actions github-actions bot mentioned this pull request Sep 6, 2022
Open
17 tasks
@github-actions github-actions bot mentioned this pull request Sep 7, 2022
Open
15 tasks
@RafaelGSS RafaelGSS force-pushed the v18.x-staging branch 2 times, most recently from c0cfb14 to 8ef5c40 Compare September 7, 2022 16:28
This was referenced Sep 8, 2022
Open
Open
Open
Open
@RafaelGSS
Copy link
Member

@bnoordhuis I'm about to create the next v18 proposal. But, looks like this commit isn't ready (CI red). In case you want to include it in the next v18 let me know

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/47068/

@trevnorris
Copy link
Contributor

Can we do something about blocking PRs because of faulty CI? This fix really needs to land. I'm needing to cherry-pick this commit constantly so I can test my code.

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/47074/

@github-actions github-actions bot mentioned this pull request Oct 5, 2022
Open
19 tasks
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/47079/

danielleadams pushed a commit that referenced this pull request Oct 5, 2022
@bnoordhuis @danielleadams
deps: V8: fix debug build
c892f35 
This is a (very) partial cherry-pick of upstream change v8/v8@f8fddd6b13
that fixes the following debug mode build error with clang:

    ../deps/v8/src/codegen/register.h:29:48: error: member reference
    base type 'RegListBase' is not a structure or union
      int num_different_regs = RegListBase{regs...}.Count();

Fixes: #44371
PR-URL: #44392
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
@danielleadams
Copy link
Contributor

Landed in c892f35

This was referenced Oct 6, 2022
Open
Open
Open
Open
Open
Open
@danielleadams danielleadams mentioned this pull request Oct 11, 2022
Merged
@panva panva removed the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RafaelGSS RafaelGSS RafaelGSS approved these changes

@himself65 himself65 himself65 approved these changes

@joyeecheung joyeecheung joyeecheung approved these changes

@gengjiawen gengjiawen gengjiawen approved these changes

@cjihrig cjihrig cjihrig approved these changes

@jasnell jasnell jasnell approved these changes

@targos targos Awaiting requested review from targos

Assignees
No one assigned
Labels
commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. v8 engine Issues and PRs related to the V8 dependency. v18.x Issues that can be reproduced on v18.x or PRs targeting the v18.x-staging branch.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.