Doxygen "unbalanced grouping commands" warnings

[jhlegarreta]

Description

The dashboard Doxygen build is showing a number of "unbalanced grouping commands" warnings:
https://open.cdash.org/viewBuildError.php?type=1&buildid=8176854

These issues are also being reported by the nightly ITKDoxygen build:
https://github.com/InsightSoftwareConsortium/ITKDoxygen/actions/runs/3120254526/jobs/5060690356#step:4:2789

Expected information

The Doxygen build should be clean.

Actual information

The groups seem to be present and working as expected in the impacted classes' Doxygen pages, but fixing the warnings is necessary to keep the toolkit healthy and to avoid distractions.

Versions

master

Additional Information

Related to PR #3640 (comment).

The relevant Doxygen source code class/method/line:
https://github.com/doxygen/doxygen/blob/ade61047308990ec679d1d59f0019a825e87451d/src/docgroup.cpp#L142

A few things I've looked into:

• Maybe the groups are not being closed properly in the CMake file defgroups somewhere in here:
  https://github.com/InsightSoftwareConsortium/ITK/blob/b870edf59bb0347116d245a445a2df8f5c6a8e56/CMake/ITKModuleDoxygen.cmake
  https://github.com/InsightSoftwareConsortium/ITK/blob/d12f14e105500362a30264e683a4045eed21ba14/CMake/ITKGroups.cmake
  Or else it's maybe a closing */ in here?
  https://github.com/InsightSoftwareConsortium/ITK/blob/b870edf59bb0347116d245a445a2df8f5c6a8e56/Utilities/Doxygen/itkgroup.pl
  e.g.
  

  ITK/Utilities/Doxygen/itkgroup.pl

  Line 74 in b870edf

  print "/**" . $savebuffer . "\n";

• Having a look at the first two warnings, which happen for itk::AutoPointer and itk::BackwardDifferenceOperator
  I don't see any hint in the group/module bar in Doxygen:
  
  

  However, when comparing to itk::Array which does not seem to trigger a warning
  

  It looks like the first two have each a defgroup (ITKSystemObjects for itk::AutoPointer and Operators for itk::BackwardDifferenceOperator) which is itself inside another group (ITKSystemObjects in SystemObjects and Operators in DataProcessing, respectively):
  

  ITK/Documentation/Doxygen/Modules.dox

  Line 510 in bcac6f9

  \ingroup SystemObjects

  Another randomly picked class which is also raising the warning, itk::NeighborhoodOperatorImageFilter, also contains the described nested group pattern (ImageFilters being in Filters).

  Maybe those nested groups are not being closed correctly when parsed by the above scripts. Inspecting the dox files that are generated by the CMake *.dox.in files and Doxygen itself (e.g. somewhere in the blaster.kitware machine) might provide a better idea. Otherwise, inspecting the artifacts available in ITKDoxygen:
  https://github.com/InsightSoftwareConsortium/ITKDoxygen/actions/runs/3120254526
  might also provide some hint.

Related Doxygen mailing list post:
https://sourceforge.net/p/doxygen/mailman/doxygen-users/thread/CAPUXxNEK00EJruPwV__AL3W_bmoMWXuGoKBysXC4XEUeLM_9Ag%40mail.gmail.com/#msg37710320

[jhlegarreta]

So I inspected the Doxygen build artifacts, e.g.
https://github.com/InsightSoftwareConsortium/ITKDoxygen/actions/runs/3269579446

and did not find any file that would contain an "unbalanced grouping commands" warning. The warnings are only shown in the GitHub actions build log, but are not present in any output file that is later read/rendered to the HTML files.

And as said before, I am unable to find a pattern that would raise those warnings.

As said, my only guess is that we are somehow generating some *.dox (?) files that are used by Doxygen to build the rendered HTML files and we are not closing (*/) the documentation block:
https://github.com/InsightSoftwareConsortium/ITK/blob/b870edf59bb0347116d245a445a2df8f5c6a8e56/CMake/ITKModuleDoxygen.cmake
which calls to
https://github.com/InsightSoftwareConsortium/ITK/blob/b870edf59bb0347116d245a445a2df8f5c6a8e56/Utilities/Doxygen/mcdoc.py
Or somewhere in the following Perl script that would open the documentation blocks but would not close them:
https://github.com/InsightSoftwareConsortium/ITK/blob/b870edf59bb0347116d245a445a2df8f5c6a8e56/Utilities/Doxygen/itkgroup.pl

I would have expected the output of the above scripts be present in the GHA build artifacts, but looks like the artifacts only contain the Doxygen documentation HTML pages.

So I'd dare to say that the only way this can be debugged is by building the documentation locally and checking the files generated by the above scripts.

The question to the Doxygen mailing list has not been answered either.

[albert-github]

A question has been created as https://stackoverflow.com/questions/74803895/fix-unbalanced-grouping-command-doxygen-warnings

Here I posted:

First question is which doxygen version are you using? I've seen references to 1.8.16 and 1.9.3, though the current version is 1.9.5, advise is to update to 1.9.5 for sure.
The warning warning: unbalanced grouping commands typically refers to code parts like /* @{ and /*@}, in the past these were correct but the /* refers to "normal" comment an not to doxygen comment so this has been corrected and one has to use doxygen types of comments (like /**). As far as I can see this has been done in version 1.8.16 (released August 8, 2019).

[albert-github]

It might have been that I found the problem, it looks like it is indeed in the perl script (as suggested somewhere).
I had a look at the file Modules/Core/Common/include/itkAutoPointer.h and translated this one with perl (like doxygen does):

perl.exe .../build/Utilities/Doxygen/itkdoxygen.pl ".../Modules/Core/Common/include/itkAutoPointer.h"

and at the end of the file we see:

/**@{ This templated function is intended to facilitate the
    transfer between AutoPointers of Derived class to Base class */
template <typename TAutoPointerBase, typename TAutoPointerDerived>
void
TransferAutoPointer(TAutoPointerBase & pa, TAutoPointerDerived & pb)
{
  // give a chance to natural polymorphism
  pa.TakeNoOwnership(pb.GetPointer());
  if (pb.IsOwner())
  {
    pa.TakeOwnership();    // pa Take Ownership
    pb.ReleaseOwnership(); // pb Release Ownership and clears
  }
}
} // end namespace itk
/**@}*/

#endif

We see that a grouping starts for the TransferAutoPointer but this doesn't end after the function but after the close of the namespace. Moving the /**@}*/ one line up solves the issue here.
I think other files might have similar problems.

The resulting file (example.tar.gz):

• itkAutoPointer.h the original file from the repository
• itkAutoPointer_new.h the file as produced by perl
• itkAutoPointer_corrected.h the perl generated file corrected by hand.

[jhlegarreta]

This is awesome @albert-github. Thanks for having taken the time to investigate this. I'll have a closer look during the coming days, and probably push a PR with the fixes following your proposal

[jhlegarreta]

A PR has been submitted: #3805, but it looks like it requires more work/some workaround to make clang happy, or else, it is the Perl script that should be changed.

@blowekamp Can the Doxygen version be updated to 1.9.5 in https://github.com/InsightSoftwareConsortium/ITKDoxygen in case that also helps? I have not found where the version is specified so as to submit a PR.

[dzenanz]

The doxygen version is provided by the build machine, blaster. I can update it to 1.9.5, and if there are issues, we can downgrade to 1.8.16.

[dzenanz]

I just updated doxygen. Let's not merge PR 3805 until at least tomorrow. That way we will see the output with just the doxygen version updated.

[jhlegarreta]

Thanks @dzenanz.

Let's not merge #3805 until at least tomorrow.

I converted the PR to a draft: as it has been highlighted in the PR, the clang-format conflicts should first be addressed so as to be able to merge the PR.

[jhlegarreta]

The dashboard is still showing such warnings and fixing them would be a tremendous benefit when going over the dashboard issues.

[albert-github]

@jhlegarreta
It would always be useful that you specify:

• the doxygen version used (the current version is 1.13.2)
• the warnings you see (when a lot just the different type ones).

[jhlegarreta]

Not sure if I follow here @albert-github. Those warnings can be openly seen in the Linux-Ubuntu-GCC-Doxygen build, blaster.kitware site in the dashboard:
https://open.cdash.org/index.php?project=Insight

[albert-github]

As non Insight user, but looking at it from the doxygen perspective how would I know where I can see the warnings etc. and furthermore these warnings are not persistent and where can I find the resulting documentation and the used Doxyfiel and version.

From the comment #3654 (comment) it looked that 2 years ago the problem was reported as coming from a perl file but this file has not been updated since a long time so most likely the problem is still at that place.

[jhlegarreta]

Still not following @albert-github:

and furthermore these warnings are not persistent and where can I find the resulting documentation and the used Doxyfiel and version.

The issues are being reported nightly and consistently since a few years now.

