Skip to content

fix: native addon compilation with msvc#51695

Merged
VerteDinde merged 2 commits into
mainfrom
robo/fix_msvc_compile_error
May 19, 2026
Merged

fix: native addon compilation with msvc#51695
VerteDinde merged 2 commits into
mainfrom
robo/fix_msvc_compile_error

Conversation

@deepak1556
Copy link
Copy Markdown
Member

@deepak1556 deepak1556 commented May 19, 2026

Description of Change

Fixes #51678

Release Notes

Notes: Fixed native addon compilation failure with undefined msvc intrinsic from v8 headers

@deepak1556 deepak1556 added the semver/patch backwards-compatible bug fixes label May 19, 2026
@deepak1556 deepak1556 requested a review from a team as a code owner May 19, 2026 07:20
@deepak1556 deepak1556 added target/42-x-y PR should also be added to the "42-x-y" branch. target/43-x-y PR should also be added to the "43-x-y" branch. labels May 19, 2026
@electron-cation electron-cation Bot added the new-pr 🌱 PR opened recently label May 19, 2026
@timfish
Copy link
Copy Markdown
Contributor

timfish commented May 19, 2026

Could this be tested in CI?

nikwen
nikwen approved these changes May 19, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@nikwen nikwen left a comment

Choose a reason for hiding this comment

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

Thank you, Deepak! Looks good to me.

Would it make sense to upstream this patch?

@deepak1556
Copy link
Copy Markdown
Member Author

deepak1556 commented May 19, 2026

Could this be tested in CI?

Good call, our existing addons for specs https://github.com/electron/electron/tree/main/spec/fixtures/native-addon do get built with msvc however they didn't catch the issue due to missing dependency on cppgc header, I will add that.

Would it make sense to upstream this patch?

Given V8 dropped support for msvc and they will cleanup the guards, I don't intend to upstream.

@VerteDinde VerteDinde merged commit 0e1fe29 into main May 19, 2026
126 of 129 checks passed
@release-clerk
Copy link
Copy Markdown

release-clerk Bot commented May 19, 2026

Release Notes Persisted

Fixed native addon compilation failure with undefined msvc intrinsic from v8 headers

@VerteDinde VerteDinde deleted the robo/fix_msvc_compile_error branch May 19, 2026 20:17
This was referenced May 19, 2026
Merged
Merged
@trop
Copy link
Copy Markdown
Contributor

trop Bot commented May 19, 2026

I have automatically backported this PR to "43-x-y", please check out #51705

@trop trop Bot added in-flight/43-x-y and removed target/43-x-y PR should also be added to the "43-x-y" branch. labels May 19, 2026
@trop
Copy link
Copy Markdown
Contributor

trop Bot commented May 19, 2026

I have automatically backported this PR to "42-x-y", please check out #51706

@trop trop Bot added in-flight/42-x-y merged/42-x-y PR was merged to the "42-x-y" branch. merged/43-x-y PR was merged to the "43-x-y" branch. and removed target/42-x-y PR should also be added to the "42-x-y" branch. in-flight/42-x-y in-flight/43-x-y labels May 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jkleinsc jkleinsc jkleinsc approved these changes

@nikwen nikwen nikwen approved these changes

@dsanders11 dsanders11 dsanders11 approved these changes

Assignees

No one assigned

Labels

merged/42-x-y PR was merged to the "42-x-y" branch. merged/43-x-y PR was merged to the "43-x-y" branch. new-pr 🌱 PR opened recently semver/patch backwards-compatible bug fixes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Windows native-module build regression (__builtin_frame_address in headers)

6 participants

@deepak1556 @timfish @jkleinsc @nikwen @dsanders11 @VerteDinde