Skip to content

[WIP] Series: Non-Copyable#814

Closed
ax3l wants to merge 1 commit intoopenPMD:devfrom
ax3l:fix-SeriesNoCopy
Closed

[WIP] Series: Non-Copyable#814
ax3l wants to merge 1 commit intoopenPMD:devfrom
ax3l:fix-SeriesNoCopy

Conversation

@ax3l
Copy link
Member

@ax3l ax3l commented Nov 3, 2020

Delete the Copy-constructor in Series and add a move-constructor to Series and Attributable. This is a quick-fix for currently inconsistent handle-creation during copy.

Reproducer and long-term strategy as handle: #804

To Do

  • tests for move constructor in Attributable and Series
  • entry in NEWS
  • Python also affected?

Delete the Copy-constructor in Series and add a move-constructor to
Series and Attributable. This is a quick-fix for currently
inconsistent handle-creation during copy.
@franzpoeschel
Copy link
Contributor

franzpoeschel commented Nov 12, 2020

Reason for the segfault:

In this line, pointer s goes not to the move-constructed new Series object copy, but to the old (and now invalid) instance o0. Essentially, the Series class is currently (via some detours) self-referential and even moving it will break things.

The old test only really ever ran fine by accident: Flushing from the copy was ok, since o0 was still around to be used. Now that it's moved from, the test breaks.

So, for a quick fix, I'd suppose that move operations must be disallowed too. For a long-term fix, pointers and references must only go to (internal) classes without copy and move constructors.

Also, I think that Series::~Series would currently fail on a moved-from Series object, likely on a null-pointer dereference of a moved-from Container::m_container.

@ax3l
Copy link
Member Author

ax3l commented Apr 21, 2021

Fixed with #886 #936 #955

@ax3l ax3l closed this Apr 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants