Skip to content

Remove wrong handling of *const Handle in wrappers#680

Merged
jdm merged 2 commits intoservo:mainfrom
sagudev:the_handle
Dec 16, 2025
Merged

Remove wrong handling of *const Handle in wrappers#680
jdm merged 2 commits intoservo:mainfrom
sagudev:the_handle

Conversation

@sagudev
Copy link
Copy Markdown
Member

@sagudev sagudev commented Dec 10, 2025

Handling of *const Handle was wrong in wrap! macro. It generated code like this:

SM(&(*arg).into())

but this is wrong as we get ptr to invalid place (args are temporary values) cause crashing. I am not sure why rust was not complaining, but I suspect it has something to do with the fact that we change ref to ptrs. The solution is to do this:

let t = (*arg).into();
SM(&t);
SM(&raw const t);

but this is hard to do in macro, so I simply removed handling of this case and wrote a manual wrapper. Servo currently uses nonwrapped function call as wrapped would cause crashes.

Signed-off-by: sagudev <[email protected]>
@sagudev
Copy link
Copy Markdown
Member Author

sagudev commented Dec 10, 2025

t
Signed-off-by: sagudev <[email protected]>
@sagudev
Copy link
Copy Markdown
Member Author

sagudev commented Dec 12, 2025

What I think is happens here that we get temporary & that is then deref into ptr and this does not trigger error, but using &raw const (which is more correct anyway) will complain as expected:

error[E0745]: cannot take address of a temporary
    --> mozjs/src/rust.rs:1532:28
     |
1532 |                 &raw const ownDesc.into(),
     |                            ^^^^^^^^^^^^^^ temporary value

@sagudev sagudev marked this pull request as ready for review December 12, 2025 11:53
@sagudev sagudev requested a review from jdm December 12, 2025 12:12
@jdm jdm added this pull request to the merge queue Dec 16, 2025
Merged via the queue into servo:main with commit 691c711 Dec 16, 2025
36 checks passed
github-merge-queue bot pushed a commit to servo/servo that referenced this pull request Dec 25, 2025
This is companion PR to servo/mozjs#680 but it
was originally written as part of #40716.

Testing: Covered by
`/webidl/ecmascript-binding/global-object-implicit-this-value-cross-realm.html`

Signed-off-by: sagudev <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants