Skip to content

script: Replace RAII of CallSetup and AutoEntryScript with a function wrapper#42715

Merged
sagudev merged 3 commits intoservo:mainfrom
sagudev:the-setup
Feb 25, 2026
Merged

script: Replace RAII of CallSetup and AutoEntryScript with a function wrapper#42715
sagudev merged 3 commits intoservo:mainfrom
sagudev:the-setup

Conversation

@sagudev
Copy link
Copy Markdown
Member

@sagudev sagudev commented Feb 19, 2026

As mentioned in #40600 we will need this to properly handle &mut JSContext (we cannot pass it to drop) and I think this wrappers are preferred ways of doing this in rust as it is safe to forget (not call drop) which is unsound in this case (due to realm stack at least).

Revieable per commits.

Testing: Just a refactor, but should be covered by WPT

@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Feb 19, 2026
@sagudev sagudev requested a review from jdm February 19, 2026 16:24
@sagudev sagudev mentioned this pull request Feb 21, 2026
1 task
@sagudev sagudev changed the title script: Replace RAII of CallSetup with a function wrapper script: Replace RAII of CallSetup and AutoEntryScript with a function wrapper Feb 21, 2026
Copy link
Copy Markdown
Member

@jdm jdm left a comment

Choose a reason for hiding this comment

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

Makes sense!

@servo-highfive servo-highfive added S-needs-rebase There are merge conflict errors. and removed S-awaiting-review There is new code that needs to be reviewed. labels Feb 24, 2026
@servo-highfive servo-highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-needs-rebase There are merge conflict errors. labels Feb 25, 2026
@sagudev sagudev enabled auto-merge February 25, 2026 09:04
@sagudev sagudev added this pull request to the merge queue Feb 25, 2026
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Feb 25, 2026
Merged via the queue into servo:main with commit 6c334f2 Feb 25, 2026
33 checks passed
@sagudev sagudev deleted the the-setup branch February 25, 2026 10:15
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Feb 25, 2026
github-merge-queue bot pushed a commit that referenced this pull request Feb 25, 2026
The span needs to be assigned to a variable, so that it drops at the end
of scope.
This was introduced in #42715. 
Additionally also switch tho the profile_traits macro to simplify the
statement.

Testing: Manually verified the warning is fixed.

Signed-off-by: Jonathan Schwender <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Feb 28, 2026
As in #42715 we will need cx in drop
so we need to go to function wrapper pattern.

Testing: Just refactor should be covered by WPT tests.

---------

Signed-off-by: sagudev <[email protected]>
shubhamg13 pushed a commit to shubhamg13/servo that referenced this pull request Mar 2, 2026
The span needs to be assigned to a variable, so that it drops at the end
of scope.
This was introduced in servo#42715. 
Additionally also switch tho the profile_traits macro to simplify the
statement.

Testing: Manually verified the warning is fixed.

Signed-off-by: Jonathan Schwender <[email protected]>
simonwuelker pushed a commit to simonwuelker/servo that referenced this pull request Mar 3, 2026
…#42905)

As in servo#42715 we will need cx in drop
so we need to go to function wrapper pattern.

Testing: Just refactor should be covered by WPT tests.

---------

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

S-awaiting-review There is new code that needs to be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants