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

Only instantiate moveit-style constructors and implement make_unique in terms of them #742

Closed
wants to merge 7 commits into from
Closed

Only instantiate moveit-style constructors and implement make_unique in terms of them #742

wants to merge 7 commits into from

Conversation

adetaylor
Copy link
Collaborator

No description provided.

This was referenced Jan 14, 2022
Merged
Closed
@adetaylor
Copy link
Collaborator Author

Having thought this through, I don't think this is the right approach. It simplifies things a bit, but not much because we still need to instantiate the alloc/dealloc functions for each type.

More seriously, this means we'd be calling std::allocator to allocate each heap cell. I suspect that's no good if a type has an overridden operator new; we might end up allocating things on the wrong heap.

The approach prior to this PR will have ended up directly calling the operator new by means of make_unique and thus would have put things on the correct heap. It would be a regression for us to land this PR, so I'm not going to. There may still be opportunities to simplify the synthesis of make_unique functions some other way.

@adetaylor adetaylor closed this Jan 27, 2022
@adetaylor
Copy link
Collaborator Author

This will be OK once we've fixed #772 so reopening. (The whole PR needs redoing, though.)

@adetaylor adetaylor reopened this Feb 8, 2022
@adetaylor adetaylor force-pushed the constructors-always-via-moveit branch from 76c80e4 to 10b0266 Compare February 16, 2022 00:32
@adetaylor adetaylor mentioned this pull request Feb 17, 2022
Closed
@adetaylor adetaylor force-pushed the constructors-always-via-moveit branch from 10b0266 to 29da394 Compare March 9, 2022 05:36
@adetaylor adetaylor force-pushed the constructors-always-via-moveit branch 2 times, most recently from d7af94e to da2f776 Compare April 1, 2022 21:22
This was referenced Apr 2, 2022
Closed
Closed
Closed
adetaylor added 7 commits April 1, 2022 21:45
@adetaylor
Add derive_make_unique macro
0b9b659
@adetaylor
Use #[derive_make_unique]
2e09c8b
@adetaylor
Fix problems.
23e0c98
@adetaylor
Move phase
ce2d920
@adetaylor
Remove unnecessary code
be444a3
@adetaylor
Ensure subclasses gain destructors from supers.
6a8731f 
Previously we didn't populate a destructor (Drop) trait for a synthesized
subclass even if the superclass had a destructor. We should have done.

The general problem of derived classes being unaware of their superclass
destructors is #988. The present fix only applies to synthesized subclasses.

As well as being incorrect, this also prevented subclasses gaining a
make_unique method.
@adetaylor
Clippy fixes.
0035433
@adetaylor adetaylor force-pushed the constructors-always-via-moveit branch from 9f3bf7e to 0035433 Compare April 2, 2022 04:47
@adetaylor
Copy link
Collaborator Author

We'll do #1040 instead. We deprecated the make_unique a few releases ago.

@adetaylor adetaylor closed this Apr 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@adetaylor