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

Scoped rendering #512

Merged
merged 18 commits into from
May 1, 2020
Merged

Conversation

jakobandersen
Copy link
Collaborator

The Short Version

Implement the idea sketched in #354 (comment):

  • Use a new mechanism in Sphinx to render content in the scope of a directive.
  • Instead of using the definition XML, and construct a fully qualified name,
    construct the declaration string from other parts of the XML.
    The qualification is now provided by the previous point.

The Slightly Longer Version

When processing the rst

.. cpp:class:: A

   .. cpp:var:: int i

Sphinx will run the class directive with A and then in a nested parse eventually run the var directive with int i. The C/C++/Python domains then handle the scoping of i inside A.
Breathe was created before this scoping mechanism was implemented, and thus worked around it by executing a hybrid of

.. cpp:class:: A
.. cpp:var:: int A::i

with respect to the domains, but rendered as if the variable was nested in the class directive. Breathe then removes the A:: part by manipulation of the doctree.
In order to construct the fully qualified name for i the definition tag in the XML is used. However, the content of this tag is not always correct. For example, many of the problems with templates is due to this, and the function pointer reshuffling is as well.

This PR changes the rendering such that Breathe renders the content of a directive as if it was the result of a nested parse, meaning the fully qualified name is no longer needed. The declarations are therefore now stitched together using other parts of the XML.

Changes (apart from the scoped rendering)

  • Enumerators are now prefixed with "enumerator" as in Sphinx.
  • Switches from deprecated PyModulelevel directive to PyFunction.
  • Fixes issue with false typing of Python function arguments
    (e.g., __init__(self self).
  • Switches from deprecated PyClassmember directive to PyAttribute.
  • Fix rendering of initializers for Python attributes.
  • Fix display of Python modules, namespace -> module.
  • Render friend classes as textual elements instead of as a declaration.

Testing

I have only tested with Breathe it self and a few repro projects mentioned in issues.

Fixed Issues

Issues that I have tested to be fixed with this PR:

Perhaps Fixed Issues

Issues with a good chance of being fixed by this PR, but not confirmed:

TODOs for the future(?)

  • Scoped enums do not seem to be differentiated from unscoped in the Doxygen XML.
  • Templated type aliases do not seem to have a template parameter list in the Doxygen XML.
  • For classes that are partial specializations, if one of the arguments contain
    :: the name extraction will probably fail.
  • Friend classes should probably use CPPExpr to get an xref, but its
    construction requires some arguments that I'm not sure how to immediately get
    (perhaps from the context object?).
  • The PHP support has not been updated (its domain has not been updated to 3.0,
    and there are no test cases). Once done, a lot of old code can be removed.
  • Breathe injects additional an additional ID in each declaration, based on the
    one in the Doxygen XML. I'm not not where these IDs are actually used.
  • It looks like the doxygenfunction directive results in
    Duplicate declaration warnings. I believe the problem is that Breathe
    renders function declarations twice, once to figure out if the function
    is actually selected by the directive (directives.py:237), and then
    secondly to actual rendering into the output. The first round is discarded.
    Instead of the full rendering it may be possible to use the Sphinx C++ parser
    directly to essentially do the same trick.
    My best guess is that this is the issue reported in Duplicate declaration warning for overloaded functions #504.
    It looks like this PR breaks the current trick when function templates are
    involved.

@vermeeren
Copy link
Collaborator

vermeeren commented Apr 22, 2020

This looks like it might also fix #511. I will try to properly review this within a few days, need to find a timeslot to take a good look at this.

On a side note, does this need Sphinx >= 3.0 still or is scoped rendering something from 3.1.x or similar future version?

@jakobandersen
Copy link
Collaborator Author

This looks like it might also fix #511.

It will at least change the result, but if I remember correctly there is something icky with the Doxygen XML and anon unions.

On a side note, does this need Sphinx >= 3.0 still or is scoped rendering something from 3.1.x or similar future version?

It should work with >= 3.0.0.

@5p4k
Copy link

5p4k commented Apr 26, 2020

Hello. I'm encountering a bunch of errors when running Exhale + Breathe on my C++ project (planning to open an issue); I'm posting here because this PR fixes a lot of them, and maybe this can help identify duplicates or other potential issues that can be fixed.

Fixed by this PR:

Link to the log file.

Copy link
Collaborator

@vermeeren vermeeren left a comment

Choose a reason for hiding this comment

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

I am almost ashamed only posting some pedantic style inconsistency comments. Still, Everything else seems fine to me. @jakobandersen I intend on merging after the two comments are handled. Thanks again.

breathe/renderer/sphinxrenderer.py Outdated Show resolved Hide resolved
examples/specific/struct_function.h Outdated Show resolved Hide resolved
@vermeeren
Copy link
Collaborator

@LizardM4 thanks for posting other related issues, will handle those too.

@jakobandersen jakobandersen deleted the scoped-rendering branch May 1, 2020 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment