sphinx-doc / sphinx

The Sphinx documentation generator
https://www.sphinx-doc.org/
Other
6.42k stars 2.09k forks source link

C++20 requires expressions not supported #7296

Open mpusz opened 4 years ago

mpusz commented 4 years ago

Could you please add the support for C++ requires expressions?

I am the author of mp-units which is a Physical Units Library targeting C++23 and implemented in C++20. You can find the initial version of docs here: https://mpusz.github.io/units/index.html. That documentation is meant to help with getting user's feedback before C++ standardization so it would be great if you could help here.

jakobandersen commented 4 years ago

Similarly to #7295: I'm not sure if a mangling scheme has been decided yet in the Itanium ABI. Though, as long as the expression is not used in a context where an ID is required, this shouldn't block the feature completely.

mpusz commented 4 years ago

I am not sure what parsing of the source code has to do with name mangling for linker use?

jakobandersen commented 4 years ago

Consider two constrained functions

template<typename T>
   requires requires(T a) { something }
int f(T t);

template<typename T>
   requires requires(T b) { somethingElse }
int f(T t);

To get a unique ID (which is stable under permutation of your documentation) the two declarations must be mangled somehow. In Sphinx I opted for sort-of-following an existing mangling schemes. In this example we definitely need to mangle the requires clauses (i.e., #7295, which should work now), and they here need to mangle the requires expressions. Getting non-parametrised requires expressions to work (e.g., requires { something }) "just" needs a mangling scheme. The parametrised ones (like those above) is more complicated due to a new scope being introduced in the expression.

Another issue is the question of how to render these expressions. When writing them in inline text (e.g., cpp:expr:`requires { something }`) they should be rendered without line breaks. When part of declarations, where should we break lines, and how should they be indented? I have some ideas, but it would be nice to see examples of how people are doing it right now.

mpusz commented 4 years ago

Yeah, I agree that you need to internally mangle the name. I just did know that you try to follow compiler's mangling scheme which should not be a strong requirement here as the tool does not process produced binaries.

Regarding the ideas, you can refer to C++20 std::ranges library or C++20 algorithms in std::ranges namespace algorithms and you can find more examples in my library.

jakobandersen commented 4 years ago

It is indeed not a strong requirement, and there are definitely deviations, but making up IDs for arbitrary C++ code is no easy task :-). It would be nice with some more direct links. Also I didn't immediately find any requires expressions in standard papers you linked.

mpusz commented 4 years ago

You can find require clauses in the following concepts and ranges chapters.

jbms commented 3 years ago

I would also like to see this. One use case even for pre-C++-20 use is to post-process e.g. doxygen XML output in order to convert std::enable_if to requires expressions for the purpose of clearer documentation.

It seems to me that in lieu of mangling you could accomplish something similar to the existing mangling by just normalizing whitespace and hashing the text of the declaration (e.g. with sha2-256).

However, I don't think mangling (whether the current scheme inspired by itanium ABI, or the simpler hashing scheme) is necessarily the right approach for producing documentation identifiers, because we may want the documentation identifiers to be more stable than the ABI, since C++ libraries often maintain only source compatibility at best, and don't maintain ABI compatibility. For example you might add an additional parameter with a default value. That breaks ABI but you don't necessarily want to break documentation links.

Instead, I would propose that the user be able to specify an explicit "overload id" that is combined with the full name to produce the full name, rather than using a mangling scheme, e.g. an :overload-id: field to the cpp:function directive. In practice the overload id would likely be specified in the doc comment for the function using some special syntax, like:

/// Does something or other with Foo.
///
/// Overload:
///   foo
void func(Foo foo);

/// Does something or other with Bar
///
/// Overload:
///   bar
void func(Bar bar);

And the documentation generation pipeline would somehow extract those and make sure the overload-id is passed to cpp:function.

Then these functions could be identified by func(foo) and func(bar).

I have implemented a scheme very much like this to deal with overloaded Python functions produced by pybind11, and have found it to work well, though I have only implemented it a week or so ago so I don't have a ton of experience using it.

jakobandersen commented 3 years ago

I prefer to have the IDs somewhat readable, it has helped with debugging earlier, but hashing may indeed be a good stop-gap until a better solution can be found.

Non-parameterised requires expressions are likely to be merged soon (https://github.com/jakobandersen/sphinx/tree/cpp_requires_expr). The parameterised ones needs some new machinery in the symbol tables, so they will take a bit more time.

Regarding the IDs and overloading. Generally the IDs are for internal use, but are of course exposed in formats like HTML. Though, they are not intended to be handled other than for copy-paste. In rst you can refer to a function simply by name, e.g., :cpp:func:`f`, in which case an arbitrary overload is used as target, or you can specify an overload, e.g., :cpp:func:`f(Foo)`. I see the point of even of needing a kind of source-compatible-stable-ID, but as such an ID would be optional, there is in the end still a need for something like the current IDs. It should be relatively easy to let users add additional IDs, but I'm not sure how users would use those IDs later. Please open a separate issue if we should develop this idea.

jakobandersen commented 3 years ago

I think as much of this feature that can be implemented is now in PR #9377. It would be great if people you test it. Unfortunately, due to the missing name mangling I'm reluctant to merge it yet. @mpusz, I tried compiling Units with this, but essentially nothing got fixed because all the declarations with requires expressions are incomplete from Doxygen (1.8.20).

mpusz commented 3 years ago

@jakobandersen, thanks for your efforts! I do not know if that helps but I've just updated Doxygen dependency in my library to 1.9.1 (it was added to Conan recently).

nicola-gigante commented 2 years ago

Hi! What’s the status of the requires clauses support? Last comments are from a year ago. Are there any news?

jbms commented 2 years ago

They are supported with some limitations. Many limitations are fixed in https://github.com/sphinx-doc/sphinx/pull/10286 which still needs to be reviewed/merged.

nicola-gigante commented 2 years ago

Great! Should the documentation be updated? It currently says requires clauses are not supported at all.

jakobandersen commented 2 years ago

Great! Should the documentation be updated? It currently says requires clauses are not supported at all.

Indeed, the docs are outdated. Just to be sure there is no confusion for any readers: this issue is about requires expressions which are not supported, while requires clauses have partial support (#7295).

nicola-gigante commented 2 years ago

Oh, I see, thanks for the clarification!