Skip to content

fix IO::RemoveVariable/Attribute

Podhorszki, Norbert requested to merge github/fork/germasch/pr/rmvar into master

Created by: germasch

For background, see #1193 (closed) .

I had suggested a fix in PR #1267, which was rejected. @williamfgc then fixed it himself in #1278, except that that fix only fixed part of the issue. (I.e., it made sure that indices would be unique even after the variable is removed from the 2nd level map, but still failed to actually do the removal.)

As I already said before, @williamfgc's fix for the uniqueness issue was clearly better than mine.

I can't help but point out, though, that the I don't agree with the contention that making data types that wrap STL containers is categorically a bad idea. Specifically, if one were to wrap these data types, one could prevent bugs where a data structure is mistakenly copied in its entirety when taking a reference was intended (by deleting the copy constructor). This PR fixes the places where that currently happened in the code, including actual bugs (that prevent 2nd level removal) and performance bugs (taking a copy doesn't hurt, but is unnecessarily expensive). It doesn't to anything do prevent these kind of bugs from happening again in the future.

Merge request reports