Breakage with restrict qualifier

[jedbrown]

We're using Breathe on a C99 project that uses restrict qualifiers. This causes error messages emanating from Sphinx (sphinx/domains/cpp.py), but it's unclear to me whether a fix needs to go into Breathe or Sphinx.

If the function has no return type:
  Error in declarator or parameters and qualifiers
  Invalid definition: Expected identifier in nested name, got keyword: int [error at 3]
    int CeedTensorContractApply(CeedTensorContract contract, CeedInt A, CeedInt B, CeedInt C, CeedInt J, const  CeedScalar  *restrict t, CeedTransposeMode tmode, const  CeedInt add, const  Ce
edScalar  *restrict u, CeedScalar  *restrict v) 
    ---^
If the function has a return type:
  Error in declarator or parameters and qualifiers
  If pointer to member declarator:
    Invalid definition: Expected '::' in pointer to member (function). [error at 27]
      int CeedTensorContractApply(CeedTensorContract contract, CeedInt A, CeedInt B, CeedInt C, CeedInt J, const  CeedScalar  *restrict t, CeedTransposeMode tmode, const  CeedInt add, const  
CeedScalar  *restrict u, CeedScalar  *restrict v)
      ---------------------------^
  If declarator-id:
    Invalid definition: Expecting "," or ")" in parameters_and_qualifiers, got "t". [error at 130]
      int CeedTensorContractApply(CeedTensorContract contract, CeedInt A, CeedInt B, CeedInt C, CeedInt J, const  CeedScalar  *restrict t, CeedTransposeMode tmode, const  CeedInt add, const  
CeedScalar  *restrict u, CeedScalar  *restrict v)

I can get a clean build by stripping restrict out of the XML files produced by Doxygen.

I can take a crack at fixing, but will likely be more efficient if you have any tips for where it should be handled. Breathe has some code for other qualifiers, such as volatile, so I assume it'll need something for restrict too. The relevant XML from my case is below.

<argsstring>(CeedTensorContract contract, CeedInt A, CeedInt B, CeedInt C, CeedInt J, const CeedScalar *restrict t, CeedTransposeMode tmode, const CeedInt add, const CeedScalar *restrict u, CeedScalar *restrict v)</argsstring>
        <name>CeedTensorContractApply</name>
        <param>
          <type><ref refid="group__CeedBasis_1ga4f7f492496583d26b2b01447a26237c7" kindref="member">CeedTensorContract</ref></type>
          <declname>contract</declname>
        </param>
        <param>
          <type><ref refid="group__Ceed_1ga036f13d1bdb5db6668fd7f9a2d68617d" kindref="member">CeedInt</ref></type>
          <declname>A</declname>
        </param>
        <param>
          <type><ref refid="group__Ceed_1ga036f13d1bdb5db6668fd7f9a2d68617d" kindref="member">CeedInt</ref></type>
          <declname>B</declname>
        </param>
        <param>
          <type><ref refid="group__Ceed_1ga036f13d1bdb5db6668fd7f9a2d68617d" kindref="member">CeedInt</ref></type>
          <declname>C</declname>
        </param>
        <param>
          <type><ref refid="group__Ceed_1ga036f13d1bdb5db6668fd7f9a2d68617d" kindref="member">CeedInt</ref></type>
          <declname>J</declname>
        </param>
        <param>
          <type>const <ref refid="group__Ceed_1gae63e6972e3fbce597562168cd8237c4c" kindref="member">CeedScalar</ref> *restrict</type>
          <declname>t</declname>
        </param>
        <param>
          <type><ref refid="group__CeedBasis_1gad1681e7884b3bb1ac4f225ae0908740a" kindref="member">CeedTransposeMode</ref></type>
          <declname>tmode</declname>
        </param>
        <param>
          <type>const <ref refid="group__Ceed_1ga036f13d1bdb5db6668fd7f9a2d68617d" kindref="member">CeedInt</ref></type>
          <declname>add</declname>
        </param>
        <param>
          <type>const <ref refid="group__Ceed_1gae63e6972e3fbce597562168cd8237c4c" kindref="member">CeedScalar</ref> *restrict</type>
          <declname>u</declname>
        </param>
        <param>
          <type><ref refid="group__Ceed_1gae63e6972e3fbce597562168cd8237c4c" kindref="member">CeedScalar</ref> *restrict</type>
          <declname>v</declname>
        </param>

[vermeeren]

Not sure where or how to fix. I did a quick grep and did not find the function named parameters_and_qualifiers in breathe. Instead I found it in the sphinx sources.

Now it could be breathe does something funny with the signature prior to passing it so sphinx, but based upon the errors you posted I suspect sphinx's C++ domain does currently not support the restrict keyword. See sphinx/domains/cpp.py. @jakobandersen could you confirm if this is the case?

A somewhat ugly but mostly functional workaround is adding the following to your doxyfile:

PREDEFINED = restrict=

[jedbrown]

Yeah, the error is definitely coming from Sphinx, but I see explicit code in Breathe to handle other type qualifiers and I haven't dug into the source deep enough to know exactly what representation Breathe is handing off to sphinx/domains/cpp.py (thus whether this problem can be fixed entirely in Sphinx).

My code is actually C, but I don't know if Breathe can be used with sphinx/domains/c.py, which is must simpler/does much less. I'll use this workaround for now, thanks.

[vermeeren]

If I remember correctly adding breathe_domain_by_extension = {"c": "c", "h": "c", "cpp": "cpp", "hpp": "cpp"} to sphinx's conf.py should cause sphinx's C domain to be used.

The C domain isn't as mature as the C++ domain though, so you may experience problems. At the same time I must say that I've been using it without any issues for quite a long time, excluding the restrict workaround.

Edit: I would like the above to be default, but I know many projects use .h for mixed or even full C++ headers, so I expect trouble if such a change is made.

[jedbrown]

Hmm, that mostly works, but enums of the form

typedef enum {
  FOO_A = 1,
  FOO_B = 2,
  FOO_C = 4,
} FooMode;

are reduced to the integer literals 1, 2, 4 (no trace of FOO_A, etc.).

[vermeeren]

@jedbrown Sphinx 3 has a mostly rewritten C domain, which Breathe >= 4.15.0 uses (hopefully somewhat) properly. There is a significant chance the problem is (partially) fixed with this. If you have the time could you check again?

[vermeeren]

This might be fixed with #512, released in Breathe v4.17.0. Note that you also need Sphinx 3.x for recent Breathe versions. Could someone try and post results?

[jedbrown]

Thanks! This typedef rendering looks fine to me: https://libceed.readthedocs.io/en/jed-breathe-4.17/api/CeedBasis/#typedefs-and-enumerations

[vermeeren]

@jedbrown I also believe the PREDEFINED = restrict= workaround is no longer needed with current Sphinx, as I can find restrict in the keywords of sphinx/domains/c.py. Do mind giving this a quick test as well?

[jedbrown]

Renders like this: https://libceed.readthedocs.io/en/jed-breathe-4.17/api/CeedBasis/?highlight=schur#c.CeedSymmetricSchurDecomposition

[vermeeren]

@jedbrown Nice, looks like it is working properly and rendering in the idiomatic way. I do notice there is one space too many after the restrict, but this appears to be happening because of the way RTD theme handles things with padding.

The restrict has class property, which means it gets the padding and also the behind it. This seems to also happen with typedef for example, so yeah it is a theme bug.

Thanks for testing, good to know restrict works too now.