Skip to content

STYLE: Rename ITK_DISALLOW_COPY_AND_ASSIGN to ITK_DISALLOW_COPY_AND_MOVE#2053

Merged
dzenanz merged 2 commits intoInsightSoftwareConsortium:masterfrom
N-Dekker:Rename-ITK_DISALLOW_COPY
Oct 13, 2020
Merged

STYLE: Rename ITK_DISALLOW_COPY_AND_ASSIGN to ITK_DISALLOW_COPY_AND_MOVE#2053
dzenanz merged 2 commits intoInsightSoftwareConsortium:masterfrom
N-Dekker:Rename-ITK_DISALLOW_COPY

Conversation

@N-Dekker
Copy link
Copy Markdown
Contributor

@N-Dekker N-Dekker commented Oct 10, 2020

Clarifies that the macro does not just disallow copy and assign, but
also move operations. Note that in this context, the term "move" refers
to both move-construct and move-assign.

With this commit, the old macro name will remain available, as long as
ITK_FUTURE_LEGACY_REMOVE = OFF (which is the default).

@dzenanz
Copy link
Copy Markdown
Member

dzenanz commented Oct 10, 2020

This will cause another round of changes in remote modules, dependent libraries and application to update this macro. I am not sure whether this clarification is worth it.

Copy link
Copy Markdown
Member

@hjmjohnson hjmjohnson left a comment

Choose a reason for hiding this comment

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

I think all 3 functions should be listed in the newly named macro ITK_DISALLOW_COPY_MOVE_ASSIGN

I like the clarity. For the moment we could make the old name an alias (not legacy), but have a note to make it legacy at the next major update. This will give a long transition period for the new name.

@dzenanz
Copy link
Copy Markdown
Member

dzenanz commented Oct 10, 2020

@hjmjohnson That makes sense. For now, put under future legacy macro.

@N-Dekker
Copy link
Copy Markdown
Contributor Author

N-Dekker commented Oct 10, 2020

Thanks for your feedback, Hans and Dženan.

@hjmjohnson

I think all 3 functions should be listed in the newly named macro ITK_DISALLOW_COPY_MOVE_ASSIGN

3 functions? But the macro actually deletes four of them:

  1. The copy-constructor
  2. The copy-assignment operator
  3. The move-constructor
  4. The move-assignment operator

https://github.com/InsightSoftwareConsortium/ITK/blob/v5.1.1/Modules/Core/Common/include/itkMacro.h#L358-L362

You could mention all four in a macro name, but that would then be a very long macro name. Would you like that?

Conceptually the macro disallows both copy and move, so I would still prefer to name it ITK_DISALLOW_COPY_AND_MOVE.

@dzenanz

For now, put under future legacy macro.

You mean ITK_FUTURE_LEGACY_REMOVE, right, instead of ITK_LEGACY_REMOVE?

#if !defined(ITK_FUTURE_LEGACY_REMOVE)
// Old macro name calls the new one. 
#  define ITK_DISALLOW_COPY_AND_ASSIGN(TypeName) ITK_DISALLOW_COPY_AND_MOVE(TypeName)
#endif

Would be fine to me.

@hjmjohnson
Copy link
Copy Markdown
Member

@N-Dekker I agree with you regarding COPY_MOVE only in the name. Thanks for pointing this out.

Also good with future deprecation rather than current deprecation.

@N-Dekker N-Dekker force-pushed the Rename-ITK_DISALLOW_COPY branch from f31ab05 to c9e8d4d Compare October 10, 2020 13:08
Comment thread Modules/Core/Common/include/itkMacro.h
@N-Dekker N-Dekker marked this pull request as ready for review October 10, 2020 13:18
@N-Dekker
Copy link
Copy Markdown
Contributor Author

The problem I feel about the old macro name "ITK_DISALLOW_COPY_AND_ASSIGN" isn't only about the fact that it does not mention "move". To me, the expression "disallow copy and assign" sounds like "disallow dogs and pets". Even though not all pets are dogs, and not all dogs are pets, it sounds a bit funny to me. Hope that makes some sense to you 😺

Clarifies that the macro does not just disallow copy and assign, but
also move operations. Note that in this context, the term "move" refers
to both move-construct and move-assign.

With this commit, the old macro name will remain available, as long as
`ITK_FUTURE_LEGACY_REMOVE = OFF` (which is the default).
The macro `ITK_DISALLOW_COPY_AND_ASSIGN` was just declared
`ITK_FUTURE_LEGACY_REMOVE` with the previous commit, so now
call the new `ITK_DISALLOW_COPY_AND_MOVE` macro instead!
@N-Dekker N-Dekker force-pushed the Rename-ITK_DISALLOW_COPY branch from c9e8d4d to 51171fa Compare October 11, 2020 15:36
Copy link
Copy Markdown
Member

@hjmjohnson hjmjohnson left a comment

Choose a reason for hiding this comment

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

@N-Dekker THANKS! This is really great. Thank you for being persistent.

Copy link
Copy Markdown
Member

@blowekamp blowekamp left a comment

Choose a reason for hiding this comment

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

Using future legacy seems remove appropriate here.

@dzenanz dzenanz merged commit 4c0982d into InsightSoftwareConsortium:master Oct 13, 2020
@jhlegarreta
Copy link
Copy Markdown
Member

jhlegarreta commented Oct 14, 2020

Looks like some were left behind?
https://open.cdash.org/index.php?project=Insight
e.g.
https://open.cdash.org/viewBuildError.php?buildid=6817858

Or maybe these builds have some CMake legacy-related flags that make the itkMacro.h legacy #if block not to be taken into account?

@hjmjohnson
Copy link
Copy Markdown
Member

@jhlegarreta See #2059

N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Sep 16, 2021
The macro `ITK_DISALLOW_COPY_AND_ASSIGN` was declared `ITK_FUTURE_LEGACY_REMOVE` with ITK pull request InsightSoftwareConsortium/ITK#2053 commit InsightSoftwareConsortium/ITK@7d177ca "STYLE: Rename ITK_DISALLOW_COPY_AND_ASSIGN to ITK_DISALLOW_COPY_AND_MOVE" (included with ITK v5.2rc01)

This commit follows ITK commit InsightSoftwareConsortium/ITK@4c0982d
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Sep 16, 2021
The macro `ITK_DISALLOW_COPY_AND_ASSIGN` was declared `ITK_FUTURE_LEGACY_REMOVE` with ITK pull request InsightSoftwareConsortium/ITK#2053 commit InsightSoftwareConsortium/ITK@7d177ca "STYLE: Rename ITK_DISALLOW_COPY_AND_ASSIGN to ITK_DISALLOW_COPY_AND_MOVE" (included with ITK v5.2rc01)

This commit follows ITK commit InsightSoftwareConsortium/ITK@4c0982d
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Oct 27, 2023
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Oct 27, 2023
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request Apr 19, 2026
Fixes changes made in InsightSoftwareConsortium#2053. ITK_DISALLOW_COPY_AND_ASSIGN will be used if ITK_FUTURE_LEGACY_REMOVE=OFF.
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request Apr 19, 2026
Fixes changes made in InsightSoftwareConsortium#2053. ITK_DISALLOW_COPY_AND_ASSIGN will be used if ITK_FUTURE_LEGACY_REMOVE=OFF.
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request Apr 19, 2026
Fixes changes made in InsightSoftwareConsortium#2053. ITK_DISALLOW_COPY_AND_ASSIGN will be used if ITK_FUTURE_LEGACY_REMOVE=OFF.
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request Apr 19, 2026
Fixes changes made in InsightSoftwareConsortium#2053. ITK_DISALLOW_COPY_AND_ASSIGN will be used if ITK_FUTURE_LEGACY_REMOVE=OFF.
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request Apr 19, 2026
Fixes changes made in InsightSoftwareConsortium#2053. ITK_DISALLOW_COPY_AND_ASSIGN will be used if ITK_FUTURE_LEGACY_REMOVE=OFF.
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request Apr 19, 2026
Fixes changes made in InsightSoftwareConsortium#2053. ITK_DISALLOW_COPY_AND_ASSIGN will be used if ITK_FUTURE_LEGACY_REMOVE=OFF.
hjmjohnson pushed a commit that referenced this pull request Apr 25, 2026
Fixes changes made in #2053. ITK_DISALLOW_COPY_AND_ASSIGN will be used if ITK_FUTURE_LEGACY_REMOVE=OFF.
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request Apr 26, 2026
Fixes changes made in InsightSoftwareConsortium#2053. ITK_DISALLOW_COPY_AND_ASSIGN will be used if ITK_FUTURE_LEGACY_REMOVE=OFF.
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request Apr 27, 2026
Fixes changes made in InsightSoftwareConsortium#2053. ITK_DISALLOW_COPY_AND_ASSIGN will be used if ITK_FUTURE_LEGACY_REMOVE=OFF.
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request Apr 27, 2026
…OVE Fi...

Original subject: STYLE: Rename ITK_DISALLOW_COPY_AND_ASSIGN to ITK_DISALLOW_COPY_AND_MOVE Fixes changes made in InsightSoftwareConsortium#2053. ITK_DISALLOW_COPY_AND_ASSIGN will be used if ITK_FUTURE_LEGACY_REMOVE=OFF.
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request Apr 27, 2026
…OVE Fi...

Original subject: STYLE: Rename ITK_DISALLOW_COPY_AND_ASSIGN to ITK_DISALLOW_COPY_AND_MOVE Fixes changes made in InsightSoftwareConsortium#2053. ITK_DISALLOW_COPY_AND_ASSIGN will be used if ITK_FUTURE_LEGACY_REMOVE=OFF.
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request Apr 27, 2026
…OVE Fi...

Original subject: STYLE: Rename ITK_DISALLOW_COPY_AND_ASSIGN to ITK_DISALLOW_COPY_AND_MOVE Fixes changes made in InsightSoftwareConsortium#2053. ITK_DISALLOW_COPY_AND_ASSIGN will be used if ITK_FUTURE_LEGACY_REMOVE=OFF.
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request Apr 27, 2026
…OVE Fi...

Original subject: STYLE: Rename ITK_DISALLOW_COPY_AND_ASSIGN to ITK_DISALLOW_COPY_AND_MOVE Fixes changes made in InsightSoftwareConsortium#2053. ITK_DISALLOW_COPY_AND_ASSIGN will be used if ITK_FUTURE_LEGACY_REMOVE=OFF.
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request Apr 27, 2026
…OVE Fi...

Original subject: STYLE: Rename ITK_DISALLOW_COPY_AND_ASSIGN to ITK_DISALLOW_COPY_AND_MOVE Fixes changes made in InsightSoftwareConsortium#2053. ITK_DISALLOW_COPY_AND_ASSIGN will be used if ITK_FUTURE_LEGACY_REMOVE=OFF.
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request Apr 27, 2026
Fixes changes made in InsightSoftwareConsortium#2053. ITK_DISALLOW_COPY_AND_ASSIGN will be used if ITK_FUTURE_LEGACY_REMOVE=OFF.
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request Apr 27, 2026
…OVE Fi...

Original subject: STYLE: Rename ITK_DISALLOW_COPY_AND_ASSIGN to ITK_DISALLOW_COPY_AND_MOVE Fixes changes made in InsightSoftwareConsortium#2053. ITK_DISALLOW_COPY_AND_ASSIGN will be used if ITK_FUTURE_LEGACY_REMOVE=OFF.
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request Apr 27, 2026
…OVE Fi...

Original subject: STYLE: Rename ITK_DISALLOW_COPY_AND_ASSIGN to ITK_DISALLOW_COPY_AND_MOVE Fixes changes made in InsightSoftwareConsortium#2053. ITK_DISALLOW_COPY_AND_ASSIGN will be used if ITK_FUTURE_LEGACY_REMOVE=OFF.
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request Apr 28, 2026
Fixes changes made in InsightSoftwareConsortium#2053. ITK_DISALLOW_COPY_AND_ASSIGN will be used if ITK_FUTURE_LEGACY_REMOVE=OFF.
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request Apr 28, 2026
Fixes changes made in InsightSoftwareConsortium#2053. ITK_DISALLOW_COPY_AND_ASSIGN will be used if ITK_FUTURE_LEGACY_REMOVE=OFF.
hjmjohnson pushed a commit that referenced this pull request Apr 30, 2026
Fixes changes made in #2053. ITK_DISALLOW_COPY_AND_ASSIGN will be used if ITK_FUTURE_LEGACY_REMOVE=OFF.
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request May 4, 2026
Fixes changes made in InsightSoftwareConsortium#2053. ITK_DISALLOW_COPY_AND_ASSIGN will be used if ITK_FUTURE_LEGACY_REMOVE=OFF.
hjmjohnson pushed a commit that referenced this pull request May 5, 2026
Fixes changes made in #2053. ITK_DISALLOW_COPY_AND_ASSIGN will be used if ITK_FUTURE_LEGACY_REMOVE=OFF.
hjmjohnson pushed a commit that referenced this pull request May 5, 2026
Fixes changes made in #2053. ITK_DISALLOW_COPY_AND_ASSIGN will be used if ITK_FUTURE_LEGACY_REMOVE=OFF.
hjmjohnson pushed a commit that referenced this pull request May 5, 2026
Fixes changes made in #2053. ITK_DISALLOW_COPY_AND_ASSIGN will be used if ITK_FUTURE_LEGACY_REMOVE=OFF.
hjmjohnson pushed a commit that referenced this pull request May 5, 2026
Fixes changes made in #2053. ITK_DISALLOW_COPY_AND_ASSIGN will be used if ITK_FUTURE_LEGACY_REMOVE=OFF.
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request May 6, 2026
Fixes changes made in InsightSoftwareConsortium#2053. ITK_DISALLOW_COPY_AND_ASSIGN will be used if ITK_FUTURE_LEGACY_REMOVE=OFF.
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request May 6, 2026
Fixes changes made in InsightSoftwareConsortium#2053. ITK_DISALLOW_COPY_AND_ASSIGN will be used if ITK_FUTURE_LEGACY_REMOVE=OFF.
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request May 6, 2026
Fixes changes made in InsightSoftwareConsortium#2053. ITK_DISALLOW_COPY_AND_ASSIGN will be used if ITK_FUTURE_LEGACY_REMOVE=OFF.
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request May 6, 2026
Fixes changes made in InsightSoftwareConsortium#2053. ITK_DISALLOW_COPY_AND_ASSIGN will be used if ITK_FUTURE_LEGACY_REMOVE=OFF.
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request May 6, 2026
Fixes changes made in InsightSoftwareConsortium#2053. ITK_DISALLOW_COPY_AND_ASSIGN will be used if ITK_FUTURE_LEGACY_REMOVE=OFF.
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request May 6, 2026
Fixes changes made in InsightSoftwareConsortium#2053. ITK_DISALLOW_COPY_AND_ASSIGN will be used if ITK_FUTURE_LEGACY_REMOVE=OFF.
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request May 6, 2026
Fixes changes made in InsightSoftwareConsortium#2053. ITK_DISALLOW_COPY_AND_ASSIGN will be used if ITK_FUTURE_LEGACY_REMOVE=OFF.
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.

5 participants