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

Use std::make_(shared|unique) #23757

Merged
merged 3 commits into from
Nov 4, 2023
Merged

Use std::make_(shared|unique) #23757

merged 3 commits into from
Nov 4, 2023

Conversation

notspiff
Copy link
Contributor

@notspiff notspiff commented Sep 12, 2023 •
edited by garbear
Loading

Description

Use std::make_shared / std::make_unique instead of .reset(new xxx)
Plus two drive-by cleanups for headers and a unused function.

Motivation and context

More expressive, for instance you can tell from call sites whether
it is a shared or unique ptr.

How has this been tested?

It builds

What is the effect on users?

None

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Clean up (non-breaking change which removes non-working, unmaintained functionality)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that will cause existing functionality to change)
  • Cosmetic change (non-breaking change that doesn't touch code)
  • Student submission (PR was done for educational purposes and will be treated as such)
  • None of the above (please explain below)

Checklist:

  • My code follows the Code Guidelines of this project
  • My change requires a change to the documentation, either Doxygen or wiki
  • I have updated the documentation accordingly
  • I have read the Contributing document
  • I have added tests to cover my change
  • All new and existing tests passed

@notspiff notspiff added Type: Cleanup non-breaking change which removes non-working or unmaintained functionality v21 Omega labels Sep 12, 2023
@notspiff notspiff added this to the Omega 21.0 Beta 1 milestone Sep 12, 2023
lrusak
lrusak approved these changes Sep 12, 2023
View reviewed changes
Copy link
Contributor

@lrusak lrusak left a comment

Choose a reason for hiding this comment

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

Nice cleanup 👍

ksooo
ksooo approved these changes Sep 12, 2023
View reviewed changes
Copy link
Member

@ksooo ksooo left a comment

Choose a reason for hiding this comment

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

definitely +1

@notspiff notspiff force-pushed the use_make_shared_unique branch 3 times, most recently from cf81ff7 to 12a0af8 Compare September 12, 2023 21:34
garbear
garbear approved these changes Sep 12, 2023
View reviewed changes
Copy link
Member

@garbear garbear left a comment

Choose a reason for hiding this comment

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

LGTM, nice cleanup

@Rechi
Copy link
Member

Rechi commented Sep 13, 2023

You may want to take a look at master...Rechi:xbmc:clang-tidy/modernize-make-shared_unique.
It includes all clang-tidy modernize-make-shared and modernize-make-unique fixes for Android, iOS, Linux, macOS and tvOS.

@notspiff
Copy link
Contributor Author

notspiff commented Sep 13, 2023 via email

@Rechi
Copy link
Member

Rechi commented Sep 13, 2023

I haven't pr'd it yet. Either you can pick the commits or, if desired, I'll create a PR.

@Rechi Rechi mentioned this pull request Oct 10, 2023
Merged
14 tasks
@jenkins4kodi jenkins4kodi added the Rebase needed PR that does not apply/merge cleanly to current base branch label Oct 12, 2023
@garbear
Copy link
Member

garbear commented Oct 12, 2023 •
edited
Loading

Is this preferred over #23904?

@notspiff
Copy link
Contributor Author

notspiff commented Oct 13, 2023 via email

@garbear
Copy link
Member

garbear commented Oct 13, 2023 •
edited
Loading

#23904 was merged. Do we still want the remaining parts of this PR?

@fuzzard fuzzard removed the v21 Omega label Oct 17, 2023
@garbear
Copy link
Member

garbear commented Oct 18, 2023

@notspiff I think the three removal commits are still valuable if you want to rebase the smart pointer commit out.

@notspiff
remove custom deleters
f513a2b 
these were rendered unnecessary when destructor was
moved to translation unit in eecc666
@notspiff
remove unused method
37f7801
@notspiff
remove unused headers
2bb4996
@jenkins4kodi jenkins4kodi removed the Rebase needed PR that does not apply/merge cleanly to current base branch label Nov 3, 2023
@notspiff
Copy link
Contributor Author

notspiff commented Nov 3, 2023

done.

garbear
garbear approved these changes Nov 3, 2023
View reviewed changes
Copy link
Member

@garbear garbear left a comment

Choose a reason for hiding this comment

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

PR is now mostly only removed code, so should be safe for v21.

@garbear garbear merged commit e549002 into xbmc:master Nov 4, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@garbear garbear garbear approved these changes

@sundermann sundermann sundermann approved these changes

@lrusak lrusak lrusak approved these changes

@ksooo ksooo ksooo approved these changes

@DaveTBlake DaveTBlake DaveTBlake approved these changes

@yol yol Awaiting requested review from yol

Assignees
No one assigned
Labels
Type: Cleanup non-breaking change which removes non-working or unmaintained functionality v21 Omega
Projects
None yet
Milestone
Omega 21.0 Beta 2
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@notspiff @Rechi @garbear @sundermann @lrusak @ksooo @DaveTBlake @fuzzard @jenkins4kodi