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

C++20 requires expressions not supported #7296

Open
mpusz opened this issue Mar 11, 2020 · 15 comments · May be fixed by #9377
Open

C++20 requires expressions not supported #7296

mpusz opened this issue Mar 11, 2020 · 15 comments · May be fixed by #9377
Labels
domains:cpp type:enhancement enhance or introduce a new feature

Comments

@mpusz
Copy link

mpusz commented Mar 11, 2020

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
Copy link
Contributor

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
Copy link
Author

mpusz commented May 17, 2020

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

@jakobandersen
Copy link
Contributor

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
Copy link
Author

mpusz commented May 17, 2020

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
Copy link
Contributor

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
Copy link
Author

mpusz commented May 17, 2020

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

jakobandersen added a commit to jakobandersen/sphinx that referenced this issue Apr 2, 2021
Implements the simplest case of sphinx-doc#7296,
but without proper ID generation.
@jbms
Copy link
Contributor

jbms commented Apr 21, 2021

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
Copy link
Contributor

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 added a commit to jakobandersen/sphinx that referenced this issue Jun 5, 2021
Implements the simplest case of sphinx-doc#7296,
but without proper ID generation.
jakobandersen added a commit to jakobandersen/sphinx that referenced this issue Jun 6, 2021
See sphinx-doc#7296.
Still without proper ID generation, and without sub-scopes.
jakobandersen added a commit to jakobandersen/sphinx that referenced this issue Jun 6, 2021
See sphinx-doc#7296.
Still without proper ID generation, and without sub-scopes.
@jakobandersen jakobandersen linked a pull request Jun 24, 2021 that will close this issue
@jakobandersen
Copy link
Contributor

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).

jakobandersen added a commit to jakobandersen/sphinx that referenced this issue Jun 24, 2021
Implements the simplest case of sphinx-doc#7296,
but without proper ID generation.
jakobandersen added a commit to jakobandersen/sphinx that referenced this issue Jun 24, 2021
See sphinx-doc#7296.
Still without proper ID generation, and without sub-scopes.
@mpusz
Copy link
Author

mpusz commented Jun 25, 2021

@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
Copy link

nicola-gigante commented Jun 27, 2022

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

@jbms
Copy link
Contributor

jbms commented Jun 27, 2022

They are supported with some limitations. Many limitations are fixed in #10286 which still needs to be reviewed/merged.

@nicola-gigante
Copy link

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

@jakobandersen
Copy link
Contributor

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
Copy link

Oh, I see, thanks for the clarification!

@AA-Turner AA-Turner added this to the some future version milestone Sep 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domains:cpp type:enhancement enhance or introduce a new feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants