help me fix the full template specializations?

[svenevs]

I'm working with the specialization here. For reference

unspecialized

.. doxygenclass:: arbitrary::DerivedClass
   :members:
   :protected-members:
   :undoc-members:

warnings:

WARNING: /Users/sven/Dropbox/fun/exhale-companion/docs/api/class_arbitrary__DerivedClass.rst:31: (WARNING/2) Too many template argument lists compared to parameter lists. Argument lists: 1, Parameter lists: 0, Extra empty parameters lists prepended: 1. Declaration:
	arbitrary::DerivedClass<T, N>::items

partial specialization

.. doxygenclass:: DerivedClass<arbitrary::arbitrary_struct, N>
   :members:
   :protected-members:
   :undoc-members:

warnings

WARNING: /Users/sven/Dropbox/fun/exhale-companion/docs/api/class_arbitrary__DerivedClassLT__arbitrary__arbitrary_struct_COMMA_N__GT.rst:30: (WARNING/2) Too many template argument lists compared to parameter lists. Argument lists: 1, Parameter lists: 0, Extra empty parameters lists prepended: 1. Declaration:
	arbitrary::DerivedClass<arbitrary::arbitrary_struct, N>

full specialization (?)

.. doxygenclass:: arbitrary::DerivedClass< bool, 2 >
   :members:
   :protected-members:
   :undoc-members:

warnings

WARNING: /Users/sven/Dropbox/fun/exhale-companion/docs/api/class_arbitrary__DerivedClassLT__bool_COMMA_2__GT.rst:22: (WARNING/2) Too many template argument lists compared to parameter lists. Argument lists: 1, Parameter lists: 0, Extra empty parameters lists prepended: 1. Declaration:
	arbitrary::DerivedClass<bool, 2>

where do I start?

Update ok I did a lot of digging and I think I've figured out what the problem is here. The sphinxrenderer is creating the right signatures, but this happens too late.

# breathe/sphinxrenderer method
def create_template_node(self, decl):
    """Creates a node for the ``template <...>`` part of the declaration."""
    if not decl.templateparamlist:
        return None
    template = 'template '
    nodes = [self.node_factory.desc_annotation(template, template), self.node_factory.Text('<')]
    nodes.extend(self.render(decl.templateparamlist))
    nodes.append(self.node_factory.Text(">"))
    signode = self.node_factory.desc_signature()
    signode.extend(nodes)
    return signode

This works perfectly, but the issue is this information actually needs to be a part of the sphinx/domain/cpp.py:CPPObject.

Presumably the same or similar logic can be used, but it has to happen much sooner. I did "editable" installs of sphinx and breathe and the warnings are emitted before the render signature is created (which makes sense).

In the same file in breathe/sphinxrenderer.py the class DomainDirectiveFactory has

@staticmethod
def create(domain, args):
    if domain == 'c':
        return c.CObject(*args)
    if domain == 'py':
        cls, name = DomainDirectiveFactory.python_classes.get(
            args[0], (python.PyClasslike, 'class'))
        args[1] = [DomainDirectiveFactory.fix_python_signature(n) for n in args[1]]
    else:
        cls, name = DomainDirectiveFactory.cpp_classes.get(
            args[0], (cpp.CPPMemberObject, 'member'))
    # Replace the directive name because domain directives don't know how to handle
    # Breathe's "doxygen" directives.
    args = [name] + args[1:]
    ### !!!!!!!!!!!!!!!!!!
    return cls(*args)

So is the right place to be doing this at ### !!!!!!!!!!!!!!!!!! or somewhere else entirely? E.g.

obj = cls(*args)
# do some magic to find template parameters and populate
return obj

[vermeeren]

I see that something was changed in Sphinx as referenced above. Is there still an issue with Breathe that remains?

[svenevs]

Yes. I will try and take a closer look this week or next weekend, but in order for Breathe to catch up with Sphinx we need to stop emitting an extra node for the template signature and instead populate the template related variables for the Sphinx types. My understanding of the evolution is that when this was all originally implemented Sphinx did not have template stuff in there.

[jakobandersen]

This is probably the core issue of several other. See my comment on #284.

[svenevs]

Hi @jakobandersen, thank you very much for your continued work on both supporting C++ in Sphinx, and helping Breathe catch up!

TL;DR: the main tension is that currently breathe injects a bunch of nodes when rendering things, but what we need to do is just create the nodes and populate the correct member variables and appending children in the graph (AKA it might be that we just remove our sphinx renderer altogether?)?

I just did a little more digging to test this, I made a really quick and dirty check repository: https://github.com/svenevs/toward_templates

All I did was use the Breathe code for the directive, but not incorporate anything from the sphinx renderer in breathe.

1. This class definition is just copied from Breathe.

2. I just add a simple directive in conf.py:setup(app) called template-test.

3. I invoke it once in index.rst with

   .. template-test:: template< int N > Foo
   
      Foo takes in a template argument.

This builds exactly as expected. So I think that means that for Breathe, we just need to:

1. Remove the injection of an extra node for templates on classes etc.
2. Figure out how to use tparam with these classes at creation of the node (for documentation coming from \tparam in Doxygen).
   • So we need to figure out how to populate this thing at the creation of the node, rather than during rendering of the object.
3. When specifying say member attributes or member functions, modify the signature being created.

   • This is what you are describing in the 284 comment, but my understanding is this is currently not possible at the top-level.

   • So what we currently are doing (I think) is trying to add everything to the CPPClassObject (incorrect).

   • What we need to be doing is adding children. So the equivalent of this:

     .. template-test:: template< int N > Foo
     
        Foo takes in a template argument.
     
        .. cpp:member:: static constexpr int n
     
           Convenience alias to template parameter ``N``.

Does this sound correct?

CC: @arximboldi @melvinvermeeren @eric-wieser

[jakobandersen]

Basically yes. I think there are two types of node/ast injection/manipulation going on:

• Haxes due to missing or previously missing support in Sphinx. One example is in the code you linked to. Bases can simply be appended to the signature. If they are needed later, they can be retrieved from the Sphinx structures.
  There are probably many more, and it may be good to get rid of them before refactoring for the next point.
• Haxes due to creating nested declarations. This is where a lot of problems, including the template stuff, comes from.

The Sphinx directives have before_content and after_content methods (see here), which is what the C++ domain uses to keep track of the current scope.
I suggest the following:

• Breathe subclasses basically all C++ domain objects and renders declarations with them as usual.
• All content for declarations (including nested declarations) is inserted in the overwritten before_content method (where the overwritten version is called first to set up the scope).
• We change Sphinx to make the content node it creates available for the before_content and after_content methods.
• If doc field nodes are inserted in that content node, they will automatically be handled.

However, I'm not familiar enough with the structure in Breathe to say how actually to begin refactoring into this scheme.

[svenevs]

Thank you for the response. I'm still trying to wrap my head around the last two bullet points:

• We change Sphinx to make the content node it creates available for the before_content and after_content methods.
• If doc field nodes are inserted in that content node, they will automatically be handled.

contentnode = addnodes.desc_content()
node.append(contentnode)
if self.names:
    # needed for association of version{added,changed} directives
    self.env.temp_data['object'] = self.names[0]
self.before_content()

You mean that contentnode? As in make before_content change to be passed to self.before_content as a parameter (and I guess the same for after_content)?

FWIW I think breathe has been sufficiently broken for long enough that it may be worth a more forceful approach, AKA "start from scratch" scenario and pin our requirements to sphinx>=1.8.x where 1.8.x has the before_content receiving the contentnode.

Breathe subclasses basically all C++ domain objects and renders declarations with them as usual.

Is it possible to make cpp:struct? I reached out to the sphinx dev list for counsel on how to implement it, but currently it is not possible to document C++ structs.

The only snag I see here is layout in terms of public / protected / private. But if I'm understanding this correctly, we should be able to transform the logic taking place for that into before_content adding our "Public Attribtes" etc titles without issue, and Sphinx will still be happy?

[jakobandersen]

You mean that contentnode? As in make before_content change to be passed to self.before_content as a parameter (and I guess the same for after_content)?

Yes, though the before_content and after_content methods are part of the domain API so I don't think we can change the signature. However, we can probably assign it temporarily as a member of the directive.

Is it possible to make cpp:struct? I reached out to the sphinx dev list for counsel on how to implement it, but currently it is not possible to document C++ structs.

Sure, I'll add it soon. Classes and structs are the same so the difference will probably only be which class key is displayed in front of the declaration, and which is shown in the index.

I don't think Breathe has gotten more broken, and it should be possible to incrementally refactor it without having to start over. For example, the code from Sphinxrenderer.visit_compounddef() looks like the one responsible for rendering classes with the sections you mention. In some sense, that could should "just" be moved into before_content of the relevant subclassed directives. Instead of putting nodes into nodelist, they should be put into the contentnode mentioned above.

[svenevs]

Ok I'm willing to try and give it a shot, I'll try just setting self._test_content_node and working with that. Hopefully it gets somewhere! Please feel free to ping me if / when a decision of how to make contentnode available in Sphinx is executed.

Thanks for the struct part too! I tried to implement it, but got really turned around on how directives work...I need to learn more Sphinx / docutils before I can become useful in that aspect :/

[vermeeren]

Should be fixed with #512, released in Breathe v4.17.0. Note that you also need Sphinx 3.x for recent Breathe versions.