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

Wdeprecated #4665

Merged
merged 1 commit into from
Jun 19, 2024
Merged

Conversation

andrei-sandor
Copy link
Contributor

Added ITK_DEPRECATED when \deprecated is used in a comment. They must both come together. Applied to structs, classes and functions

@github-actions github-actions bot added area:Core Issues affecting the Core module area:Filtering Issues affecting the Filtering module labels May 15, 2024
Copy link
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

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

Generally looks good, but CI should be green.

@@ -32,8 +32,7 @@
* (i.e. itkNotUsed cannot be used) but is not always used.
*/
template <typename T>
[[deprecated("Preferably use the C++ `[[maybe_unused]]` attribute instead!")]] inline void
Copy link
Member

Choose a reason for hiding this comment

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

@seanm
Copy link
Contributor

seanm commented May 31, 2024

So the issue here is that -Wdocumentation warns if a function/class is documented with a \deprecated but isn't also tagged [[deprecated]] for the compiler. Indeed, it generally makes sense that you should have both or neither.

But there are some tricky cases here:

This struct is "deprecated":

/**
   * \brief A structure which enable changing any image class' pixel
   * type to another.
   *
   *
   * \sa Image::Rebind
   * \deprecated Use template alias RebindImageType instead
   */
  template <typename UPixelType, unsigned int VUImageDimension = VImageDimension>
  struct Rebind
  {
    using Type = itk::VectorImage<UPixelType, VUImageDimension>;
  };

but its replacement references the deprecated struct:

using RebindImageType = typename Rebind<UPixelType, VUImageDimension>::Type;

If the struct is tagged [[deprecated]] then using it above triggers a warning about using a deprecated class.


What do you all think of just changing the \deprecated in these cases to just Deprecated:?

@seanm
Copy link
Contributor

seanm commented Jun 13, 2024

@andrei-sandor please amend the commit messge, as "COMP: Added ITK_DEPRECATED when \deprecated there" is no longer true.

… of code

This is used for struct,classes and functions. Decided to not use \deprecated Doxygen command and [[deprecated]] at the same time.
@seanm
Copy link
Contributor

seanm commented Jun 19, 2024

/azp ITK.macOS

Copy link
Member

@thewtex thewtex left a comment

Choose a reason for hiding this comment

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

🔢

@thewtex thewtex merged commit b5771fa into InsightSoftwareConsortium:master Jun 19, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:Core Issues affecting the Core module area:Filtering Issues affecting the Filtering module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants