COMP: Fix the WDocumentation \struct warning for template struct

[andrei-sandor]

Remove \struct Doxygen keyword from template struct

[andrei-sandor]

For this bug https://github.com/llvm/llvm-project/issues/92023 related to clang, the solution is to remove the \struc Doxygen comment. Should this modification be performed each time that we have a template/struct combination?

[N-Dekker]

If removing the \struct does not affect the Doxygen output, it's fine by me to have them all removed.

[seanm]

If removing the \struct does not affect the Doxygen output, it's fine by me to have them all removed.

@dzenanz @jhlegarreta if no one else objects, we can make this change everywhere. It seems to be the only way to work around that clang -Wdocumentation bug, and it would be nice to get to zero -Wdocumentation warnings.

[jhlegarreta]

@seanm I am not sure what is the right way to go, but Doxygen does have a struct command: https://www.doxygen.nl/manual/commands.html#cmdstruct

So the underlying problem may be another one.

[seanm]

@jhlegarreta the issue is that clang's -Wdocumentation warning is triggered by this construct:

/** \struct BooleanDispatch
 * \brief Templated class to produce a unique type "true" and "false".
 *
 * BooleanDispatch is a templated class that produce a unique type for
 * for "true" and for "false".  These types may be used to decide which
 * version of an overloaded function to call.
 */
template <bool>
struct BooleanDispatch
{};

It warns:

structDoxygenFinal.cxx:3:5: warning: '@struct' command should not be used in a comment attached to a non-struct declaration [-Wdocumentation]
 * \struct BooleanDispatch
   ~^~~~~~~~~~~~~~~~~~~~~~

I think it's confused by the intermediate presence of the template keyword. It thinks you're trying to attach a \struct comment to a template, and so it warns. This warning is a false positive in our opinion. But it can be worked around by just removing the \struct since the comment is directly above the struct, it's redundant.

So far, this is the only false positive we've seen from -Wdocumentation, which has caught other nice things, so I'd like to enable it on CI.

[jhlegarreta]

I will not oppose to it, but wondering if there is a way to tell clang -Wdocumentation to skip this particular line. And yes, enabling it in the CI would be great. Thanks Sean.

[seanm]

I will not oppose to it, but wondering if there is a way to tell clang -Wdocumentation to skip this particular line.

It could be wrapped with #pragma clang diagnostic ignored but that would be a major uglification. And @andrei-sandor's commit here is showing just one example so we could have this discussion, but there are dozens of instances of this pattern.

[jhlegarreta]

OK, as said, will not oppose. It's been a while since I do not have enough bandwidth to contribute/review as I'd like to, so go ahead with all changes as required without waiting for me.

[seanm]

@dzenanz how do you kick a bot again?

[dzenanz]

/azp.run ITK.macOS.Arm64

[dzenanz]

/azp run ITK.macOS.Arm64

[dzenanz]

I don't think either of those "commands" worked. I kicked it off via GUI.

[seanm]

I don't think either of those "commands" worked. I kicked it off via GUI.

Where in the GUI can one do this? Or can you because you have admin privileges?

Anyway, I'm trying to tell what's failing on that macOS bot. I see one test "itkFFTConvolutionImageFilterStreamingTest" failing, is that the issue? Is that test known-flaky? This PR changes only comments, so I don't get why the test would fail...

[dzenanz]

Clicking on the Details link next to the failure takes me here:

If I am not signed in I don't have the option to re-run the jobs.

[dzenanz]

@thewtex might comment on the failing test flaky-ness.

[seanm]

Ah, thanks for pointing out where that button is!

[thewtex]

I am not familiar with the reported flakiness of itkFFTConvolutionImageFilterStreamingTest.

[seanm]

I am not familiar with the reported flakiness of itkFFTConvolutionImageFilterStreamingTest.

Hmm, well, re-runnning it for the 3rd time now has the CI green.

[seanm]

This looks ready to merge to me.