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

Cleanup delegate-related code #107966

Merged
merged 5 commits into from
Sep 18, 2024
Merged

Cleanup delegate-related code #107966

merged 5 commits into from
Sep 18, 2024

Conversation

jkotas
Copy link
Member

@jkotas jkotas commented Sep 18, 2024

  • Deleted unused and unreachable code
  • Fix minor leak of delegate Stubs with collectible assemblies

@jkotas
Cleanup delegate-related code
e07fa49 
- Deleted unused and unreachable code
- Fix minor leak of delegate Stubs with collectible assemblies
@dotnet-policy-service Dotnet Policy Service
Copy link
Contributor

Tagging subscribers to this area: @mangod9
See info in area-owners.md if you want to be subscribed.

jkotas
jkotas commented Sep 18, 2024
View reviewed changes
src/coreclr/vm/class.h Show resolved Hide resolved
src/coreclr/vm/comdelegate.cpp Show resolved Hide resolved
src/coreclr/vm/stublink.cpp Show resolved Hide resolved
src/coreclr/vm/stublink.h Show resolved Hide resolved
src/coreclr/vm/stubmgr.cpp Show resolved Hide resolved
src/coreclr/vm/codeman.h Show resolved Hide resolved
@jkotas
Copy link
Member Author

jkotas commented Sep 18, 2024

Cleanup motivated by codereview of #104731

hoyosjs
hoyosjs approved these changes Sep 18, 2024
View reviewed changes
Copy link
Member

@hoyosjs hoyosjs left a comment

Choose a reason for hiding this comment

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

The stuff in the stubmgr and such lgtm

src/coreclr/vm/stubmgr.cpp Outdated Show resolved Hide resolved
src/coreclr/vm/stubmgr.cpp Outdated Show resolved Hide resolved
noahfalk
noahfalk approved these changes Sep 18, 2024
View reviewed changes
Copy link
Member

@noahfalk noahfalk left a comment

Choose a reason for hiding this comment

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

LGTM, a few minor suggestions inline

src/coreclr/vm/stublink.h Show resolved Hide resolved
src/coreclr/vm/stubmgr.cpp Outdated Show resolved Hide resolved
src/coreclr/vm/stubmgr.cpp Outdated Show resolved Hide resolved
@build-analysis build-analysis bot mentioned this pull request Sep 18, 2024
Open
3 tasks
janvorli
janvorli approved these changes Sep 18, 2024
View reviewed changes
Copy link
Member

@janvorli janvorli left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@jkotas jkotas merged commit 7b5f7d4 into dotnet:main Sep 18, 2024
88 of 90 checks passed
@jkotas jkotas deleted the cleanup branch September 18, 2024 16:26
sirntar pushed a commit to sirntar/runtime that referenced this pull request Sep 30, 2024
@jkotas @hoyosjs
@noahfalk @sirntar
Cleanup delegate-related code (dotnet#107966)
8497225 
* Cleanup delegate-related code
- Deleted unused and unreachable code
- Fix minor leak of delegate Stubs with collectible assemblies

---------

Co-authored-by: Juan Hoyos <[email protected]>
Co-authored-by: Noah Falk <[email protected]>
@github-actions github-actions bot locked and limited conversation to collaborators Oct 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@huoyaoyuan huoyaoyuan huoyaoyuan left review comments

@hoyosjs hoyosjs hoyosjs approved these changes

@janvorli janvorli janvorli approved these changes

@noahfalk noahfalk noahfalk approved these changes

Assignees

@jkotas jkotas

Labels
area-VM-coreclr
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jkotas @huoyaoyuan @noahfalk @janvorli @hoyosjs