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

Internal crash for "previously declared class name" friend specifier #37

Closed
1 task done
benjaminulmer opened this issue Sep 4, 2024 · 16 comments
Closed
1 task done
Assignees
Labels
bug Something isn't working

Comments

@benjaminulmer
Copy link

Environment

Python 3.10.12
Poxy 0.18.0
Doxygen 1.12.0

Describe the bug

Making a friend declaration using a previously declared class name simple type specifier causes poxy to encounter an internal error. Changing the friend declaration to an elaborated type specifier fixes the issue.

#pragma once

namespace test {

struct ForwardDec;

struct MyType {
    friend ForwardDec;
    // friend struct ForwardDec;
    // ^^^ this line works
};

}  // namespace test

Additional information

Reproducing repo available here

--bug-report

poxy_bug_report.zip

  • I have attached the zip file generated when using the --bug-report option
@benjaminulmer benjaminulmer added the bug Something isn't working label Sep 4, 2024
@marzer
Copy link
Owner

marzer commented Sep 4, 2024

Ohhh, interesting. @mosra is this some plumbing that's missing from m.css, or a side-effect of Doxygen bugs? here's the stderr from m.css:

    DEBUG:root:Parsing configuration from /tmp/poxy/bug_report/temp/Doxyfile
    DEBUG:root:Extracting metadata from friends_8hpp.xml
    DEBUG:root:Extracting metadata from index.xml
    DEBUG:root:Extracting metadata from structtest_1_1_my_type.xml
    DEBUG:root:Extracting metadata from namespacetest.xml
    DEBUG:root:Ignoring Doxyfile.xml
    DEBUG:root:Parsing friends_8hpp.xml
    DEBUG:asyncio:Using selector: EpollSelector
    DEBUG:root:Parsing index.xml
    DEBUG:asyncio:Using selector: EpollSelector
    DEBUG:asyncio:Using selector: EpollSelector
    DEBUG:asyncio:Using selector: EpollSelector
    DEBUG:asyncio:Using selector: EpollSelector
    DEBUG:asyncio:Using selector: EpollSelector
    DEBUG:asyncio:Using selector: EpollSelector
    DEBUG:root:Parsing structtest_1_1_my_type.xml
    Traceback (most recent call last):
      File "/home/benjaminulmer/.cache/pypoetry/virtualenvs/bgcore-1rZqfR6r-py3.10/lib/python3.10/site-packages/poxy/mcss/documentation/doxygen.py", line 4059, in <module>
        run(state, templates=os.path.abspath(args.templates), wildcard=args.wildcard, index_pages=args.index_pages, search_merge_subtrees=not args.search_no_subtree_merging, search_add_lookahead_barriers=not args.search_no_lookahead_barriers, search_merge_prefixes=not args.search_no_prefix_merging)
      File "/home/benjaminulmer/.cache/pypoetry/virtualenvs/bgcore-1rZqfR6r-py3.10/lib/python3.10/site-packages/poxy/mcss/documentation/doxygen.py", line 3916, in run
        parsed = parse_xml(state, file)
      File "/home/benjaminulmer/.cache/pypoetry/virtualenvs/bgcore-1rZqfR6r-py3.10/lib/python3.10/site-packages/poxy/mcss/documentation/doxygen.py", line 3049, in parse_xml
        var = parse_var(state, memberdef)
      File "/home/benjaminulmer/.cache/pypoetry/virtualenvs/bgcore-1rZqfR6r-py3.10/lib/python3.10/site-packages/poxy/mcss/documentation/doxygen.py", line 2202, in parse_var
        assert element.tag == 'memberdef' and element.attrib['kind'] == 'variable'
    AssertionErro

(the XML can be found in the bug report zip under temp/xml)

@mosra
Copy link

mosra commented Sep 10, 2024

It's a behavior change in newer Doxygen, it used to treat those as "variables" (yes), now it treats them as "friends". The assertion expects the former. Ultimately, Doxygen fails to extract anything meaningful from those anyway so m.css ignores all friend declarations.

I'll see if I'm able make any progress on the giant pile of TODOs and workarounds required for Doxygen 1.9+, will comment back here once I have something. Until then, the workaround is either adding a struct, or excluding the friend declarations from Doxygen altogether with an ifdef, for example.

@marzer
Copy link
Owner

marzer commented Sep 10, 2024

Until then, the workaround is either adding a struct

Yeah, to be honest, I didn't even realize this C++11-style of friend declaration was possible until this bug report. I'd literally never seen it "in the wild" 😅

@mosra
Copy link

mosra commented Sep 10, 2024

I use it extensively myself, a nice side effect is that this way you don't accidentally forward-declare something that doesn't exist.

Hmm... I'm trying to reproduce using my own test suite with 1.12 now and I'm not getting the <memberdef kind="friend" like in the ZIP file, but kind="variable" like the assert expects. I wonder what's to blame, if it's some insane interaction with some random other setting or whatnot.

Briefly trying Doxygen 1.12 on a larger projects ends with a SIGABRT due to /usr/include/c++/14.2.1/string_view:256: constexpr const std::basic_string_view<_CharT, _Traits>::value_type& std::basic_string_view<_CharT, _Traits>::operator[](size_type) const [with _CharT = char; _Traits = std::char_traits<char>; const_reference = const char&; size_type = long unsigned int]: Assertion '__pos < this->_M_len' failed. This will be a fun day, it seems.

@mosra
Copy link

mosra commented Sep 10, 2024

Now I'm fixing a bug where Doxygen fails to recognize an end of an inline code block, causing the rest of the page to be wrapped in <code>, completely giving up on parsing anything after. Followed by a similar bug where Doxygen interprets Class<U> as start of an underlined text ... until the end of the page.

@marzer
Copy link
Owner

marzer commented Sep 10, 2024

Doxygen interprets Class<U> as start of an underlined text

we should have never given rocks the ability to think

@mosra
Copy link

mosra commented Sep 10, 2024

In retrospect the <U> is obvious, should always have behaved that way, not sure why it didn't until now. (And that's my personal problem with Markdown and its implicit HTML support, just too many tags and other footguns to type by accident.)

I'm now bisecting those bugs down to a concrete commit between Doxygen 1.8 and 1.12, once I have those squashed I'll tackle this one too. I'm hopeful that 1.12 might actually be the release that finally works again for all my use cases.

@mosra
Copy link

mosra commented Sep 13, 2024

Just FYI, I didn't give up, just slowly getting there. Didn't manage to reproduce this particular bug yet, but found many others. Watch mosra/m.css#215 for progress.

@mosra
Copy link

mosra commented Sep 15, 2024

I'm sorry, I just cannot reproduce this on my end.

Took the code from https://github.com/benjaminulmer/poxy-bugs, tried with (modified) Doxyfile from the ZIP file as well as a hand-crafted minimal Doxyfile that looked like this

INPUT                   = friend/friends.hpp
AUTOLINK_SUPPORT        = NO
QUIET                   = YES
GENERATE_HTML           = NO
GENERATE_LATEX          = NO
GENERATE_XML            = YES
XML_PROGRAMLISTING      = NO
CASE_SENSE_NAMES        = YES

And in both cases I get

    <sectiondef kind="public-attrib">
      <memberdef kind="variable" id="structtest_1_1MyType_1af90e90123bb9d05c344bc4915dd2ba4f" prot="public" static="no" mutable="no">
      <!--             ^^^^^^^^ *not* a friend here -->
        <type>friend</type>
        <definition>friend test::MyType::ForwardDec</definition>
        <argsstring></argsstring>

I tried with both Doxygen 1.11 and 1.12 from ArchLinux repos, as well as my own fork. Neither of them produces <memberdef kind="friend"> in the output.

I have only one idea why that could be, assuming your 1.12 is indeed the 1.12 tag, and not some random (and unfortunately broken) commit between 1.11 and 1.12 (which would report itself as 1.12 as well). Given that the XMLs in the ZIP look significantly different than the XMLs I get by running Doxygen directly, I suspect that poxy itself is doing some patching on them, which then results in this being changed? And by running Doxygen directly, without either m.css or poxy on top, I just don't get that output? @marzer is that possible?

@marzer
Copy link
Owner

marzer commented Sep 15, 2024

@mosra Yeah, it's possible. There's a number of pre-emptive fixes for various type system mismatches. Quite possible that something else I'm accounting for is breaking this use-case 😅 I'll have a look.

@mosra
Copy link

mosra commented Sep 15, 2024

Bold suggestion: could the original XMLs be backed up somewhere and then included in the bugreport zip? Possibly ran through the same pretty printer so they can be easily diffed for comparison.

With that we'd know for sure which side is to blame, whether doxygen itself, m.css or poxy :)

@marzer
Copy link
Owner

marzer commented Sep 15, 2024

Bold suggestion: could the original XMLs [...]

That's a good idea. Would be relatively easy to implement.

which side is to blame, whether doxygen itself, m.css or poxy :)

After a brief look, I think it might be my fault after all. There's some code that handles doxygen leaving extraneous keywords in types, and it may be too overzealous in the friend-case. Testing it now 😄

@mosra
Copy link

mosra commented Sep 15, 2024

There's some code that handles doxygen leaving extraneous keywords in types

I think that should be all handled by m.css by now (I think you even contributed that?), the last bug I found got fixed in mosra/m.css@53ac5a6.

And since 1.11, Doxygen does that clean up on its own, which I adapted to in mosra/m.css@fc372e6, so in case you still need that for certain cases (and it can't be fixed in m.css), you could version-limit it at least.

@marzer
Copy link
Owner

marzer commented Sep 15, 2024

I think you even contributed that?

Yep! It was based on my findings in dealing with it in poxy. There was some historical reason why I left the handling for it also in poxy, but I can't think what it was at the moment.

On an unrelated note, here's my nightmarish workaround for the <member> -> <memberdef> thing:

b952317#diff-d8062028ff90cbd259045873792a5136f9add44aa928b1bed8a301ed1b3aea2aR470-R540

😅

@mosra
Copy link

mosra commented Sep 15, 2024

Yeah I saw that. Definitely wouldn't want to maintain that on my end, sweating just from imagining the interesting ways in which it could break on the next Doxygen update.

@marzer marzer closed this as completed in a9d5b29 Sep 15, 2024
@benjaminulmer
Copy link
Author

Thanks for the quick fix @mosra and @marzer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants