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

Incorrect rendering of functions returning decltype(auto) #330

Closed
arximboldi opened this issue Apr 30, 2017 · 6 comments
Closed

Incorrect rendering of functions returning decltype(auto) #330

arximboldi opened this issue Apr 30, 2017 · 6 comments
Assignees
Labels
upstream Issue in external software

Comments

@arximboldi
Copy link
Contributor

When functions return decltype(auto), Breathe seems to get confused and render the method incorrectly. Here is an example screenshot:

screenshot from 2017-04-30 21-37-50

And here is the generated Doxygen, showing one of the correct functions and one of the correct ones. It seems that Doxygen is not the problem here, but Breathe is misinterpreting the result.

<!-- THIS ONE IS CORRECTLY INTERPRETED BY BRETHE -->
      <memberdef kind="function" id="classimmer_1_1flex__vector_1a44a3fb50a1ffe5415c9a5f32f3c1f4cb" prot="public" static="no" const="yes" explicit="no" inline="yes" refqual="lvalue" virt="non-virtual">
        <type><ref refid="classimmer_1_1flex__vector" kindref="compound">flex_vector</ref></type>
        <definition>flex_vector immer::flex_vector&lt; T, MemoryPolicy, B, BL &gt;::push_back</definition>
        <argsstring>(value_type value) const &amp;</argsstring>
        <name>push_back</name>
        <param>
          <type>value_type</type>
          <declname>value</declname>
        </param>
        <briefdescription>
        </briefdescription>
        <detaileddescription>
<para>Returns a <ref refid="classimmer_1_1flex__vector" kindref="compound">flex_vector</ref> with <computeroutput>value</computeroutput> inserted at the end. It may allocate memory and its complexity is <emphasis>effectively</emphasis> <formula id="1">$ O(1) $</formula>. </para>        </detaileddescription>
        <inbodydescription>
        </inbodydescription>
        <location file="/home/raskolnikov/dev/immer/immer/flex_vector.hpp" line="199" column="1" bodyfile="/home/raskolnikov/dev/immer/immer/flex_vector.hpp" bodystart="199" bodyend="200"/>
      </memberdef>
<!-- THIS ONE IS **NOT** CORRECTLY INTERPRETED BY BRETHE -->
      <memberdef kind="function" id="classimmer_1_1flex__vector_1a6b5b378187750c6c4a5bbfe44d15e5e2" prot="public" static="no" const="no" explicit="no" inline="yes" refqual="rvalue" virt="non-virtual">
        <type>decltype(auto)</type>
        <definition>decltype(auto) immer::flex_vector&lt; T, MemoryPolicy, B, BL &gt;::push_back</definition>
        <argsstring>(value_type value) &amp;&amp;</argsstring>
        <name>push_back</name>
        <param>
          <type>value_type</type>
          <declname>value</declname>
        </param>
        <briefdescription>
        </briefdescription>
        <detaileddescription>
        </detaileddescription>
        <inbodydescription>
        </inbodydescription>
        <location file="/home/raskolnikov/dev/immer/immer/flex_vector.hpp" line="202" column="1" bodyfile="/home/raskolnikov/dev/immer/immer/flex_vector.hpp" bodystart="202" bodyend="203"/>
      </memberdef>
@arximboldi
Copy link
Contributor Author

Just realized of another thing that is incorrect in that example: the const& overload is rendered as just const. To be more specific, this is the signature of those two overloads:

    flex_vector push_back(value_type value) const&;
    decltype(auto) push_back(value_type value) &&;

The first one takes an l-value cons reference const&, while the second one takes r-value reference (&&) on the this argument . This is not the most common C++ but it is valid C++11 and quite a useful pattern. We see that breathe renders the first overload mostly right (but missing the & after the signature) but the second one is completely messed up (most probably because of the decltype(auto), but not entirely sure).

@jakobandersen
Copy link
Collaborator

Can you post the terminal output from running Sphinx? I think the missing ref qualifiers may be a missing conversion in Breathe, but I'm very sure decltype(auto) is a Sphinx problem (the parser doesn't recognize it yet).

@arximboldi
Copy link
Contributor Author

Yeah, last night I studied a bit the Breathe code and I think I found where the const/&& problem is and should be easy to fix. The decltype(auto) problem as you suggest looks like it is a problem in the Sphinx C++ domain (I guess that's harder to fix, damn :p)

Here is the relevant part of the output:

WARNING: /home/raskolnikov/dev/immer/doc/containers.rst:185: (WARNING/2) Error when parsing function declaration.
If the function has no return type:
  Error in declarator or parameters and qualifiers
  Invalid definition: Expected identifier in nested name, got keyword: decltype [error at 8]
    decltype(auto) immer::vector::push_back(value_type value)
    --------^
If the function has a return type:
  Invalid definition: "decltype(.)" in trailing_type_spec not implemented [error at 8]
    decltype(auto) immer::vector::push_back(value_type value)
    --------^

@jakobandersen
Copy link
Collaborator

The master branch of Sphinx should now handle decltype(auto) (but not general decltype yet).

@arximboldi
Copy link
Contributor Author

A few notes:

  1. I tried Sphinx master and it correctly handles decltype(auto).
  2. However, the methods in the screenshot are not all correctly documented due to a doxygen bug. In particular, they should be in the friends section but they are not: https://bugzilla.gnome.org/show_bug.cgi?id=782262
  3. I have made a PR for the specific problem of the const& && cualifications... Show reference qualification for methods #332 😄

@vermeeren
Copy link
Collaborator

In relation to the comment above:

    1. is fixed upstream
    1. is a bug in doxygen, also upstream
    1. has just been merged

In an attempt to keep the issue tracker clean I will be closing this as it is fixed from Breathe's perspective. Please comment if you disagree.

@vermeeren vermeeren self-assigned this May 16, 2018
@vermeeren vermeeren added the upstream Issue in external software label May 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
upstream Issue in external software
Projects
None yet
Development

No branches or pull requests

3 participants