C++: breathe AssertionError during build

[reupen]

Hi,

I've been testing this out on my project and was keen to try out the recent C++ improvements.

Unfortunately, since version 0.5.0 I'm getting the following assertion failure during a build:

  File "c:\python39\lib\site-packages\breathe\renderer\sphinxrenderer.py", line 687, in handle_declaration
    assert isinstance(line, addnodes.desc_signature_line)
AssertionError

The full log is here (using version 0.7.2): https://gist.github.com/reupen/52848dcd31c6266ab5fcb3c469dd85fb

Rough repro steps (Windows):

git clone https://github.com/reupen/columns_ui-sdk
cd columns_ui-sdk
git switch test-docs-material
doxygen
cd docs
pip install -U -r requirements.txt
make.bat html

The last version with a successful build was 0.4.1.

Thanks

[jbms]

The issue here is that both sphinx-immaterial and breathe are doing their own munging of the Sphinx C++ domain output.

The munging by sphinx-immaterial happens first, and then breathe does not find the expected structure.

The actual error you are seeing is triggered by this line:
https://github.com/michaeljones/breathe/blob/d3022c1017ff44575b6cec7f017b68719d3e4480/breathe/renderer/sphinxrenderer.py#L694

Sphinx-immaterial wraps template parameters in a desc_cpp_template_params node, which breathe does not expect.

The following patch disables the use of desc_cpp_template_params and allows your documentation to build:

diff --git a/sphinx_immaterial/cpp_domain_fixes.py b/sphinx_immaterial/cpp_domain_fixes.py
index 59a34fa..7fd17d8 100644
--- a/sphinx_immaterial/cpp_domain_fixes.py
+++ b/sphinx_immaterial/cpp_domain_fixes.py
@@ -148,18 +148,19 @@ def _monkey_patch_cpp_ast_template_params():
         lineSpec: bool,
     ) -> None:
         fake_parent = sphinx.addnodes.desc_signature("", "")
-        signode = desc_cpp_template_params("", "")
-        parentNode += signode
+        #signode = desc_cpp_template_params("", "")
+        #parentNode += signode
+        signode = parentNode
         orig_describe_signature_as_introducer(
             self, signode, mode, env, symbol, lineSpec
         )
         # Ensure the requires clause is not wrapped in
         # `desc_cpp_template_params`.
-        if signode.children:
-            last_child = signode.children[-1]
-            if isinstance(last_child, desc_cpp_requires_clause):
-                del signode.children[-1]
-                parentNode += last_child
+        #if signode.children:
+        #    last_child = signode.children[-1]
+        #    if isinstance(last_child, desc_cpp_requires_clause):
+        #        del signode.children[-1]
+        #        parentNode += last_child
         for x in signode.traverse(condition=sphinx.addnodes.desc_name):
             # Ensure template parameter names aren't styled as the main entity
             # name.

However, I'm not sure how to best deal with this issue in general.

[2bndy5]

Could we make desc_cpp_template_params inherit from addnodes.desc_signature_line as well as addnodes.desc_sig_element? Or would that cause a later conflict in sphinx (like node visitor)?

[jbms]

Currently we don't actually rely on desc_cpp_template_params at all -- it is used by my C++ autosummary extension that I haven't yet created a PR for.

The specific way it is used is that the autosummary extension abbreviates template parameters as follows:

Let's say we have:

template <typename T>
class Foo {
  template <typename U>
  void bar(U u);

  template <typename X>
  class Baz;
};

On the page that documents Foo, Foo will be shown as:

template <typename T>
class Foo;

and bar and Baz will be summarized as:

void bar(U u);

class Baz<X>;

Note: Template parameters are excluded from function templates, but for class templates the template arguments are added in.

On the individual pages documenting bar and Baz, they will be shown as:

template <typename U>
void Foo<T>::bar(U u);

template <typename X>
class Foo<T>::Baz;

Note: Only template parameters for the final entity being documented are shown, any template parameters of the surrounding scope are elided, and just shown more compactly as template arguments <T>. But T is still a hyperlink to the documentation for the template parameter T of Foo., if it is documented

We can't just feed in these elided declarations to the Sphinx C++ domain, though, because then it would not properly associated the template parameters with the template arguments, and produce the wrong symbol and possibly emit warnings/errors. Instead, we feed the Sphinx C++ domain the full declaration with all template parameters, and then post-process the resulting docutils nodes to remove the template parameters that we want to exclude.. Currently that code makes use of the desc_cpp_template_params node to identify the template parameters, but I think it could be changed to work differently. (In particular, if there are multiple template parameter lists, we need to know where each one starts and ends.)

I think the simplest solution is to just remove desc_cpp_template_params, as in my suggested patch in the previous comment, and then I can find a different solution when I bring over the C++ autosummary PR.

[2bndy5]

It would be best if we support your custom C++ autosummary as well as breathe (though I can't imagine them used simultaneously).

This will have to wait till #92 is being addressed.

[jbms]

Yeah, I think it will be possible to support both, it will just be necessary to use a different mechanism to "tag" the template parameters, e.g. by setting an additional attribute on the desc_signature_line node. Note: Sphinx already indicates which desc_signature_line nodes correspond to template parameters, but if you have multiple template parameter lists, it does not make it easy to distinguish them.