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

NVCC + C++17 #1103

Merged
merged 2 commits into from
Sep 22, 2021
+6 −1
Merged

NVCC + C++17 #1103

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e6c7e1c
NVCC + C++17
ax3l Sep 15, 2021
284512c
NVCC 11.0.2 C++17 work-around: Add Comment
ax3l Sep 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion include/openPMD/backend/Attributable.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,8 @@ AttributableInterface::setAttribute( std::string const & key, T value )
&& !attri.m_attributes.key_comp()(key, it->first) )
{
// key already exists in map, just replace the value
it->second = Attribute(value);
Attribute::resource val(value);
it->second = Attribute(std::move(val));
Copy link
Member Author

@ax3l ax3l Sep 15, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Just FYI, this triggers the same issue:

Suggested change
Attribute::resource val(value);
it->second = Attribute(std::move(val));
it->second = Attribute(std::move(value));

Copy link
Contributor

@franzpoeschel franzpoeschel Sep 15, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Could you test if this one works:

Attribute::resource val(std::move(value));
it->second = Attribute(std::move(vale));

or even:

it->second = Attribute(Attribute::resource(std::move(value)));

That way would at least be without copying

Also, let's put a comment there to make it clear that this is purposefully strange

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea. I have no access to Cori due to maintenance today and will test it tomorrow.

Copy link
Member Author

Choose a reason for hiding this comment

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

Tested it now, the explicit construction of the resource in place works as a work-around, too :)
Added also a comment :)

return true;
} else
{
Expand Down