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

Missing support for GNU extension types like _Complex or _Fract #9535

Closed
marxin opened this issue Aug 9, 2021 · 16 comments · Fixed by #9547 or #9553
Closed

Missing support for GNU extension types like _Complex or _Fract #9535

marxin opened this issue Aug 9, 2021 · 16 comments · Fixed by #9547 or #9553
Assignees
Labels
domains:c type:enhancement enhance or introduce a new feature
Milestone

Comments

@marxin
Copy link
Contributor

marxin commented Aug 9, 2021

I'm currently working on the transition of the GNU C compiler (GCC) manuals and I noticed there are unsupported C extensions like:

.. c:function:: complex long foo(int)
.. c:function:: _Complex long foo(int)
.. c:function:: long fract __satfractunssisq (unsigned int a)

  My function.

where I see the following parsing error:

/home/marxin/Programming/texi2rst-generated/sphinx/demo/demo.rst:66: WARNING: Invalid C declaration: Expected identifier, got user-defined keyword: complex. Remove it from c_extra_keywords to allow it as identifier.
Currently c_extra_keywords is ['alignas', 'alignof', 'bool', 'complex', 'imaginary', 'noreturn', 'static_assert', 'thread_local']. [error at 7]
  complex long foo(int)
  -------^
/home/marxin/Programming/texi2rst-generated/sphinx/demo/demo.rst:67: WARNING: Invalid C declaration: Expected identifier in nested name, got keyword: _Complex [error at 8]
  _Complex long foo(int)
  --------^
/home/marxin/Programming/texi2rst-generated/sphinx/demo/demo.rst:68: WARNING: Error in declarator or parameters
Invalid C declaration: Expecting "(" in parameters. [error at 11]
  long fract __satfractunssisq (unsigned int a)
  -----------^

Right now, there's some special casing for e.g. 'unsigned' type:

sphinx/sphinx/domains/c.py

Lines 2566 to 2585 in 6ac326e

elements = []
if self.skip_word_and_ws('signed'):
elements.append('signed')
elif self.skip_word_and_ws('unsigned'):
elements.append('unsigned')
while 1:
if self.skip_word_and_ws('short'):
elements.append('short')
elif self.skip_word_and_ws('long'):
elements.append('long')
else:
break
if self.skip_word_and_ws('char'):
elements.append('char')
elif self.skip_word_and_ws('int'):
elements.append('int')
elif self.skip_word_and_ws('double'):
elements.append('double')
elif self.skip_word_and_ws('__int64'):
elements.append('__int64')

One possible fix is adding the mentioned C extension handling for the following types:
https://gcc.gnu.org/onlinedocs/gcc/Fixed-Point.html
https://gcc.gnu.org/onlinedocs/gcc/Complex.html

or I can see a domain parser can become public via an API entry point:

sphinx/sphinx/domains/c.py

Lines 2128 to 2131 in 6ac326e

_simple_fundamental_types = (
'void', '_Bool', 'bool', 'char', 'int', 'float', 'double',
'__int64',
)

What do you think?

@marxin marxin added the type:enhancement enhance or introduce a new feature label Aug 9, 2021
@marxin
Copy link
Contributor Author

marxin commented Aug 9, 2021

One related problem might be something like:

.. c:function:: unsigned HOST_WIDE_INT foo ()

Where we have defined HOST_WIDE_INT with a typedef to something like unsigned long. Can one handle it with Sphinx somehow?

@marxin
Copy link
Contributor Author

marxin commented Aug 9, 2021

@jakobandersen

@jakobandersen
Copy link
Contributor

Good idea. I believe PR #9547 should contain a fully updated parsing of fundamental types and the fixed-point types.

@jakobandersen
Copy link
Contributor

One related problem might be something like:

.. c:function:: unsigned HOST_WIDE_INT foo ()

Where we have defined HOST_WIDE_INT with a typedef to something like unsigned long. Can one handle it with Sphinx somehow?

Yes. If you treat HOST_WIDE_INT as a user-configurable fundamental type (component) then it would be formatted as such, which I'm not sure is a good idea (and it's currently not supported).
Instead, you can treat it as a user-configurable attribute: in conf.py add

c_id_attributes = ["HOST_WIDE_INT"]

That should work in the latest versions.

@marxin
Copy link
Contributor Author

marxin commented Aug 15, 2021

Good idea. I believe PR #9547 should contain a fully updated parsing of fundamental types and the fixed-point types.

Thanks for working on that. One comment I have is that the type parsing should be shared among c and cpp domains.

@marxin
Copy link
Contributor Author

marxin commented Aug 15, 2021

That should work in the latest versions.

I can confirm that approach works, thanks.

@marxin
Copy link
Contributor Author

marxin commented Aug 15, 2021

And one more example I noticed is unsupported by cpp domain:

.. cpp:function:: int gimple_build (int (*valueize) (int) = NULL)

  Just a fn.

resulting in the following error:

/home/marxin/Programming/texi2rst-generated/sphinx/demo/demo.rst:66: WARNING: Error when parsing function declaration.
If the function has no return type:
  Error in declarator or parameters-and-qualifiers
  Invalid C++ declaration: Expected identifier in nested name, got keyword: int [error at 3]
    int gimple_build (int (*valueize) (int) = NULL)
    ---^
If the function has a return type:
  Error in declarator or parameters-and-qualifiers
  If pointer to member declarator:
    Invalid C++ declaration: Expected '::' in pointer to member (function). [error at 17]
      int gimple_build (int (*valueize) (int) = NULL)
      -----------------^
  If declarator-id:
    Error in declarator
    If declarator-id with parameters-and-qualifiers:
      Invalid C++ declaration: Expected identifier in nested name. [error at 23]
        int gimple_build (int (*valueize) (int) = NULL)
        -----------------------^
    If parenthesis in noptr-declarator:
      Error in declarator
      If declarator-id with parameters-and-qualifiers:
        Invalid C++ declaration: Expected "0" or "delete" or "default" in initializer-specifier. [error at 42]
          int gimple_build (int (*valueize) (int) = NULL)
          ------------------------------------------^
      If parenthesis in noptr-declarator:
        Invalid C++ declaration: Expected ')' in "( ptr-declarator )" [error at 35]
          int gimple_build (int (*valueize) (int) = NULL)
          -----------------------------------^

@jakobandersen
Copy link
Contributor

Good idea. I believe PR #9547 should contain a fully updated parsing of fundamental types and the fixed-point types.

Thanks for working on that. One comment I have is that the type parsing should be shared among c and cpp domains.

Right, I suspected that but didn't work on it yet as it requires mangling to generate stable IDs. Are the fixed-point types supported in C++ mode? I couldn't get it to compile.

@marxin
Copy link
Contributor Author

marxin commented Aug 15, 2021

Are the fixed-point types supported in C++ mode?

No, it's only C extension.

@jakobandersen
Copy link
Contributor

And one more example I noticed is unsupported by cpp domain:

.. cpp:function:: int gimple_build (int (*valueize) (int) = NULL)

Ah, good example. Should be fixed by #9553.

@marxin
Copy link
Contributor Author

marxin commented Aug 15, 2021

Ah, good example. Should be fixed by #9553.

Great, thanks for it!

@jakobandersen
Copy link
Contributor

Thanks for working on that. One comment I have is that the type parsing should be shared among c and cpp domains.

The PR should now be updated with C++ support at a similar level, though with the types I couldn't immediately confirm the mangling for removed (e.g., the _Decimal* types). Can you confirm if it handles your cases?

@marxin
Copy link
Contributor Author

marxin commented Aug 16, 2021

The PR should now be updated with C++ support at a similar level, though with the types I couldn't immediately confirm the mangling for removed (e.g., the _Decimal* types). Can you confirm if it handles your cases?

Looks good to me based on my testing. One failing example I found:

/home/marxin/Programming/texi2rst-generated/sphinx/demo/demo.rst:67: WARNING: Invalid C declaration: Expected identifier in nested name, got keyword: _Complex [error at 8]
  _Complex long foo(int a)
  --------^

@jakobandersen
Copy link
Contributor

/home/marxin/Programming/texi2rst-generated/sphinx/demo/demo.rst:67: WARNING: Invalid C declaration: Expected identifier in nested name, got keyword: _Complex [error at 8]
  _Complex long foo(int a)
  --------^

Fixed.

@marxin
Copy link
Contributor Author

marxin commented Aug 17, 2021

Fixed.

Thanks, I'm looking forward to the PR is accepted to the mainline.

@jakobandersen
Copy link
Contributor

Great, if you have no more failing cases I'l merge it within the next days.

jakobandersen added a commit to jakobandersen/sphinx that referenced this issue Aug 20, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
domains:c type:enhancement enhance or introduce a new feature
Projects
None yet
2 participants