Skip to content

Replace some calls to new with make_{unique,shared} #160581

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
Closed
wants to merge 3 commits into from
Closed

Replace some calls to new with make_{unique,shared} #160581

wants to merge 3 commits into from
+9 −10

Conversation

lakshayg
Copy link
Collaborator

No description provided.

@pytorch-bot PyTorch Bot
Copy link

pytorch-bot bot commented Aug 13, 2025 •
edited
Loading

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/160581

Note: Links to docs will display an error until the docs builds have been completed.

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

✅ No Failures

As of commit 287adb6 with merge base cd87f30 (image):
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@linux-foundation-easycla Linux Foundation: EasyCLA
Copy link

linux-foundation-easycla bot commented Aug 13, 2025 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

@lakshayg
Copy link
Collaborator Author

@pytorchbot label "topic: not user facing"

@pytorch-bot pytorch-bot bot added the topic: not user facing topic category label Aug 13, 2025
@cpuhrsch cpuhrsch requested a review from malfet August 14, 2025 05:18
@lakshayg lakshayg force-pushed the replace-new-with-make-unique branch from 3d042c6 to 135dc9f Compare August 14, 2025 21:49
Skylion007
Skylion007 reviewed Aug 15, 2025
View reviewed changes
aten/src/ATen/core/boxing/KernelFunction_impl.h
std::unique_ptr<Base>>
make_unique_base(Args&&... args) {
return std::unique_ptr<Base>(new Child(std::forward<Args>(args)...));
return std::make_unique<Child>(std::forward<Args>(args)...);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we not need to cast to Base?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, the cast happens implicitly https://en.cppreference.com/w/cpp/memory/unique_ptr.html

If T is a derived class of some base B, then unique_ptr<T> is implicitly convertible to unique_ptr<B>. The default deleter of the resulting unique_ptr<B> will use operator delete for B, leading to undefined behavior unless the destructor of B is virtual.

@lakshayg
Copy link
Collaborator Author

@pytorchmergebot rebase

@pytorchmergebot
Copy link
Collaborator

@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here

@pytorchmergebot
Copy link
Collaborator

Successfully rebased replace-new-with-make-unique onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via git checkout replace-new-with-make-unique && git pull --rebase)

@pytorchmergebot pytorchmergebot force-pushed the replace-new-with-make-unique branch from 135dc9f to 00b565c Compare August 21, 2025 20:14
@eqy
Copy link
Collaborator

eqy commented Aug 22, 2025

@pytorchmergebot rebase

@pytorchmergebot
Copy link
Collaborator

@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here

@pytorchmergebot
Copy link
Collaborator

Successfully rebased replace-new-with-make-unique onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via git checkout replace-new-with-make-unique && git pull --rebase)

@pytorchmergebot pytorchmergebot force-pushed the replace-new-with-make-unique branch from 00b565c to 74223f1 Compare August 22, 2025 18:07
@lakshayg
Copy link
Collaborator Author

@pytorchmergebot rebase

@pytorchmergebot
Copy link
Collaborator

@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here

@pytorchmergebot
Copy link
Collaborator

Successfully rebased replace-new-with-make-unique onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via git checkout replace-new-with-make-unique && git pull --rebase)

@pytorchmergebot pytorchmergebot force-pushed the replace-new-with-make-unique branch from 74223f1 to 55a63eb Compare August 26, 2025 20:17
@lakshayg
Copy link
Collaborator Author

@pytorchmergebot rebase

@pytorchmergebot
Copy link
Collaborator

@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here

@pytorchmergebot
Copy link
Collaborator

Successfully rebased replace-new-with-make-unique onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via git checkout replace-new-with-make-unique && git pull --rebase)

@pytorchmergebot pytorchmergebot force-pushed the replace-new-with-make-unique branch from 55a63eb to 287adb6 Compare August 27, 2025 00:49
@lakshayg
Copy link
Collaborator Author

@pytorchmergebot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Aug 27, 2025
@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: 1 jobs have failed, first few of them are: trunk / libtorch-linux-jammy-cuda12.8-py3.10-gcc11-debug / build

Details for Dev Infra team Raised by workflow job

@lakshayg
Copy link
Collaborator Author

@pytorchmergebot merge

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

markc-614 pushed a commit to markc-614/pytorch that referenced this pull request Sep 17, 2025
@lakshayg @markc-614
Replace some calls to new with make_{unique,shared} (pytorch#160581)
bab945a 
Pull Request resolved: pytorch#160581
Approved by: https://github.com/malfet
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Skylion007 Skylion007 Skylion007 left review comments

@malfet malfet malfet approved these changes

Assignees

No one assigned

Labels

ciflow/trunk Trigger trunk jobs on your pull request Merged open source topic: not user facing topic category

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@lakshayg @pytorchmergebot @eqy @Skylion007 @malfet @pytorchbot