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

References to multiply nested class is broken with INLINE_SIMPLE_STRUCTS=YES #8588

Closed
dhebbeker opened this issue Jun 7, 2021 · 12 comments
Closed
Labels

Comments

@dhebbeker
Copy link
Contributor

Describe the bug

Generated references (in HTML) to a documented, simple class FooFlags which is nested in a simple struct, nested in a non-simple class is faulty in case Doxygen is configured with

 INLINE_SIMPLE_STRUCTS=YES

The generated HTML link points to

struct_outer.html#struct_outer_1_1_foo_1_1_foo_flags

The documentation for FooFlags is properly generated, but on a different page.

The faulty references can be found at the

  • file reference (there is only one file in the example, see below)
  • outer class reference

"Simple" refers to the definition in the configuration documentation:

structs, classes, and unions with only public data fields or simple typedef fields

Expected behavior

The generated link in the HTML documentation should point to the documentation of the class FooFlags which is in the file documentation. This would be the correct link:

structs_8hpp.html#struct_outer_1_1_foo_1_1_foo_flags

To Reproduce

A self contained example that allows to reproduce the problem: doxy-nested-test.zip

Version

Using Doxygen version 1.9.1 and Windows 10 64bit.

@albert-github
Copy link
Collaborator

It is a bit unclear to me where the problem lies, would be good to mention the html files and what is present and what is expected or some images where you indicate the current, wrong, behavior.

I've run the problem with the 1.9.1 version and with the current master (1.9.2 (ca24685))

  • does the master show the right behavior?

Output 1.9.1 and master: output.tar.gz

@albert-github albert-github added C/C++ needinfo reported bug is incomplete, please add additional info labels Jun 7, 2021
@dhebbeker
Copy link
Contributor Author

It is a bit unclear to me where the problem lies, would be good to mention the html files and what is present and what is expected or some images where you indicate the current, wrong, behavior.

In all generated HTML files of the documentation apply the following replacement:

- href="structs_8hpp.html#struct_outer_1_1_foo_1_1_foo_flags"
+ href="struct_outer.html#struct_outer_1_1_foo_1_1_foo_flags"

These are the affected generated documentation files for version 1.9.1:

  • annotated.html
  • classes.html
  • struct_outer.html
  • structs_8hpp.html
  • structs_8hpp_source.html

The result of the replacement is the correct output.


I had a look at the output.tar.gz you provided.
There is an additional error in the documentation of union Outer::Foo (in struct_outer.html#union_outer_1_1_foo) with the "master" version of the output:
grafik
The type of the class member flags is Outer::Foo::FooFlags and not Foo.

The output of version 1.9.1 is correct:
grafik

@albert-github
Copy link
Collaborator

Regression: Regarding the problem as mentioned for the master version, I bisected it and this gave:

f8f4979682293b8f15c8293ac968402fe0c48a4e is the first bad commit
commit f8f4979682293b8f15c8293ac968402fe0c48a4e
Date:   Wed Feb 17 21:21:23 2021 +0100

    Refactoring: replace QRegExp by std::regex in memberdef.cpp

 src/memberdef.cpp | 149 ++++++++++++++++++++++--------------------------------
 1 file changed, 60 insertions(+), 89 deletions(-)

@albert-github
Copy link
Collaborator

I had a bit a deeper look into the regression of memberdef.cpp and it looks like the function simplifyTypeForTable is the function causing the problem. It looks like he regular expression handler cannot handle expressions like struct Outer::Foo::FooFlags:

  • multiple levels of "class" nesting
  • qualifier struct in front of it

In this respect the comments

// strip scope and field name from the type
// example: "struct N<K::J>::S.v.c" will become "struct v"

and

// take the identifier after the last ::

are quite illustrative of the requirements

I only looked at the non-templated regular expression part, replacing the part:

  static const reg::Ex re1(R"(\a\w*::(\a\w*))");       // non-template version
  static const reg::Ex re2(R"(\a\w*<[^>]*>::(\a\w*))"); // template version
  reg::Match match;
  std::string t = ts.str();
  if (reg::search(t,match,re1) || reg::search(t,match,re2))
  {
    ts = match[1].str(); // take the identifier after the last ::
  }

by

  static const reg::Ex re1(R"([^\s\t]*::)");       // non-template version
  static const reg::Ex re1a(R"(.*[\s\t])");       // non-template version
  static const reg::Ex re2(R"(\a\w*<[^>]*>::(\a\w*))"); // template version
  reg::Match match;
  reg::Match match1;
  std::string t = ts.str();
  if (reg::search(t,match,re1) || reg::search(t,match,re2))
  {
    QCString ts1;
    if (reg::search(t,match1,re1a))
    {
      ts1 = ts.left(match1.length());
    }
    ts1 += ts.right(match.position()).left(match.position()+match.length());
    ts = ts1;
  }

results, in my opinion, in the correct result but it is a bit a brute force pattern and I didn't consider the templated part.
For the templated regular expression something similar might have to be done, but I think here it is possible that there is whitespace in the template part (<..>) and I don't know what the problems with nested templates can be.

@dhebbeker
Copy link
Contributor Author

@albert-github, is still more information on the reported bug needed? I am asking as the label is still attached to this issue.

@albert-github albert-github removed the needinfo reported bug is incomplete, please add additional info label Jun 11, 2021
@albert-github
Copy link
Collaborator

No more information needed, I think I nailed the part of the code reasonably well with your example though I'm not sure about the fix.
I removed the label.

doxygen added a commit that referenced this issue Jun 12, 2021
@doxygen
Copy link
Owner

doxygen commented Jun 12, 2021

Please verify that the referenced commit fixes the problem.

@albert-github
Copy link
Collaborator

As far as I can see this does solve the problem.

@albert-github albert-github added the fixed but not released Bug is fixed in github, but still needs to make its way to an official release label Jun 13, 2021
@dhebbeker
Copy link
Contributor Author

I tested it with the automatic build for ecde208. That version includes 7536e3a. I used this workaround as I have no build environment at hand at the moment. That version identifies itself as 1.9.2 (1b05b97a97dd2745b19e49a47e07cfb7a022eaa5*).

I tested using the original example from above.

The original problem is not fixed. The references to Outer::Foo::FooFlags are still erroneous. @albert-github I suggest to remove the label "fixed but not released". ❌

The second problem (related to the regular expression) although seem to be fixed. ✔️

@albert-github
Copy link
Collaborator

@dhebbeker do you mean the fact that the links point not at the right place? as for this I submitted a new issue #8596 as in fact there were 2 problems:

so technically this issue is not 100% solved, but has been split in 2 parts.

@dhebbeker
Copy link
Contributor Author

OK, thanks! I did not notice the separate issue before. It makes sense to track the problems in different Github issues.

As the link part has been extracted, this (the remaining) issue, is resolved. I will leave the "closing" operation to one of you owner/collaborator as I do not know your workflow in detail.

albert-github added a commit to albert-github/doxygen that referenced this issue Jun 19, 2021
In case `INLINE_SIMPLE_STRUCTS` has been set the resulting opening / closing  section tags don't match.
This problem can be be seen with the doxygen tests 36, 67 and 68 and well as with the test case used for issue doxygen#8588
Instead of explicit using special flags now a general close is done (in the cases examined no problems were found with this strategy).
@doxygen
Copy link
Owner

doxygen commented Aug 18, 2021

This issue was previously marked 'fixed but not released',
which means it should be fixed in doxygen version 1.9.2.
Please verify if this is indeed the case. Reopen the
issue if you think it is not fixed and please include any additional information
that you think can be relevant (preferably in the form of a self-contained example).

@doxygen doxygen removed the fixed but not released Bug is fixed in github, but still needs to make its way to an official release label Aug 18, 2021
@doxygen doxygen closed this as completed Aug 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants