Renderer crash when visiting typedef

[IvanoBilenchi]

Breathe 4.18.1 (Sphinx 3.0.3), was previously using Breathe 4.14.1 (Sphinx 2.4.3) which does not have this issue.

This header crashes the renderer:

/**
 * My integer type.
 *
 * @typedef my_uint_t
 */

/**
 * Maximum value of a my_uint_t variable.
 *
 * @def MY_UINT_MAX
 */

#if defined SOME_MACRO
    typedef uint32_t my_uint_t;
    #define MY_UINT_MAX UINT32_MAX
#else
    typedef uint64_t my_uint_t;
    #define MY_UINT_MAX UINT64_MAX
#endif

The exception thrown is:

Exception occurred:
File "/usr/local/lib/python3.7/site-packages/breathe/renderer/sphinxrenderer.py", line 1476, in visit_typedef
return self.handle_declaration(node, declaration)
UnboundLocalError: local variable 'declaration' referenced before assignment

Having a look at the code, declaration is probably uninitialized because Breathe fails to retrieve the definition for the typedef, which is nested in the #ifdef, so none of the two conditions are met:

if node.definition.startswith('typedef '):
    declaration = ' '.join([type_, name, node.get_argsstring()])
elif node.definition.startswith('using '):
    # TODO: looks like Doxygen does not generate the proper XML
    #       for the template paramter list
    declaration = self.create_template_prefix(node)

Of course I can get it to work pretty easily by changing the condition to:

if node.definition.startswith('using '):
    # TODO: looks like Doxygen does not generate the proper XML
    #       for the template paramter list
    declaration = self.create_template_prefix(node)
    declaration += ' ' + name + " = " + type_
else:
    declaration = ' '.join([type_, name, node.get_argsstring()])

Though I'm not sure if that's the right course of action in all cases, so I don't know if this would be fine as a PR. Thoughts?

Thanks!

[vermeeren]

I think the fix proposed is fine @IvanoBilenchi , thanks for investigating. Does appear the real problem is something else as is often the case, but this seems like a decent workaround. Should add a comment though marking the bug and what was the original situation so it isn't forgotten about in the future when this could be fixed properly.

@jakobandersen Could you also share thoughts if possible?

[jakobandersen]

Hmm, this is strange. What is the supposed meaning of @typedef my_uint_t?
If the C++ is

/**
 * My integer type.
 *
 * @typedef my_uint_t
 */

typedef uint64_t my_uint_t;

the XML is

        <type>uint64_t</type>
        <definition>my_uint_t</definition>
        <argsstring></argsstring>
        <name>my_uint_t</name>

but if the C++ is just

/**
 * My integer type.
 *
 */

typedef uint64_t my_uint_t;

the XML becomes

        <type>uint64_t</type>
        <definition>typedef uint64_t my_uint_t</definition>
        <argsstring></argsstring>
        <name>my_uint_t</name>

Unfortunately, if typedef uint64_t my_uint_t; is replaced with using my_uint_t = uint64_t;, the two XML snippets become

        <type>uint64_t</type>
        <definition>my_uint_t</definition>
        <argsstring></argsstring>
        <name>my_uint_t</name>

and

        <type>uint64_t</type>
        <definition>using my_uint_t =  uint64_t</definition>
        <argsstring></argsstring>
        <name>my_uint_t</name>

so the proposed fix unfortunately does not work for type aliases.
It seems like the @typedef my_uint_t makes Doxygen spit out wrong XML.

[IvanoBilenchi]

Thanks for the reply!

I did some digging in the Doxygen docs: the typedef command seems to require a "typedef declaration" parameter, so likely the whole typedef existing new or using new = existing statements. The docs are not very explicit, though if you look at the fn command, which is marked as being "equivalent" to @typedef, the whole function declaration needs to be embedded (not just the symbol name). This is a shame as it leads to more duplication than needed, as also reported in the docs for @fn, but that's how it is. I guess I can find a workaround by e.g. using @typedef typedef my_uint_t, which will likely work, but it's indeed not something either Doxygen or Breathe need to support.

I guess this is more of a "better error handling needed" situation. Maybe throw an explicit exception if either condition is not met? Something along the lines of raise ValueError('Incomplete typedef declaration.') may work way better than the current UnboundLocalError.

