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 with overloaded functions #32

Closed
avilchess opened this issue Apr 30, 2018 · 17 comments · Fixed by #45
Closed

Problem with overloaded functions #32

avilchess opened this issue Apr 30, 2018 · 17 comments · Fixed by #45

Comments

@avilchess
Copy link

Hi there,

I'm just using Exhale to document a small c++ library, the tool works pretty well except for overloaded functions. When it tries to parse them, it does not seem to be able to choose the right definition, as it produces the following warning message.

WARNING: doxygenfunction: Unable to resolve multiple matches for function "tsa::dimensionality::PAA" with arguments () in doxygen xml output for project "TSA" from directory: /grijander/repositories/fistro-lib/build/doc/doxygen/xml. Potential matches: - array<Point> fistro::PAA(array<Point>, int) - std::vector<Point> fistro::PAA(std::vector<Point>, int)

Actually, it includes both function definitions in the list of available functions, but If you click on any of them, I get a yellow/orange warning box with the same text as above.

Do you know any work-around to overcome this problem.
Thanks in advance, you have done a great work.
Antonio Vilches.

@svenevs
Copy link
Owner

svenevs commented Apr 30, 2018

duplicate of #13.

No worries about the duplicate, I'm doing that here so I know which ones to close when its fixed ;)

TBH I haven't actually looked very closely at the problem. I wouldn't be surprised if there is already a solution. If you're willing to hunt a little, what you can do to test

  1. Generate like normal with exhale_args and stuff in conf.py. Run make html once.

  2. Now that all of the rst docs are generated by exhale, comment out the line 'exhale' from extensions list in conf.py

    extensions = [
        'breathe',
        # 'exhale'
    ]

Just dont make clean or change your API at all for the time being. Since you did (1), all of the doxygen output and generated API from exhale are still hanging around, and you can actually edit the files directly.

I think the error above is actually saying the fix. In one of them, it should be

.. doxygenfunction:: array<Point> fistro::PAA(array<Point>, int)

and in the other

.. doxygenfunction:: std::vector<Point> fistro::PAA(std::vector<Point>, int)

(whereas currently, Exhale is naively doing .. doxygenfunction:: fistro::PAA).

If you can help identify what are valid signatures for Breathe I'd be very grateful and should be able to get a fix in here pretty quickly. I remember I gave it a brief stab and wasn't able to get anything working manually though, and then kind of forgot about it x0

@svenevs
Copy link
Owner

svenevs commented Apr 30, 2018

P.S. if I remember correctly Breathe is exceptionally sensitive to whitespace, e.g. there has to be a space between the , and the next parameter. Or at least I faced something similar with templates.

@avilchess
Copy link
Author

avilchess commented Apr 30, 2018

After diving a little bit among the generated .rst files. I have observed the following:

I have one file that gathers the information for the dimensionality namespace, this file contains two references to the function PAA, however they do not seem to make any differences, as they are just pointing to the same ref.

.. _namespace_fistro__dimensionality:

Namespace fistro::dimensionality
=============================


.. contents:: Contents
   :local:
   :backlinks: none





Functions
---------


- :ref:`function_fistro__dimensionality__PAA`

- :ref:`function_fistro__dimensionality__PAA`

- :ref:`function_fistro__dimensionality__PIP`


Typedefs
--------


- :ref:`typedef_fistro__dimensionality__Point`

Additionally, I also have an unique .rst file that defines the PAA function (In some how I would expect two flavours of this file).

.. _function_fistro__dimensionality__PAA:

Function PAA
============

- Defined in :ref:`file__grijander_repositories_fistro_include_fistro_dimensionality.h`


Function Documentation
----------------------


.. doxygenfunction:: tsa::dimensionality::PAA

I have tried to add the whole signature to this second file, where the function is being referenced, however I have got the following error message:
WARNING: doxygenfunction: Cannot find function "std::vector<Point> tsa::dimensionality::PAA" in doxygen xml output for project "FISTRO"
Hope this gives you more light on this issue.

@svenevs
Copy link
Owner

svenevs commented Apr 30, 2018

It does, thank you. Try editing the other file. I'm overwriting the first function with the second one. Which one is arbitrary (order based on doxygen's index.xml).

I'll need to parse function signatures...

@svenevs
Copy link
Owner

svenevs commented May 1, 2018

Wait actually I'm confused now. I think this is two separate issues, one is the file defining the function is getting overwritten. But what namespace is this function in?

a) fistro::dimensionality::PAA, or
b) tsa::dimensionality::PAA?

@avilchess
Copy link
Author

Sorry, copy paste error, both are fistro namespace (editing the previous message)

@mquinson
Copy link

I'm hit with the same issue. One of my classes have two methods of the same name:
https://framagit.org/simgrid/simgrid/blob/master/include/simgrid/s4u/Actor.hpp#L259

  void kill();   
  static void kill(aid_t pid); 

There is no mention of these functions in my build logs: https://framagit.org/simgrid/simgrid/-/jobs/204729

If I dump my objects.inv (using intersphinx.debug(['', './build/html/objects.inv'])), I see the following elements:

...
cpp:function
        ...
        simgrid::s4u::Actor::kill                api/classsimgrid_1_1s4u_1_1Actor.html#_CPPv3N7simgrid3s4u5Actor4killEv
        ...
cpp:functionParam
        ...
        simgrid::s4u::Actor::kill::pid           api/classsimgrid_1_1s4u_1_1Actor.html#_CPPv3N7simgrid3s4u5Actor4killE5aid_t
...

As one could expect, a reference to :cpp:func:simgrid::s4u::Actor::kill points to the first function without a parameter, but I fail to generate a link to the static void kill(aid_t pid); function. I was trying to generate a link to the parameter, but I fail to do so.

@svenevs
Copy link
Owner

svenevs commented Aug 19, 2018

I am working on this today, and hope to solve this once and for all!

@svenevs
Copy link
Owner

svenevs commented Aug 20, 2018

@mquinson sorry I misunderstood your problem. For your situation you need to specify the parameter types, I think for your case you should be able to do

:cpp:function:`simgrid::s4u::Actor::kill(aid_t)`

Or at least that's my understanding. Some gotchas are sensitivity to whitespace like int & breaks (no space, must be int&), but pointers are the opposite float * rather than float*). However these constraints may be relaxed for the Sphinx cross-refs, these are the rules I'm learning for breathe with .. doxygenfunction::. FWIW I don't believe you can link directly to the functionParam, so if the code above doesn't work this may be a different bug altogether. But maybe try replacing cpp: function above with cpp:functionParam? That doesn't seem to be an official Sphinx cross-ref though...

Please report back so that if not successful we can open an issue upstream. This issue is actually about exhale failing on free functions (not scoped to any classes or structs), but I'm having trouble deciphering the exact rules breathe needs because they don't mirror what doxygen actually produces :(

Note this issue does affect your framework, e.g. simgrid::s4u::this_actor::sleep_for. Unfortunately I think I've discovered an upstream bug and will probably only be able to solve this issue partially at this time. Template functions being the main hurdle.

@mquinson
Copy link

Unfortunately, it does not seem to work:

:cpp:function:`simgrid::s4u::Actor::kill(aid_t)`

leads to

WARNING: Unknown interpreted text role "cpp:function".

while

:cpp:func:`simgrid::s4u::Actor::kill(aid_t)` 

leads to

WARNING: Unparseable C++ cross-reference: 'simgrid::s4u::Actor::kill(aid_t)'
Invalid definition: Expected end of definition. [error at 25]
  simgrid::s4u::Actor::kill(aid_t)
  -------------------------^

Maybe it is because the () are added automatically. I seem to remember that I added a configuration option to do so somewhere, but I fail to find it again, sorry. Here is my config:
https://framagit.org/simgrid/simgrid/blob/master/docs/source/conf.py

@mquinson
Copy link

I found the configuration item back, but it's not helping. If I add add_function_parentheses = False to my config.py, the () are not added by default, but :cpp:func:simgrid::s4u::Actor::kill(aid_t) still produces the exact same error message.

@svenevs
Copy link
Owner

svenevs commented Aug 21, 2018

Ok I think I got to the bottom of this @mquinson 😄

  1. The newer syntax is introduced as of 1.8 (pre-release, pip install -U --pre sphinx). Before you upgrade, please read on to the end. I need your help forming this bug report.

  2. Apply the following diff, which fixes three things:

    • What you gave for "doxygenStripFromPath" didn't work for me (see File pages and long declarations). But this depends on where you run sphinx-build from, so you may not want that. I was running from the source dir, you were probably one directory above (in which case apply the patch and just undo that).
    • Trailing whitespace auto-trimmed by my editor for gitlab_hostand some others :)
    • Working references on index.rst because it's easy to un-do later to the Breathe generated targets in your project, and the Sphinx demo code from here because it's a really great test.
diff --git a/docs/source/conf.py b/docs/source/conf.py
index 70f5238..1ead91c 100644
--- a/docs/source/conf.py
+++ b/docs/source/conf.py
@@ -58,7 +58,7 @@ exhale_args = {
     "containmentFolder":     "./api",
     "rootFileName":          "library_root.rst",
     "rootFileTitle":         "SimGrid Full API",
-    "doxygenStripFromPath":  "..",
+    "doxygenStripFromPath":  "../..",
     # Suggested optional arguments
     "createTreeView":        True,
     # TIP: if using the sphinx-bootstrap-theme, you need
@@ -83,7 +83,7 @@ exhale_args = {
         XBT_ATTRIB_DEPRECATED_v322(m)= \
         XBT_ATTRIB_DEPRECATED_v323(m)= \
         XBT_ATTRIB_DEPRECATED_v324(m)=
-    
+
     """
 }
 
@@ -154,7 +154,7 @@ htmlhelp_basename = 'SimGrid-doc'
 
 html_context = {
     "display_gitlab": True, # Integrate Gitlab
-    "gitlab_host": "framagit.org", 
+    "gitlab_host": "framagit.org",
     "gitlab_user": "simgrid",
     "gitlab_repo": "simgrid",
     "gitlab_version": "master", # Version
diff --git a/docs/source/index.rst b/docs/source/index.rst
index 93d9c1a..5ffa70c 100644
--- a/docs/source/index.rst
+++ b/docs/source/index.rst
@@ -9,7 +9,7 @@ Welcome to SimGrid's documentation!
 
 	Simulating Algorithms <tuto_s4u.rst>
 	Simulating MPI Apps <usecase_mpi.rst>
-   
+
 .. toctree::
    :maxdepth: 1
    :caption: Getting Started:
@@ -25,6 +25,44 @@ Welcome to SimGrid's documentation!
 
 	     API <api/library_root.rst>
 
+Link Tests
+==========
+
+The Desired Links (Second Sometimes Breaks)
+-------------------------------------------
+
+- See :cpp:func:`simgrid::s4u::Actor::kill` for the no parameter version.
+- See :cpp:func:`void simgrid::s4u::Actor::kill(aid_t)` for the parameter version.
+
+Using Sphinx C++ Domain
+-----------------------
+
+When a (member) function is referenced using just its name, the reference
+will point to an arbitrary matching overload.
+The :rst:role:`cpp:any` and :rst:role:`cpp:func` roles will an alternative
+format, which simply is a complete function declaration.
+This will resolve to the exact matching overload.
+As example, consider the following class declaration:
+
+.. cpp:namespace-push:: overload_example
+.. cpp:class:: C
+
+   .. cpp:function:: void f(double d) const
+   .. cpp:function:: void f(double d)
+   .. cpp:function:: void f(int i)
+   .. cpp:function:: void f()
+
+References using the :rst:role:`cpp:func` role:
+
+- Arbitrary overload: ``C::f``, :cpp:func:`C::f`
+- Also arbitrary overload: ``C::f()``, :cpp:func:`C::f()`
+- Specific overload: ``void C::f()``, :cpp:func:`void C::f()`
+- Specific overload: ``void C::f(int)``, :cpp:func:`void C::f(int)`
+- Specific overload: ``void C::f(double)``, :cpp:func:`void C::f(double)`
+- Specific overload: ``void C::f(double) const``, :cpp:func:`void C::f(double) const`
+
+.. cpp:namespace-pop::
+
 Indices and tables
 ==================
 

So basically, assuming you're on say Sphinx v1.7.7 (run sphinx-build --version), what will happen is only the C::f "arbitrary" references in the demo code work, none of the void C::f() will work. This is what is introduced in 1.8 (apparently). So

  1. Apply the patch
  2. Run with Sphinx 1.7. It should be broken / many missing links on index.html, even from the Sphinx example code.
  3. Now pip install -U --pre sphinx and you should get 1.8.0b1. If you build again, the links should work!

However, this is where I need your help. I'm experiencing this really weird behavior where every other build will fail. So if I sphinx-build -b html -d ../build/doctrees . ../build/html from the source dir, and it succeeds, then the next time I run that same command without changing anything, I get

Exception occurred:
  File "/opt/spack/opt/spack/linux-fedora25-x86_64/clang-4.0.0/python-3.6.4-uc6nu4vwxopos2avdooqutdoloypy4hw/lib/python3.6/site-packages/sphinx/domains/cpp.py", line 4170, in find_declaration
    candId = symbol.declaration.get_newest_id()
AttributeError: 'NoneType' object has no attribute 'get_newest_id'

Then if i run again, success. Run again, same failure.

Do you experience anything like that or does it just reliably work?

@svenevs
Copy link
Owner

svenevs commented Aug 21, 2018

Damn. Sorry. TL;DR after the fact:

  • Sphinx v1.7 cannot link to it, but Breathe is generating the correctly mangled name for kill(aid_t).

  • Sphinx v1.8 introduces the ability to explicitly specify the overloads, specifically the function parameters have to be accompanied by the return type:

    .. -------vvvv
    :cpp:func`void simgrid::s4u::Actor::kill(aid_t)`
    .. -------------------------------------^^^^^^^

@mquinson
Copy link

Here are the version I was using so far (the one provided by Debian testing):

  • breathe-doc 4.9.1-1 Sphinx autodox support for languages with doxygen support (documentation)
  • libjs-sphinxdoc 1.7.6-1 JavaScript support for Sphinx documentation
  • python-sphinx 1.7.6-1 documentation generator for Python projects (implemented in Python 2)
  • python-sphinx-rtd-theme 0.4.0+dfsg-1 sphinx theme from readthedocs.org (Python 2)
  • python3-breathe 4.9.1-1 Sphinx autodox support for languages with doxygen support (Python 3)
  • python3-sphinx 1.7.6-1 documentation generator for Python projects (implemented in Python 3)
  • python3-sphinx-rtd-theme 0.4.0+dfsg-1 sphinx theme from readthedocs.org (Python 3)
  • sphinx-common 1.7.6-1 documentation generator for Python projects - common data
  • sphinx-intl 0.9.11-2 translation support utility for Sphinx
  • sphinx-rtd-theme-common 0.4.0+dfsg-1 sphinx theme from readthedocs.org (common files)

Exhale was installed with pip3 as it's not packaged yet. With them, the compilation of your tests produce:

/home/mquinson/Code/simgrid/docs/source/index.rst:39: WARNING: Unparseable C++ cross-reference: 'void simgrid::s4u::Actor::kill(aid_t)'
Invalid definition: Expected identifier in nested name, got keyword: void [error at 4]
  void simgrid::s4u::Actor::kill(aid_t)
  ----^
/home/mquinson/Code/simgrid/docs/source/index.rst:63: WARNING: Unparseable C++ cross-reference: 'void C::f'
Invalid definition: Expected identifier in nested name, got keyword: void [error at 4]
  void C::f
  ----^
/home/mquinson/Code/simgrid/docs/source/index.rst:64: WARNING: Unparseable C++ cross-reference: 'void C::f(int)'
Invalid definition: Expected identifier in nested name, got keyword: void [error at 4]
  void C::f(int)
  ----^
/home/mquinson/Code/simgrid/docs/source/index.rst:65: WARNING: Unparseable C++ cross-reference: 'void C::f(double)'
Invalid definition: Expected identifier in nested name, got keyword: void [error at 4]
  void C::f(double)
  ----^
/home/mquinson/Code/simgrid/docs/source/index.rst:66: WARNING: Unparseable C++ cross-reference: 'void C::f(double) const'
Invalid definition: Expected identifier in nested name, got keyword: void [error at 4]
  void C::f(double) const
  ----^

I will uninstall the debian packages, move to pip for all of them, and report what I get in another post.

@mquinson
Copy link

It took me a while to get sphinx-build --version display sphinx-build 1.8.0b1, but here I am.

I confirm what you have, where every second build in the same tree produces

checking consistency... /home/mquinson/Code/simgrid/docs/source/api/unabridged_api.rst: WARNING: document isn't included in any toctree
fait
preparing documents... fait
writing output... [100%] index                                                                                                                                               
Exception occurred:
  File "/home/mquinson/.local/lib/python3.6/site-packages/sphinx/domains/cpp.py", line 4170, in find_declaration
    candId = symbol.declaration.get_newest_id()
AttributeError: 'NoneType' object has no attribute 'get_newest_id'

Isn't that a bit weird that unabridged_api is not listed in a toctree when it fails?

@svenevs
Copy link
Owner

svenevs commented Aug 21, 2018

Most excellent, thank you for confirming @mquinson! Yes I'm really confused by this behavior, especially the unabridged API shenanigans. I'm going to see what I can do to reproduce this using a reduced example with / without exhale.

FWIW, it's a little late now, but next time you want to do a quick version test virtualenv makes it so you don't have to screw around with system packages, it remains isolated by default. Sorry it was so painful for you to test this, I should have mentioned virtualenv above. Given that, thank you again for going through the extended steps to help test this.

So for production, are you needing to link to this from an rst doc, or a docstring in the code itself? If the former, it seems not possible. But if the latter, you should be able to use \ref (the doxygen command) to skirt around this.

@mquinson
Copy link

Hello,

I know about virtualenv, but wanted to use pip --requirement instead because that's what I use to rebuild the pages in production. Now that I got my requirements.txt to work (by forcing sphinx>=1.8.0b1), the problem is solved for me in prod:

https://simgrid.frama.io/simgrid/tuto_s4u.html#lab-3-fixed-experiment-duration The link to simgrid::s4u::Actor::kill(aid_t) is working there.

Thanks for your help,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants