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

Problem building documentation with overloaded functions #98

Closed
AkibAzmain opened this issue Dec 14, 2020 · 15 comments
Closed

Problem building documentation with overloaded functions #98

AkibAzmain opened this issue Dec 14, 2020 · 15 comments
Labels

Comments

@AkibAzmain
Copy link

AkibAzmain commented Dec 14, 2020

When I use exhale to generate documentation for my library with overloaded function, it shows warnings like this:

/home/akib/Projects/docview/docs/api/function_namespacedocview_1a23dabfacba6b32d41bd966ee72449980.rst:13: WARNING: doxygenfunction: Unable to resolve multiple matches for function "docview::unload_extension" with arguments (std::string) in doxygen xml output for project "libdocview" from directory: xml.
Potential matches:
    - void unload_extension(extension::extension &extension_object)
    - void unload_extension(std::string uuid)
/home/akib/Projects/docview/docs/api/function_namespacedocview_1ab268c8d6637aa0009f5ef5822efc10fe.rst:13: WARNING: doxygenfunction: Unable to resolve multiple matches for function "docview::unload_extension" with arguments (extension::extension&) in doxygen xml output for project "libdocview" from directory: xml.
Potential matches:
    - void unload_extension(extension::extension &extension_object)
    - void unload_extension(std::string uuid)

To diagnose the problem, I disabled Exhale and opened one of the files with problems. It had this line:

.. doxygenfunction:: docview::unload_extension(extension::extension&)

I changed the line to the following but the warning still showed:

.. doxygenfunction:: docview::unload_extension(extension::extension& extension_object)

Then I changed to this and the warning disappeared:

.. doxygenfunction:: docview::unload_extension(extension::extension &extension_object)

It looks like Breathe is very sensitive to whitespace and parameter names.

Is there any workaround?

Sphinx version: 3.3.1
Breathe version: 4.24.1
Exhale version: 0.2.3

Note: Although this issue is similar to #32, it's not the same or duplicate of that issue.

@svenevs
Copy link
Owner

svenevs commented Dec 14, 2020

Oh my, the function overloads are back at it 😱 It looks like maybe the rules in breathe have changed, thank you for producing a thorough bug report!

  1. If you have time, could you try earlier versions of breathe to see if it stopped working at a certain point? If earlier versions work then you could potentially pin your breathe version if you just need it to work ™️ (please report back if you find the latest functioning version).

  2. Helpful reference, Fix function overloads (part 1 of 2) #45 is the changes that are relevant for function overloads which used to allow removing the variable name (which is why I excluded it from the parameter list). It seems like maybe that's required now. This is the code that generates docview::unload_extension(extension::extension&) currently:

    exhale/exhale/graph.py

    Lines 245 to 250 in 58c6c77

    if self.kind == "function":
    # TODO: breathe bug with templates and overloads, don't know what to do...
    return "{name}({parameters})".format(
    name=self.name,
    parameters=", ".join(self.parameters)
    )

    This is the code that parses / populates self.parameters:

    exhale/exhale/graph.py

    Lines 1999 to 2003 in 58c6c77

    # 2. The function parameter list.
    parameters = []
    for param in memberdef.find_all("param", recursive=False):
    parameters.append(param.type.text)
    func.parameters = utils.sanitize_all(parameters)

    Note that the santize_all may now be a problem if you coerce self.parameters to include the parameter name in addition to the parameter type.

  3. You can also in your conf.py introduce a funny hack to bypass this, untested but should be close enough to get you there, see sphinx api docs on hooking into core events for more info:

    def source_read(app, docname, source):
        # NOTE: not sure about docname, maybe just `print(docname)` to start out
        if docname == "api/function_namespacedocview_1a23dabfacba6b32d41bd966ee72449980.rst":
            source[0] = source[0].replace(
                ".. doxygenfunction:: docview::unload_extension(extension::extension&)",
                ".. doxygenfunction:: docview::unload_extension(extension::extension &extension_object)")
        elif docname == "api/function_namespacedocview_1ab268c8d6637aa0009f5ef5822efc10fe.rst":
            source[0] = source[0].replace(
                ".. doxygenfunction:: docview::unload_extension(std::string)",
                ".. doxygenfunction:: docview::unload_extension(std::string uuid)")
    
    def setup(app):  # called automatically for you by sphinx
        app.connect("source-read", source_read)

    But I may have mixed up the filenames, not sure, but the point is that you can ask sphinx to call your custom handler and then transform whatever documents you need to apply bandaids as needed. Not pretty, but works 🙃

Wanted to send you a response, but the function overloads were really hacky and frustrating. Long term, I want to PR to breathe to ask them if we can enable every directive to have a bypass for refid lookup. So that we can e.g., .. doxygenfunction:: __refid__::{function_refid} or .. doxygenclass:: __refid__::{class_refid}. That way we don't need to know the rules, which would fix the longstanding template function overload problem. But that may be challenging to implement.

I'm not in a good place and don't know if or when that is going to change 😞 So for now pretty much all open source is on hold for me.

@svenevs svenevs added the bug label Dec 14, 2020
@svenevs
Copy link
Owner

svenevs commented Dec 14, 2020

Seems related: breathe-doc/breathe#606

I will try and test that PR quickly tonight after work, seems like a recent regression and I think my test cases may be helpful for them on this but have not run tests in ages...

@mattip
Copy link

mattip commented Dec 14, 2020

Note I have added tests to breathe-doc/breathe#606 and fixed a minor issue with initialization values in a PR to that PR jakobandersen/breathe#1

@AkibAzmain
Copy link
Author

AkibAzmain commented Dec 14, 2020

@svenevs I have fixed the problem. This fix works in my case. I have also submitted a pull request, #99. Closing this issue.

@AkibAzmain
Copy link
Author

Oops, this repository still has this issue, reopening issue.

@AkibAzmain AkibAzmain reopened this Dec 14, 2020
@AkibAzmain
Copy link
Author

@svenevs I'm confused about this bug. Is this is from upstream or a bug of both?

Also, using Breathe from pull request breathe-doc/breathe#606 makes it work (with Exhale from pip).

@AkibAzmain
Copy link
Author

breathe-doc/breathe#606 is merged, closing issue.

@aprotyas
Copy link

I'm getting something similar still. In my case, the complaint is between is_directory(const path&) and is_directory(const path& p). 😞

@svenevs svenevs reopened this Jul 22, 2021
@svenevs
Copy link
Owner

svenevs commented Jul 22, 2021

Ok the rules probably changed again, using an earlier version of breathe may fix but I will have to look more closely. This fix is not without peril

@aprotyas
Copy link

Ok the rules probably changed again, using an earlier version of breathe may fix but I will have to look more closely. This fix is not without peril

Do you know a version of breathe I can pin to and try?

@svenevs
Copy link
Owner

svenevs commented Jul 22, 2021

No :( probably 4.29 or 4.28 looking at the release dates for breathe on PyPI and that the PR 606 linked above fixed it.

I can't test at this moment but if you find one that works please post back. Sorry that's not very helpful :/

@aprotyas
Copy link

aprotyas commented Jul 29, 2021

I've tried with a couple of versions of breathe around v4.28, unfortunately nothing seemed to work for me.

[Edit]: This problem goes away with breathe==4.26.0, FYI.

@mattip
Copy link

mattip commented Jul 29, 2021

@aprotyas Can you post a complete example, preferably minimal, that fails for you?

@aprotyas
Copy link

aprotyas commented Jul 29, 2021

@mattip I've attached as small an example as I could. To reproduce, run:

> doxygen Doxyfile
> sphinx-build . out

You should get an error that reads:

/tmp/exhale98/sample_proj/api/function_namespacefoo_1_1bar_1a4ba5a34f396d13152562b45891e82a13.rst:13: WARNING: doxygenfunction: Unable to resolve function "foo::bar::bad_function" with arguments (const int&) in doxygen xml output for project "svenevs_exhale_issue_98" from directory: doc_output/xml.
Potential matches:
 - bool bad_function(const int &p) noexcept

In hindsight, given the example I could reproduce with, this is not an "overloaded functions" problem precisely. I suspect this is a problem with breathe's hypersensitivity to white-space. Maybe the sanitize function needs to be looked at.

exhale/exhale/utils.py

Lines 252 to 254 in 58c6c77

def sanitize(name):
"""
Sanitize the specified ``name`` for use with breathe directives.

svenevs_exhale_issue_98.tar.gz

aprotyas pushed a commit to aprotyas/rosdoc2 that referenced this issue Aug 19, 2021
As discovered in svenevs/exhale#98,
older versions of `breathe` do not throw unresolved function
name warnings and/or choke on template parameter lists.

Hence, temporarily pinning to 4.26.0.

Signed-off-by: Abrar Rahman Protyasha <abrar@openrobotics.org>
@svenevs
Copy link
Owner

svenevs commented Jan 1, 2022

closing with reference to #106, right now many overloads work, the ones that still fail are non-specialized template functions and sometimes global (not in a namespace) functions. this bug does resurface differently over time, just trying to consolidate the function overloads discussion

@svenevs svenevs closed this as completed Jan 1, 2022
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

4 participants