The Doxygen file has not changed in relevant ways for this, and I remember @blowekamp mentioning whether we should update the Doxygen version being used, but I cannot find the comment/PR/issue now.

From the comment #3654 (comment) it looked that 2 years ago the problem was reported as coming from a perl file but this file has not been updated since a long time so most likely the problem is still at that place.

I am not sure what the root cause of the warnings is: #3805 (comment) was a manual fix that seemed to fix things, but then clang would not like the fix, so the underlying issue is maybe somewhere else, or we have to make clang understand that the change should persist.

[albert-github]

The per script is only used during the documentation generation and has, as far as I can tell, nothing to do with clang.
The problem is that the perl script, as mentioned in the earlier comment, places a /**@}*/ at the wrong place, so it is even independent of the doxygen version.
As a proof of concept I manually fixed a few files.

Furthermore I don't see the reason why one would like to place the @{ and @} everywhere, but probably there is a, to me unknown, reason for this.

I think a maintainer / contributor / developer of ITK should fix the perl script. I see that there is the PR #3805 that should have been merged in December 2022 but never was. Furthermore I don't like the fix in the PR #3805 as it works around a problem in the perl script, thus it can easily resurface. The problem should be fixed ad the root cause of the problem. I think this is also already mentioned in #3805

As side note:

The issues are being reported nightly and consistently since a few years now.

what I mean that when fixed the problem cannot be seen anymore.

[blowekamp]

The Doxygen file has not changed in relevant ways for this, and I remember @blowekamp mentioning whether we should update the Doxygen version being used, but I cannot find the comment/PR/issue now.

Yes. We are currently using 1.9.4 and the latest release of Doxygen is 1.13.2. Time is likely better spent getting a newer version of Doxygen to work than maintaining the old one. With the ITKDoxygen repository being used for the daily production this is a task that any community member can take on.

Unfortunately ITKDoxygen's build is not being reported to the dashboard yet.

[blowekamp]

As side note:

The issues are being reported nightly and consistently since a few years now.

what I mean that when fixed the problem cannot be seen anymore.

I agree that the script should be fixed. I just used copilot to translate it to python, and it looks reasonable. I think the change in language will make the issue more approachable for the community.

[jhlegarreta]

I see that there is the PR #3805 that should have been merged in December 2022 but never was

Never was because the solution was not optimal even to me, unfortunately, and hence the draft status. As you say, the underlying Perl issue also surfaced in that conversation, and some comments of yours earlier in this thread also mentioned the role of the Perl script in this issue.

what I mean that when fixed the problem cannot be seen anymore.

The only solution we have is to trust a local build from the contributor.

I agree that the script should be fixed. I just used copilot to translate it to python, and it looks reasonable. I think the change in language will make the issue more approachable for the community.

Sounds reasonable, Brad.

[albert-github]

@blowekamp

1. Do you know why the perl / python script is used?
2. When the clang(-formatter) doesn't like some constructs used by doxygen, maybe it is an idea to drop them with the clang(-formatter) community or mention them here so a suggestion can be made to to overcome the problems.
3. I fully agree that the python script is possibly easier to be fixed by the community, but I also think point 1. is very important.

[blowekamp]

1. Do you know why the perl / python script is used?

From the perl file:

# if regular doxycomment add a @{
# at next empty line add a //@}

It is use to apply one doxyen string to multiple member functions. For example:

/** Set/Get the Ivar */
void SetIVar(int);
int GetIVar();

That one string will be applied to both the the added grouping commands.

[albert-github]

@blowekamp

That makes sense, but should normally be in the done straight in the code as now one is depending on some side effects (i.e. required empty lines). As you already use ALIASES it could be done by means of an ALIASES so probably clang(-formatter) won't bark either.

Regarding the ALIASES it looks to me that they are currently generated properly in the build/Utilities/Doxygen/Doxyfile.Documentation, the inside quotes (") are not escaped.
I saw also an error in Documentation/Doxygen/Modules.dox around line 142 that reads:

 \ defgroup ImageEnhancement Image Enhancement Filters

but should be

  \defgroup ImageEnhancement Image Enhancement Filters

This one I quickly saw, but there are more.
(doxygen is in the newer version a bit stricter regarding the \ingroup command).

[dzenanz]

Issue #4161 might be related.

[albert-github]

I saw that 2 problems mentioned in #3654 (comment)

• regarding ALIASES
• regarding \ defgroup ImageEnhancement Image Enhancement Filters

are already fixed, thanks. Gives some encouragement to report other problems as well (in new issues).