Or a reasonable default: given that you cannot infer if the @typedef command refers to an actual typedef or a type alias due to the incomplete declaration in Doxygen's XML, maybe a good course of action would be assuming it is an actual typedef? (e.g. similarly to the fix I proposed).

[jakobandersen]

Hmm, from the XML it looks like one can completely leave out the @typedef command. The comment is still put in the <detaileddescription> tag.
But you are right, there should be a warning about potentially iffy comments, making the rendering potentially wrong.

[vermeeren]

Would it be acceptable to emit a warning and then render it via the fallback way anyway, using declaration = ' '.join([type_, name, node.get_argsstring()]) as posted above? Effectively this would just add an else to the statement.

[jakobandersen]

Sounds like a good fallback to me. Note that this method may also be called for C declarations, so falling back to typdef as suggested is probably the best option.

[utzig]

Hmm, this is strange. What is the supposed meaning of @typedef my_uint_t?
If the C++ is

/**
 * My integer type.
 *
 * @typedef my_uint_t
 */

typedef uint64_t my_uint_t;

the XML is

        <type>uint64_t</type>
        <definition>my_uint_t</definition>
        <argsstring></argsstring>
        <name>my_uint_t</name>

It seems like the @typedef my_uint_t makes Doxygen spit out wrong XML.

It might be wrong but seems like Doxygen is expecting exactly this in its tests:

https://github.com/doxygen/doxygen/blob/master/testing/018_def.c#L31
https://github.com/doxygen/doxygen/blob/master/testing/018/018__def_8c.xml#L49

Also the few uses of "@typedef" in the Doxygen code base, are not documenting the typedef itself but its use, as can be seen here:

https://github.com/doxygen/doxygen/blob/master/qtools/qfile.cpp#L535
https://github.com/doxygen/doxygen/blob/master/qtools/qfile.h#L61

[jakobandersen]

It seems like the @typedef my_uint_t makes Doxygen spit out wrong XML.

It might be wrong but seems like Doxygen is expecting exactly this in its tests:

https://github.com/doxygen/doxygen/blob/master/testing/018_def.c#L31
https://github.com/doxygen/doxygen/blob/master/testing/018/018__def_8c.xml#L49

Hmm, from that XML it is not clear to me how to figure out of it's a typedef or a type alias. Perhaps there is a different marker for type aliases?

Also the few uses of "@typedef" in the Doxygen code base, are not documenting the typedef itself but its use, as can be seen here:

https://github.com/doxygen/doxygen/blob/master/qtools/qfile.cpp#L535
https://github.com/doxygen/doxygen/blob/master/qtools/qfile.h#L61

I think it is documenting the typedef, but just in a different position than where it is declared. The Doxygen docs for \fn seems to confirm this (https://www.doxygen.nl/manual/commands.html#cmdfn).
That is, if the documentation comment is at the declaration, don't use @typedef/\typedef. But if you would like to position the documentation somewhere else, then use these markers to lookup the full data from the declaration. If you, for example, try to permute the comments in https://github.com/doxygen/doxygen/blob/master/testing/018_def.c#L31 then the XML should get permuted similarly. For Breathe this should be irrelevant, but it seems that Doxygen is not consistent in the XML output.

[utzig]

Perhaps there is a different marker for type aliases?

My codebases are in C, so I won't be seeing much "using", but here are the results with a local test:

/**
 * Just typedef, no doxygen header
 */
typedef int inta_t;

/**
 * @typedef intb_t
 */
typedef int intb_t;

/**
 * Just using, no doxygen header
 */
using intc_t = int;

/**
 * @typedef intd_t
 */
using intd_t = int;

Resulting Doxygen XML:

<memberdef kind="typedef">
  <type>int</type>
  <definition>typedef int inta_t</definition>
  <name>inta_t</name>
</memberdef>
<memberdef kind="typedef">
  <type>int</type>
  <definition>intb_t</definition>
  <name>intb_t</name>
</memberdef>
<memberdef kind="typedef">
  <type>int</type>
  <definition>using intc_t =  int</definition>
  <name>intc_t</name>
</memberdef>
<memberdef kind="typedef">
  <type>int</type>
  <definition>intd_t</definition>
  <name>intd_t</name>
</memberdef>

[vermeeren]

Fix released in v4.19.2 just now, thanks for all the work!