sphinx-doc / sphinx

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

[C++] Top-level `const` and `volatile` should not be present for function parameters #11441

Open cjdb opened 1 year ago

cjdb commented 1 year ago

Describe the bug

void f(int), void f(int const), and void f(int volatile) are all the declarations for the same overload A C++ best-practice is to not const-qualify proper function declarations, and only function definitions. This is because one can declare f as void f(int const), and then define it as void f(int). You can verify this using this Compiler Explorer example.

Note that this doesn't apply to references. void f(int&) and void f(int const&) are different overloads, as demonstrated by this second Compiler Explorer example.

Top-level const-qualifier is redundant at best, but it's potentially misleading, since a reader may interpret this to mean that x won't change, which isn't guaranteed by a proper declaration, since the definition can change this up.

How to Reproduce

  1. Create a default Sphinx project.
  2. Add .. cpp:function:: int square(int const x, int volatile y); to index.rst.
  3. Run make html.

Environment Information

Platform:              linux; (Linux-6.3.2-arch1-1-x86_64-with-glibc2.37)
Python version:        3.11.3 (main, Apr  5 2023, 15:52:25) [GCC 12.2.1 20230201])
Python implementation: CPython
Sphinx version:        6.2.1
Docutils version:      0.18.1
Jinja2 version:        3.1.2
Pygments version:      2.15.1

Sphinx extensions

No response

Additional context

No response

picnixz commented 1 year ago

If you write .. cpp:function:: int square(int const x, int volatile y); as a documentation writer, then this means that you want to indicate the modifiers on x and y.

If you don't want to mislead users, drop the modifiers. Sphinx should not make assumptions on the implementation. In particular, it "trusts" that a documentation writer writing int const guarantees that the implementation is defined using an int const and not just an int.

For people who want to investigate more, you'll need to dig in domains/cpp.py, especially in ASTType.describe_signature responsible for rendering the int const x (this also renders the return type so you'll probably need to check whether we are "in the return type" or "in the parameters list", and treat the case of templates).

cjdb commented 1 year ago

The standard wording for this can be found at [dcl.fct]/p5.

If you write .. cpp:function:: int square(int const x, int volatile y); as a documentation writer, then this means that you want to indicate the modifiers on x and y.

If you don't want to mislead users, drop the modifiers. Sphinx should not make assumptions on the implementation. In particular, it "trusts" that a documentation writer writing int const guarantees that the implementation is defined using an int const and not just an int.

There are two different groups that this problem affects.

The first is that many programmers are unaware that int square(int const x, int volatile y) and int square(int x, int y) are the same function. Programmers who fall into this category are typically under the impression that they're communicating a required part of the API, when they're in fact communicating an implementation detail that users of said API don't need to know about. There isn't any benefit a user can receive from knowing that square's parameters are top-level cv-qualified. We actually have tooling that warns when proper declarations have top-level cv-qualifiers (here's an example on Compiler Explorer) because these qualifiers are useless at best. It's very easy for handwritten docs to fall out of sync with the source, so it could find itself to be misleading in the worst-case scenario.

The second group doesn't have control over the reStructured Text that's output: their docs are generated by tooling that can be consumed by Sphinx. Since Sphinx has a C++ parser (or part thereof), I think that it's worth addressing both here and in whatever tooling one uses to generate the reStructured Text.

For people who want to investigate more, you'll need to dig in domains/cpp.py, especially in ASTType.describe_signature responsible for rendering the int const x (this also renders the return type so you'll probably need to check whether we are "in the return type" or "in the parameters list", and treat the case of templates).

Thanks for the note about the return type. With the exception of built-in types, top-level cv-qualifiers on a return type are important (although heavily discouraged as of C++11). I'm a bit burnt out from writing docs tooling at the moment, but may take a look into this in a few weeks.

2bbac commented 1 year ago

multumesc pentru explicatie eu nu stiu nimic despre asta dar am vazut ca administratorii de la plataforma unde am facur investitie sa jucat cu banii mei si eu nuam retras nici un ban si am vrut sa vad cine a scos banii an numele meu acestia au fost o academie de la criptoesy 4 si au falsificat contul meu de retragere sau folosit de adresa mea de cripto Duminică, 4 iunie 2023 20:04:29 CEST, Christopher Di Bella @.***> a scris:

Formularea standard pentru aceasta poate fi găsită la [dcl.fct]/p5 .

Dacă scrieți .. cpp:function:: int square(int const x, int volatile y);ca redactor de documentație, atunci aceasta înseamnă că doriți să indicați modificatorii pe x și y .

Dacă nu doriți să induceți utilizatorii în eroare, renunțați la modificatori. Sfinxul nu ar trebui să facă presupuneri cu privire la implementare. În special, „are încredere” că un redactor de documentație int constgarantează că implementarea este definită folosind un int constși nu doar un int.

Există două grupuri diferite pe care le afectează această problemă.

Primul este că mulți programatori nu știu asta int square(int const x, int volatile y)și int square(int x, int y)au aceeași funcție. Programatorii care se încadrează în această categorie au de obicei impresia că comunică o parte necesară a API-ului, când de fapt comunică un detaliu de implementare despre care utilizatorii API-ului respectiv nu trebuie să știe. Nu există niciun beneficiu pe care un utilizator îl poate primi dacă știe că squareparametrii lui sunt calificați cv de nivel superior . De fapt, avem instrumente care avertizează atunci când declarațiile adecvate au calificări cv de nivel superior (iată un exemplu în Compiler Explorer) deoarece aceste calificative sunt inutile în cel mai bun caz. Este foarte ușor ca documentele scrise de mână să nu se sincronizeze cu sursa, așa că s-ar putea dovedi a induce în eroare în cel mai rău caz.

Al doilea grup nu are control asupra textului reStructurat care este rezultat: documentele lor sunt generate de instrumente care pot fi consumate de Sphinx. Deoarece Sphinx are un parser C++ (sau o parte a acestuia), cred că merită abordat atât aici, cât și în orice instrument folosit pentru a genera textul restructurat.

Pentru persoanele care doresc să investigheze mai mult, va trebui să cercetați domains/cpp.py, în special în ASTType.describe_signatureresponsabilitatea redării int const x(aceasta redă și tipul de returnare, așa că probabil va trebui să verificați dacă suntem „în tipul de returnare” sau „în lista de parametri”, și tratați cazul șabloanelor).

Mulțumesc pentru nota despre tipul de returnare. Cu excepția tipurilor încorporate, calificatorii CV de nivel superior pentru un tip de returnare sunt importanți (deși foarte descurajați începând cu C++11). Sunt puțin epuizat de scrierea instrumentelor de documente în acest moment, dar s-ar putea să mă uit la asta în câteva săptămâni.

— Răspundeți direct la acest e-mail, vizualizați-l pe GitHub sau dezabonați-vă . Primești acest lucru pentru că ești abonat la acest thread.ID mesaj: <sphinx-doc/sphinx/issues/11441/1575658891 @ github . com>

picnixz commented 1 year ago

Programmers who fall into this category are typically under the impression that they're communicating a required part of the API, when they're in fact communicating an implementation detail that users of said API don't need to know about

IMHO, this is not what Sphinx is meant to be. We do not check (and should not) whether the code is good or not. This is a linter's job but a tool for generating documentation should not be concerned about whether the documentation being produced is good or not, whether it makes sense or not, or whether it reflects the implementation (unless it is generated automatically but this is only available for Python).

There isn't any benefit a user can receive from knowing that square's parameters are top-level cv-qualified.

If I want to stress that the implementation is using int const even if it is redundant or it is bad practice, Sphinx should not complain at all. Because that's what I want. At best, a warning should be emitted but this should be done by the compiler itself or a linter. Note that we do not have an autodocumentation process for C/C++, so we really should make no assumption on the signature in the documentation.

when they're in fact communicating an implementation detail that users of said API don't need to know about.

And what if that is what they want ? the documentation can also be generated for private functions (otherwise, we wouldn't let users be able to generate documentation for private members at all since it's more of an implementation detail).

The second group doesn't have control over the reStructured Text that's output: their docs are generated by tooling that can be consumed by Sphinx.

In this case, the tooling is responsible for passing to Sphinx the correct inputs.