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.

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

Generate impl UniquePtr for abstract types too #1137

Merged
merged 1 commit into from
Aug 1, 2022

Conversation

bsilver8192
Copy link
Contributor

Similarly to non-abstract types, if one include_cpp! section
generate!s an abstract type but never uses unique_ptr for that type,
but another one does, we need the explicit impl UniquePtr so that cxx
allows it.

@bsilver8192 bsilver8192 force-pushed the impl-abstract-type branch 2 times, most recently from a50fc60 to 42d7d46 Compare July 26, 2022 08:46
Similarly to non-abstract types, if one `include_cpp!` section
`generate!`s an abstract type but never uses `unique_ptr` for that type,
but another one does, we need the explicit `impl UniquePtr` so that cxx
allows it.
@adetaylor
Copy link
Collaborator

Thanks!

Note to my future self:

I'm going to go ahead and merge this. It makes me a little nervous since (a) I don't claim to have my head around the requirements which std::unique_ptr places on its contained types; (b) I am concerned that there might be cases where we previously allowed UniquePtr<T> and we no longer do so, which might not be adequately represented in the test suite. But if they're not adequately represented in the test suite, that's my fault...

@adetaylor adetaylor merged commit d0bc76e into google:main Aug 1, 2022
@bsilver8192 bsilver8192 deleted the impl-abstract-type branch August 1, 2022 18:00
frc971-automation pushed a commit to frc971/971-Robot-Code that referenced this pull request Aug 17, 2022
Backport of google/autocxx#1137.

Change-Id: I80e69a93880adf3f907078a55fbdb4e99f15ff2b
Signed-off-by: Brian Silverman <bsilver16384@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants