Code Style: documentation at declaration or definition #92709

New Issue

Closed

opened 2021-11-01 13:17:12 +01:00 by Sybren A. Stüvel · 56 comments

Sybren A. Stüvel commented 2021-11-01 13:17:12 +01:00

Member

Copy Link

The API docs section in the C/C++ style guide dictates that interface documentation should be part of the implementation file, and*Comments in header are optional but keep brief//.

Note that in this proposal I'm not writing about APIs, because we do not have a clear definition of what an API is. For me it's more important to talk about the public interface of a module, where "module" means "combination of header file and implementation file(s)".

Advantages of the current guideline:

• Many IDEs default to jump-to-definition when ctrl+clicking on a symbol, making the documentation there readily available.
• Documentation is closer to the source code, so it's easier to spot when changes in implementation require changes in documentation.

Disadvantages of the current guideline:

• Implementation files often contain private fuctionality (static/anonymous namespace) and don't give a clear overview of the public interface of the functionality. This is aggravated by the fact that the order of code in implementation files is often dictated by constraints of the C/C++ language. For example, most static functions are declared before they are used, putting the lowest-level functions before the higher-level main code (when reading top to bottom).
• Declarations in header files are indicative of the public interface of that module, and having documentation in the implementation file means that there are two places to check.
• There is little separation between interface documentation (which functions are there, what do they do, how do they play together, what do the parameters mean, what does the returned value mean) and documentation of implementation details.
• IDEs can have a hard time finding the correct implementation of a certain function, especially when the header file and the implementation file are named differently. The declaration can always be found, though, because it's in a header file that is included in the current scope.

Proposal:

• Documentation of aspects of the function/class/method/field/constant/whatever that are relevant to the "user" (being the calling/instancing/etc code) is part of the interface, and thus should be written in the header file.
• Documentation of implementation details, and things not declared in the header file (because they're "private' to the implementation file), should be written in the implementation file.

I think that this change addresses all the disadvantages listed above. Since the order of symbols in the header file is less important for the compiler than in the implementation file, the header file can be ordered to make sense to human readers. Having the interface documentation there makes it easier to discover, and also easier to write (because it has a clearer focus on functionality/meaning rather than implementation details).

---

Update: After the below discussion (up to and including 2021-11-11), this is the proposed new guideline:

As for placement of documentation, follow these guidelines:

• Symbols (functions, constants, structs, classes, etc.) that are declared in a header file are considered part of the module's public interface, and should be documented in the header file. This makes it possible to document & organise the header file in a way that makes sense to the reader, to document groups of symbols together, and to read through the available functionality without being hindered by implementation details and internal code. This documentation should describe the public interface, but not internal implementation details that are irrelevant to calling code.
• Symbols that are internal to a file (static, anonymous namespace) should be documented at the implementation. This allows forward-declaring such functions in the implementation file, then listing the higher-level public functions, and only then have the lower-level internal/helper functions with their documentation. The documentation can be more to the point when the higher-level concepts are already known to the reader (when reading top-to-bottom throught he file).
• Implementation details that are irrelevant to the calling code should be documented at the definition/implementation of the symbol. Sometimes such information can even go inside a function, when it applies only to a part of its internals.

When there is overlap between internal and public functions, for example when two public functions actually call an internal function with some additional parameters, the internal function's documentation can refer to the public function. That way documentation doesn't have to be copied between those.

In Summary:

• Try to make it possible for developers to use a module by only reading its header file. In other words, improve black-boxing by documenting the public symbols in the header file.
• Optionally use doxygen comments for detailed docs.
• Keep comments about implementation details close to the implementation.
• Try to avoid duplication of comments between header & implementation doc-strings. From an internal symbol, just refer to the public one instead of copying its comments.
• These guidelines also apply to *_internal.h headers.
• When a symbol has two blocks of documentation (f.e. public doc in the header file, and implementation details doc in the .c file), only use formal parameter and return documentation (\param and \return) to the public doc-string. Doxygen cannot deal with having those defined twice in different files.

The [API docs section in the C/C++ style guide](https:*wiki.blender.org/wiki/Style_Guide/C_Cpp#API_Docs) dictates that interface documentation should be part of the implementation file, and*Comments in header are optional but keep brief//. Note that in this proposal I'm not writing about *APIs*, because we do not have a clear definition of what an API is. For me it's more important to talk about the *public interface* of a module, where "module" means "combination of header file and implementation file(s)". **Advantages of the current guideline:** - Many IDEs default to jump-to-definition when ctrl+clicking on a symbol, making the documentation there readily available. - Documentation is closer to the source code, so it's easier to spot when changes in implementation require changes in documentation. **Disadvantages of the current guideline:** - Implementation files often contain private fuctionality (static/anonymous namespace) and don't give a clear overview of the public interface of the functionality. This is aggravated by the fact that the order of code in implementation files is often dictated by constraints of the C/C++ language. For example, most `static` functions are declared before they are used, putting the lowest-level functions before the higher-level main code (when reading top to bottom). - Declarations in header files are indicative of the public interface of that module, and having documentation in the implementation file means that there are two places to check. - There is little separation between interface documentation (which functions are there, what do they do, how do they play together, what do the parameters mean, what does the returned value mean) and documentation of implementation details. - IDEs can have a hard time finding the correct implementation of a certain function, especially when the header file and the implementation file are named differently. The declaration can always be found, though, because it's in a header file that is included in the current scope. **Proposal:** - Documentation of aspects of the function/class/method/field/constant/whatever that are relevant to the "user" (being the calling/instancing/etc code) is part of the interface, and thus should be written in the header file. - Documentation of implementation details, and things not declared in the header file (because they're "private' to the implementation file), should be written in the implementation file. I think that this change addresses all the disadvantages listed above. Since the order of symbols in the header file is less important for the compiler than in the implementation file, the header file can be ordered to make sense to human readers. Having the interface documentation there makes it easier to discover, and also easier to write (because it has a clearer focus on functionality/meaning rather than implementation details). ---------- **Update: After the below discussion (up to and including 2021-11-11), this is the proposed new guideline:** As for placement of documentation, follow these guidelines: - **Symbols (functions, constants, structs, classes, etc.) that are declared in a header file** are considered part of the module's *public interface*, and should be documented in the header file. This makes it possible to document & organise the header file in a way that makes sense to the reader, to document groups of symbols together, and to read through the available functionality without being hindered by implementation details and internal code. This documentation should describe the public interface, but not internal implementation details that are irrelevant to calling code. - Symbols that are **internal to a file** (static, anonymous namespace) should be documented at the implementation. This allows forward-declaring such functions in the implementation file, then listing the higher-level public functions, and only then have the lower-level internal/helper functions with their documentation. The documentation can be more to the point when the higher-level concepts are already known to the reader (when reading top-to-bottom throught he file). - **Implementation details** that are irrelevant to the calling code should be documented at the definition/implementation of the symbol. Sometimes such information can even go inside a function, when it applies only to a part of its internals. When there is overlap between internal and public functions, for example when two public functions actually call an internal function with some additional parameters, the internal function's documentation can refer to the public function. That way documentation doesn't have to be copied between those. In Summary: - Try to make it possible for developers to use a module by only reading its header file. In other words, improve [black-boxing](https://en.wikipedia.org/wiki/Black_box) by documenting the public symbols in the header file. - Optionally use doxygen comments for detailed docs. - Keep comments about implementation details close to the implementation. - Try to avoid duplication of comments between header & implementation doc-strings. From an internal symbol, just refer to the public one instead of copying its comments. - These guidelines also apply to `*_internal.h` headers. - When a symbol has two blocks of documentation (f.e. public doc in the header file, and implementation details doc in the `.c` file), only use formal parameter and return documentation (`\param` and `\return`) to the public doc-string. Doxygen cannot deal with having those defined twice in different files.

Sybren A. Stüvel commented 2021-11-01 13:17:12 +01:00

Author

Member

Copy Link

Changed status from 'Needs Triage' to: 'Confirmed'

Changed status from 'Needs Triage' to: 'Confirmed'

Sybren A. Stüvel commented 2021-11-01 13:17:12 +01:00

Author

Member

Copy Link

Added subscriber: @dr.sybren

Added subscriber: @dr.sybren

Paul Larson commented 2021-11-01 17:20:03 +01:00

Copy Link

Added subscriber: @GeorgiaPacific

Added subscriber: @GeorgiaPacific

Howard Trickey commented 2021-11-01 18:47:19 +01:00

Member

Copy Link

Added subscriber: @howardt

Added subscriber: @howardt

Howard Trickey commented 2021-11-01 18:47:19 +01:00

Member

Copy Link

This proposal makes sense to me.

This proposal makes sense to me.

Campbell Barton commented 2021-11-02 04:10:13 +01:00

Owner

Copy Link

Added subscriber: @ideasman42

Added subscriber: @ideasman42

Campbell Barton commented 2021-11-02 04:10:13 +01:00

Owner

Copy Link

There are some issues I noticed with keeping code-comments in headers.

• Developers would make changes to the code and not updated the headers ... or note important details to the implementation (which should have been in the public doc-string).

• Duplicate/overlapping doc-strings. More than once I ran into situations where both header and implementation had different doc-strings (not different in public/implementation sense), I needed to use SVN annotations to figure out which comment was more up to date or how comments would be merged into a single comment.

  Note that it seems likely some of this doubling up on docs was caused by developers not using editors that support easily jumping to declarations.

There are some other advantages in having comprehensive docs in the code.

• When using a debugger to step over code the docs are immediately at hand.
• When reviewing code a functions doc-string can always be viewed when changes are made to the contents of a function, making it possible to check if some text in the doc-string needs to be updated (without having to apply the patch or manually open the header locally).

---

If everyone is committed to making a different convention work, it could be fine.

Just noting some pitfalls - given the number of issues in our current doc-strings (documentation for parameters that don't exist - to pick a simple example) it's possible for these kinds of issues to bite us again.

---

Details to Consider:

• Would doc-strings in headers be applied everywhere?

For example would (`*_intern.h*`, `_internal.h*`) headers contain doc-strings? (not public by definition).

• How exactly to structure both public/private comments?

Suggest that public interface comments in the header use `\param`. Since doxygen doesn't permit both the header and implementation to document the same parameter, the implementation can reference variables as part of comments if it's needed *(although I wouldn't expect every parameter would need some associated text for the implementation comments in most cases).*

example.hh

    /**
     * \param foo: Public detail about this parameter.
     */
    void function(void *foo);

example.cc

    /**
     * Private detail about `foo` parameter.
     */
    void function(void *foo);

• How would this change be applied? (assume module teams would apply changes), not an automated update.

There are some issues I noticed with keeping code-comments in headers. - Developers would make changes to the code and not updated the headers ... or note important details to the implementation (which should have been in the public doc-string). - Duplicate/overlapping doc-strings. More than once I ran into situations where both header and implementation had different doc-strings *(not different in public/implementation sense)*, I needed to use SVN annotations to figure out which comment was more up to date or how comments would be merged into a single comment. *Note that it seems likely some of this doubling up on docs was caused by developers not using editors that support easily jumping to declarations.* There are some other advantages in having comprehensive docs in the code. - When using a debugger to step over code the docs are immediately at hand. - When reviewing code a functions doc-string can always be viewed when changes are made to the contents of a function, making it possible to check if some text in the doc-string needs to be updated (without having to apply the patch or manually open the header locally). ---- If everyone is committed to making a different convention work, it could be fine. Just noting some pitfalls - given the number of issues in our current doc-strings *(documentation for parameters that don't exist - to pick a simple example)* it's possible for these kinds of issues to bite us again. ---- **Details to Consider:** - Would doc-strings in headers be applied everywhere? ``` For example would (`*_intern.h*`, `_internal.h*`) headers contain doc-strings? (not public by definition). ``` - How exactly to structure both public/private comments? ``` Suggest that public interface comments in the header use `\param`. Since doxygen doesn't permit both the header and implementation to document the same parameter, the implementation can reference variables as part of comments if it's needed *(although I wouldn't expect every parameter would need some associated text for the implementation comments in most cases).* ``` `example.hh` ``` /** * \param foo: Public detail about this parameter. */ void function(void *foo); ``` `example.cc` ``` /** * Private detail about `foo` parameter. */ void function(void *foo); ``` - How would this change be applied? (assume module teams would apply changes), not an automated update.

Julian Eisel commented 2021-11-03 16:05:44 +01:00

Member

Copy Link

Added subscriber: @JulianEisel

Added subscriber: @JulianEisel

Julian Eisel commented 2021-11-03 16:05:44 +01:00

Member

Copy Link

Another thing to consider is functions without declarations in headers. These should also have a design that should be explained in a function comment. But since they don't have a declaration in a header, do we just put them in the source file (unlike other function design comments?).

(This is not to argue against the guidline change, just mentioning as something to consider.)

Edit: I see this is mentioned in the proposal:

Documentation of implementation details, and things not declared in the header file (because they're "private' to the implementation file), should be written in the implementation file.

Another thing to consider is functions without declarations in headers. These should also have a design that should be explained in a function comment. But since they don't have a declaration in a header, do we just put them in the source file (unlike other function design comments?). (This is not to argue against the guidline change, just mentioning as something to consider.) *Edit: I see this is mentioned in the proposal:* > Documentation of implementation details, and things not declared in the header file (because they're "private' to the implementation file), should be written in the implementation file.

Campbell Barton commented 2021-11-04 02:50:41 +01:00

Owner

Copy Link

In #92709#1247622, @JulianEisel wrote:
Another thing to consider is functions without declarations in headers. These should also have a design that should be explained in a function comment. But since they don't have a declaration in a header, do we just put them in the source file (unlike other function design comments?).

(This is not to argue against the guidline change, just mentioning as something to consider.)

Edit: I see this is mentioned in the proposal:

Documentation of implementation details, and things not declared in the header file (because they're "private' to the implementation file), should be written in the implementation file.

This is not always as straightforward as you might expect.

In some cases a static function is called by a public function with minor differences (typically fewer arguments).

Take ui_handle_afterfunc_add_operator_ex for example, directly below it is ui_handle_afterfunc_add_operator which calls it, currently the static function has the doc-string.

If we document both ui_handle_afterfunc_add_operator_ex and ui_handle_afterfunc_add_operator, there will be significant overlap in the information in two different files, with a high likleyhood of them getting out of sync.

Other examples where near duplicate doc-strings could get split up between files include.

• snap_selected_to_location / ED_view3d_snap_selected_to_location
• view3d_camera_border / (ED_view3d_calc_camera_border / ED_view3d_calc_camera_border_size).
• validate_object_select_id / ED_view3d_select_id_validate.

While the issue of which to document (the static or the public function) already exists with the current convention, when doc-strings are in the implementation it's obvious when the public function is a wrapper for a static function and quick to jump to the static function and see it's doc-string (which is in the same file).

> In #92709#1247622, @JulianEisel wrote: > Another thing to consider is functions without declarations in headers. These should also have a design that should be explained in a function comment. But since they don't have a declaration in a header, do we just put them in the source file (unlike other function design comments?). > > (This is not to argue against the guidline change, just mentioning as something to consider.) > > *Edit: I see this is mentioned in the proposal:* >> Documentation of implementation details, and things not declared in the header file (because they're "private' to the implementation file), should be written in the implementation file. This is not always as straightforward as you might expect. In some cases a static function is called by a public function with minor differences (typically fewer arguments). Take `ui_handle_afterfunc_add_operator_ex` for example, directly below it is `ui_handle_afterfunc_add_operator` which calls it, currently the static function has the doc-string. If we document both `ui_handle_afterfunc_add_operator_ex` and `ui_handle_afterfunc_add_operator`, there will be significant overlap in the information in two different files, with a high likleyhood of them getting out of sync. Other examples where near duplicate doc-strings could get split up between files include. - `snap_selected_to_location` / `ED_view3d_snap_selected_to_location` - `view3d_camera_border` / (`ED_view3d_calc_camera_border` / `ED_view3d_calc_camera_border_size`). - `validate_object_select_id` / `ED_view3d_select_id_validate`. While the issue of which to document (the static or the public function) already exists with the current convention, when doc-strings are in the implementation it's obvious when the public function is a wrapper for a static function and quick to jump to the static function and see it's doc-string (which is in the same file).

Aaron Carlisle commented 2021-11-05 22:46:05 +01:00

Member

Copy Link

Added subscriber: @Blendify

Added subscriber: @Blendify

Sybren A. Stüvel commented 2021-11-08 13:15:32 +01:00

Author

Member

Copy Link

In #92709#1246354, @ideasman42 wrote:
There are some issues I noticed with keeping code-comments in headers.

• Developers would make changes to the code and not updated the headers ... or note important details to the implementation (which should have been in the public doc-string).

There will always be developers who don't update things. I've also seen this happen with above-the-implementation documentation, so I don't think this is a particularly strong argument.

• Duplicate/overlapping doc-strings. More than once I ran into situations where both header and implementation had different doc-strings (not different in public/implementation sense), I needed to use SVN annotations to figure out which comment was more up to date or how comments would be merged into a single comment.

This is indeed an issue, but it cannot be solved by having documentation in one place. Until we ditch C (or the language changes drastically), there will always be two places where documentation can be added.

There are some other advantages in having comprehensive docs in the code.

• When using a debugger to step over code the docs are immediately at hand.

True, although many IDEs show documentation on hovering. Also I think that the need to read the docs while stepping through code is less frequent than the regular need to understand how to use a certain module.

• When reviewing code a functions doc-string can always be viewed when changes are made to the contents of a function, making it possible to check if some text in the doc-string needs to be updated (without having to apply the patch or manually open the header locally).

This is an important point. It's not that hard to check though, because as soon as the functionality of some code changes, so should its documentation. If there is no change to that, you know something is wrong; either there is no documentation, which is bad, or it wasn't updated, which is also bad.

Just noting some pitfalls - given the number of issues in our current doc-strings (documentation for parameters that don't exist - to pick a simple example) it's possible for these kinds of issues to bite us again.

I don't quite follow you here. Given that our current guideline is to have doc-strings at the implementation, how are current problems an argument in favour of this? To me it just shows that, no matter where the documentation is located, there will always be lazy/hasty people who don't update it.

Details to Consider:

• Would doc-strings in headers be applied everywhere?
  For example would (*_intern.h*, _internal.h*) headers contain doc-strings? (not public by definition).

Yes, IMO they should be. Even with internal headers, there is the distinction between "interface to the rest of the code" and ""private to only the implementation in this file". IMO it's so much nicer to be able to read through the "this is how you use it" documentation, without having to juggle implementation details and static functions, that it's definitely nicer to have this also for internal code.

• How exactly to structure both public/private comments?

  Suggest that public interface comments in the header use \param. Since doxygen doesn't permit both the header and implementation to document the same parameter, the implementation can reference variables as part of comments if it's needed (although I wouldn't expect every parameter would need some associated text for the implementation comments in most cases).

example.hh

    /**
     * \param foo: Public detail about this parameter.
     */
    void function(void *foo);

example.cc

    /**
     * Private detail about `foo` parameter.
     */
    void function(void *foo);

• How would this change be applied? (assume module teams would apply changes), not an automated update.

I think your example is fine. Until we actually build & publish doxygen documentation, I feel that it's of a lower priority than to have actually understandable, human-readable docs. If it's really a hard choice between \param on the interface or the implementation, I'd say use them in the interface documentation.

UPDATE: I added my response to other comments here, so that I only have one comment that has everything I want to reply.

In #92709#1247927, @ideasman42 wrote:
This is not always as straightforward as you might expect.

In some cases a static function is called by a public function with minor differences (typically fewer arguments).

Take ui_handle_afterfunc_add_operator_ex for example, directly below it is ui_handle_afterfunc_add_operator which calls it, currently the static function has the doc-string.

If we document both ui_handle_afterfunc_add_operator_ex and ui_handle_afterfunc_add_operator, there will be significant overlap in the information in two different files, with a high likleyhood of them getting out of sync.

The static function doc can simply refer to the public function, IMO. If it really becomes hard to document something well, to me it's an indication that the code structure itself is wrong, and should be redesigned. We should design testable, documentable code, and not avoid high-quality documentation because the code is hard to describe.

While the issue of which to document (the static or the public function) already exists with the current convention, when doc-strings are in the implementation it's obvious when the public function is a wrapper for a static function and quick to jump to the static function and see it's doc-string (which is in the same file).

To me this is the wrong way around. What a function does, and in which situations it's expected to work, is part of its public interface documentation. The fact that it defers a big part of its code to some internal function, that's an implementation detail. If that static, internal function is at the same abstraction level as the public functions, yet cannot be documented in terms of those public functions, again I see it as a code design issue, and not as a reason to avoid documenting the public interface.

Clean Code by Robert Martin suggests that high-level functions preceed low-level functions. When reading a file top to bottom, it allows you to read the broad design first, and only later get down to the nitty-gritty of the low-level helper functions. To me this makes a lot of sense. However, this high-to-low ordering is uncommon in C, as low-level functions don't require separate declarations if they are defined before they are used. This does present us with considerable issues in terms of understandability of the code, and I think also that this discussion is also related to it. When the high-level internal functions are well documented & seen first, the lower-level functions can be described more succintly, as higher-level concepts have already been explained.

> In #92709#1246354, @ideasman42 wrote: > There are some issues I noticed with keeping code-comments in headers. > > - Developers would make changes to the code and not updated the headers ... or note important details to the implementation (which should have been in the public doc-string). There will always be developers who don't update things. I've also seen this happen with above-the-implementation documentation, so I don't think this is a particularly strong argument. > - Duplicate/overlapping doc-strings. More than once I ran into situations where both header and implementation had different doc-strings *(not different in public/implementation sense)*, I needed to use SVN annotations to figure out which comment was more up to date or how comments would be merged into a single comment. This is indeed an issue, but it cannot be solved by having documentation in one place. Until we ditch C (or the language changes drastically), there will always be two places where documentation can be added. > There are some other advantages in having comprehensive docs in the code. > > - When using a debugger to step over code the docs are immediately at hand. True, although many IDEs show documentation on hovering. Also I think that the need to read the docs while stepping through code is less frequent than the regular need to understand how to use a certain module. > - When reviewing code a functions doc-string can always be viewed when changes are made to the contents of a function, making it possible to check if some text in the doc-string needs to be updated (without having to apply the patch or manually open the header locally). This is an important point. It's not that hard to check though, because as soon as the functionality of some code changes, so should its documentation. If there is no change to that, you know something is wrong; either there is no documentation, which is bad, or it wasn't updated, which is also bad. > Just noting some pitfalls - given the number of issues in our current doc-strings *(documentation for parameters that don't exist - to pick a simple example)* it's possible for these kinds of issues to bite us again. I don't quite follow you here. Given that our current guideline is to have doc-strings at the implementation, how are current problems an argument in favour of this? To me it just shows that, no matter where the documentation is located, there will always be lazy/hasty people who don't update it. > **Details to Consider:** > > - Would doc-strings in headers be applied everywhere? > For example would (`*_intern.h*`, `_internal.h*`) headers contain doc-strings? (not public by definition). Yes, IMO they should be. Even with internal headers, there is the distinction between "interface to the rest of the code" and ""private to only the implementation in this file". IMO it's so much nicer to be able to read through the "this is how you use it" documentation, without having to juggle implementation details and static functions, that it's definitely nicer to have this also for internal code. > - How exactly to structure both public/private comments? > > Suggest that public interface comments in the header use `\param`. Since doxygen doesn't permit both the header and implementation to document the same parameter, the implementation can reference variables as part of comments if it's needed *(although I wouldn't expect every parameter would need some associated text for the implementation comments in most cases).* > > `example.hh` > ``` > /** > * \param foo: Public detail about this parameter. > */ > void function(void *foo); > ``` > > `example.cc` > ``` > /** > * Private detail about `foo` parameter. > */ > void function(void *foo); > ``` > > - How would this change be applied? (assume module teams would apply changes), not an automated update. I think your example is fine. Until we actually build & publish doxygen documentation, I feel that it's of a lower priority than to have actually understandable, human-readable docs. If it's really a hard choice between `\param` on the interface or the implementation, I'd say use them in the interface documentation. **UPDATE: I added my response to other comments here, so that I only have one comment that has everything I want to reply.** > In #92709#1247927, @ideasman42 wrote: > This is not always as straightforward as you might expect. > > In some cases a static function is called by a public function with minor differences (typically fewer arguments). > > Take `ui_handle_afterfunc_add_operator_ex` for example, directly below it is `ui_handle_afterfunc_add_operator` which calls it, currently the static function has the doc-string. > > If we document both `ui_handle_afterfunc_add_operator_ex` and `ui_handle_afterfunc_add_operator`, there will be significant overlap in the information in two different files, with a high likleyhood of them getting out of sync. The static function doc can simply refer to the public function, IMO. If it really becomes hard to document something well, to me it's an indication that the code structure itself is wrong, and should be redesigned. We should design testable, documentable code, and not avoid high-quality documentation because the code is hard to describe. > While the issue of which to document (the static or the public function) already exists with the current convention, when doc-strings are in the implementation it's obvious when the public function is a wrapper for a static function and quick to jump to the static function and see it's doc-string (which is in the same file). To me this is the wrong way around. What a function does, and in which situations it's expected to work, is part of its public interface documentation. The fact that it defers a big part of its code to some internal function, that's an implementation detail. If that static, internal function is at the same abstraction level as the public functions, yet cannot be documented in terms of those public functions, again I see it as a code design issue, and not as a reason to avoid documenting the public interface. Clean Code by Robert Martin suggests that high-level functions preceed low-level functions. When reading a file top to bottom, it allows you to read the broad design first, and only later get down to the nitty-gritty of the low-level helper functions. To me this makes a lot of sense. However, this high-to-low ordering is uncommon in C, as low-level functions don't require separate declarations if they are defined before they are used. This does present us with considerable issues in terms of understandability of the code, and I think also that this discussion is also related to it. When the high-level internal functions are well documented & seen first, the lower-level functions can be described more succintly, as higher-level concepts have already been explained.

Campbell Barton commented 2021-11-09 02:14:22 +01:00

Owner

Copy Link

In #92709#1250301, @dr.sybren wrote:

In #92709#1246354, @ideasman42 wrote:
There are some issues I noticed with keeping code-comments in headers.

• Developers would make changes to the code and not updated the headers ... or note important details to the implementation (which should have been in the public doc-string).

There will always be developers who don't update things. I've also seen this happen with above-the-implementation documentation, so I don't think this is a particularly strong argument.

Documentation may include details or descriptions of subtle behavior, if the text documenting this is stored elsewhere it's less obvious when changes to the code end up contradicting statements made in those doc-strings.

This is an observation of what happened in the past, while not exactly the same as C/C++ header comments in 2.4x the Python documentation for e.g. continually got out of sync when the docs were maintained in separate files, since moving the doc-strings directly above the functions this has been much less of a problem.

• Duplicate/overlapping doc-strings. More than once I ran into situations where both header and implementation had different doc-strings (not different in public/implementation sense), I needed to use SVN annotations to figure out which comment was more up to date or how comments would be merged into a single comment.

This is indeed an issue, but it cannot be solved by having documentation in one place. Until we ditch C (or the language changes drastically), there will always be two places where documentation can be added.

Not sure what you mean, as long as C++ code uses headers and source files there will be two places that can be documented (unless you mean where implementations are in the header).

There are some other advantages in having comprehensive docs in the code.

• When using a debugger to step over code the docs are immediately at hand.

True, although many IDEs show documentation on hovering. Also I think that the need to read the docs while stepping through code is less frequent than the regular need to understand how to use a certain module.

Good point.

• When reviewing code a functions doc-string can always be viewed when changes are made to the contents of a function, making it possible to check if some text in the doc-string needs to be updated (without having to apply the patch or manually open the header locally).

This is an important point. It's not that hard to check though, because as soon as the functionality of some code changes, so should its documentation. If there is no change to that, you know something is wrong; either there is no documentation, which is bad, or it wasn't updated, which is also bad.

True, although doc-strings can include all sorts of non-obvious details about arguments which I wouldn't necessarily be able to know without looking at the doc-string if the doc-string needs updating.

Just noting some pitfalls - given the number of issues in our current doc-strings (documentation for parameters that don't exist - to pick a simple example) it's possible for these kinds of issues to bite us again.

I don't quite follow you here. Given that our current guideline is to have doc-strings at the implementation, how are current problems an argument in favour of this? To me it just shows that, no matter where the documentation is located, there will always be lazy/hasty people who don't update it.

The point is I believe maintaining docs in separate files requires some additional discipline (in an area we have not been all that disciplined).

Details to Consider:

• Would doc-strings in headers be applied everywhere?
  For example would (*_intern.h*, _internal.h*) headers contain doc-strings? (not public by definition).

Yes, IMO they should be. Even with internal headers, there is the distinction between "interface to the rest of the code" and ""private to only the implementation in this file". IMO it's so much nicer to be able to read through the "this is how you use it" documentation, without having to juggle implementation details and static functions, that it's definitely nicer to have this also for internal code.

Seems fine, another issue: would we expect doc-strings to be above static function declarations too? (where static functions are forward declared before they are defined within a file)

• How exactly to structure both public/private comments?

  Suggest that public interface comments in the header use \param. Since doxygen doesn't permit both the header and implementation to document the same parameter, the implementation can reference variables as part of comments if it's needed (although I wouldn't expect every parameter would need some associated text for the implementation comments in most cases).

example.hh

    /**
     * \param foo: Public detail about this parameter.
     */
    void function(void *foo);

example.cc

    /**
     * Private detail about `foo` parameter.
     */
    void function(void *foo);

• How would this change be applied? (assume module teams would apply changes), not an automated update.

I think your example is fine. Until we actually build & publish doxygen documentation, I feel that it's of a lower priority than to have actually understandable, human-readable docs. If it's really a hard choice between \param on the interface or the implementation, I'd say use them in the interface documentation.

UPDATE: I added my response to other comments here, so that I only have one comment that has everything I want to reply.

In #92709#1247927, @ideasman42 wrote:
This is not always as straightforward as you might expect.

In some cases a static function is called by a public function with minor differences (typically fewer arguments).

Take ui_handle_afterfunc_add_operator_ex for example, directly below it is ui_handle_afterfunc_add_operator which calls it, currently the static function has the doc-string.

If we document both ui_handle_afterfunc_add_operator_ex and ui_handle_afterfunc_add_operator, there will be significant overlap in the information in two different files, with a high likleyhood of them getting out of sync.

The static function doc can simply refer to the public function, IMO. If it really becomes hard to document something well, to me it's an indication that the code structure itself is wrong, and should be redesigned. We should design testable, documentable code, and not avoid high-quality documentation because the code is hard to describe.

While the issue of which to document (the static or the public function) already exists with the current convention, when doc-strings are in the implementation it's obvious when the public function is a wrapper for a static function and quick to jump to the static function and see it's doc-string (which is in the same file).

To me this is the wrong way around. What a function does, and in which situations it's expected to work, is part of its public interface documentation. The fact that it defers a big part of its code to some internal function, that's an implementation detail. If that static, internal function is at the same abstraction level as the public functions, yet cannot be documented in terms of those public functions, again I see it as a code design issue, and not as a reason to avoid documenting the public interface.

Perhaps this can be made int a guide-line then (even as part of this proposal), that static functions should refer to public functions docs instead of duplicating them.

There still remains the awkward issue of the static function containing arguments not found in the public function but I don't think there is a good way around this.

Clean Code by Robert Martin suggests that high-level functions preceed low-level functions. When reading a file top to bottom, it allows you to read the broad design first, and only later get down to the nitty-gritty of the low-level helper functions. To me this makes a lot of sense. However, this high-to-low ordering is uncommon in C, as low-level functions don't require separate declarations if they are defined before they are used. This does present us with considerable issues in terms of understandability of the code, and I think also that this discussion is also related to it. When the high-level internal functions are well documented & seen first, the lower-level functions can be described more succintly, as higher-level concepts have already been explained.

Not against this but seems like it could cause a lot of code-churn if applied to the current code-base, so best make that part of a separate proposal (even if it's a guide-line for new code).

> In #92709#1250301, @dr.sybren wrote: >> In #92709#1246354, @ideasman42 wrote: >> There are some issues I noticed with keeping code-comments in headers. >> >> - Developers would make changes to the code and not updated the headers ... or note important details to the implementation (which should have been in the public doc-string). > > There will always be developers who don't update things. I've also seen this happen with above-the-implementation documentation, so I don't think this is a particularly strong argument. Documentation may include details or descriptions of subtle behavior, if the text documenting this is stored elsewhere it's less obvious when changes to the code end up contradicting statements made in those doc-strings. This is an observation of what happened in the past, while not exactly the same as C/C++ header comments in 2.4x the Python documentation for e.g. continually got out of sync when the docs were maintained in separate files, since moving the doc-strings directly above the functions this has been much less of a problem. >> - Duplicate/overlapping doc-strings. More than once I ran into situations where both header and implementation had different doc-strings *(not different in public/implementation sense)*, I needed to use SVN annotations to figure out which comment was more up to date or how comments would be merged into a single comment. > > This is indeed an issue, but it cannot be solved by having documentation in one place. Until we ditch C (or the language changes drastically), there will always be two places where documentation can be added. Not sure what you mean, as long as C++ code uses headers and source files there will be two places that can be documented (unless you mean where implementations are in the header). >> There are some other advantages in having comprehensive docs in the code. >> >> - When using a debugger to step over code the docs are immediately at hand. > > True, although many IDEs show documentation on hovering. Also I think that the need to read the docs while stepping through code is less frequent than the regular need to understand how to use a certain module. Good point. >> - When reviewing code a functions doc-string can always be viewed when changes are made to the contents of a function, making it possible to check if some text in the doc-string needs to be updated (without having to apply the patch or manually open the header locally). > > This is an important point. It's not that hard to check though, because as soon as the functionality of some code changes, so should its documentation. If there is no change to that, you know something is wrong; either there is no documentation, which is bad, or it wasn't updated, which is also bad. True, although doc-strings can include all sorts of non-obvious details about arguments which I wouldn't necessarily be able to know without looking at the doc-string if the doc-string needs updating. >> Just noting some pitfalls - given the number of issues in our current doc-strings *(documentation for parameters that don't exist - to pick a simple example)* it's possible for these kinds of issues to bite us again. > I don't quite follow you here. Given that our current guideline is to have doc-strings at the implementation, how are current problems an argument in favour of this? To me it just shows that, no matter where the documentation is located, there will always be lazy/hasty people who don't update it. The point is I believe maintaining docs in separate files requires some additional discipline (in an area we have not been all that disciplined). >> **Details to Consider:** >> >> - Would doc-strings in headers be applied everywhere? >> For example would (`*_intern.h*`, `_internal.h*`) headers contain doc-strings? (not public by definition). > > Yes, IMO they should be. Even with internal headers, there is the distinction between "interface to the rest of the code" and ""private to only the implementation in this file". IMO it's so much nicer to be able to read through the "this is how you use it" documentation, without having to juggle implementation details and static functions, that it's definitely nicer to have this also for internal code. Seems fine, another issue: would we expect doc-strings to be above static function declarations too? (where static functions are forward declared before they are defined within a file) >> - How exactly to structure both public/private comments? >> >> Suggest that public interface comments in the header use `\param`. Since doxygen doesn't permit both the header and implementation to document the same parameter, the implementation can reference variables as part of comments if it's needed *(although I wouldn't expect every parameter would need some associated text for the implementation comments in most cases).* >> >> `example.hh` >> ``` >> /** >> * \param foo: Public detail about this parameter. >> */ >> void function(void *foo); >> ``` >> >> `example.cc` >> ``` >> /** >> * Private detail about `foo` parameter. >> */ >> void function(void *foo); >> ``` >> >> - How would this change be applied? (assume module teams would apply changes), not an automated update. > > I think your example is fine. Until we actually build & publish doxygen documentation, I feel that it's of a lower priority than to have actually understandable, human-readable docs. If it's really a hard choice between `\param` on the interface or the implementation, I'd say use them in the interface documentation. > > **UPDATE: I added my response to other comments here, so that I only have one comment that has everything I want to reply.** > >> In #92709#1247927, @ideasman42 wrote: >> This is not always as straightforward as you might expect. >> >> In some cases a static function is called by a public function with minor differences (typically fewer arguments). >> >> Take `ui_handle_afterfunc_add_operator_ex` for example, directly below it is `ui_handle_afterfunc_add_operator` which calls it, currently the static function has the doc-string. >> >> If we document both `ui_handle_afterfunc_add_operator_ex` and `ui_handle_afterfunc_add_operator`, there will be significant overlap in the information in two different files, with a high likleyhood of them getting out of sync. > > The static function doc can simply refer to the public function, IMO. If it really becomes hard to document something well, to me it's an indication that the code structure itself is wrong, and should be redesigned. We should design testable, documentable code, and not avoid high-quality documentation because the code is hard to describe. > >> While the issue of which to document (the static or the public function) already exists with the current convention, when doc-strings are in the implementation it's obvious when the public function is a wrapper for a static function and quick to jump to the static function and see it's doc-string (which is in the same file). > > To me this is the wrong way around. What a function does, and in which situations it's expected to work, is part of its public interface documentation. The fact that it defers a big part of its code to some internal function, that's an implementation detail. If that static, internal function is at the same abstraction level as the public functions, yet cannot be documented in terms of those public functions, again I see it as a code design issue, and not as a reason to avoid documenting the public interface. Perhaps this can be made int a guide-line then (even as part of this proposal), that static functions should refer to public functions docs instead of duplicating them. There still remains the awkward issue of the static function containing arguments not found in the public function but I don't think there is a good way around this. > Clean Code by Robert Martin suggests that high-level functions preceed low-level functions. When reading a file top to bottom, it allows you to read the broad design first, and only later get down to the nitty-gritty of the low-level helper functions. To me this makes a lot of sense. However, this high-to-low ordering is uncommon in C, as low-level functions don't require separate declarations if they are defined before they are used. This does present us with considerable issues in terms of understandability of the code, and I think also that this discussion is also related to it. When the high-level internal functions are well documented & seen first, the lower-level functions can be described more succintly, as higher-level concepts have already been explained. Not against this but seems like it could cause a lot of code-churn if applied to the current code-base, so best make that part of a separate proposal (even if it's a guide-line for new code).

Sybren A. Stüvel commented 2021-11-09 12:43:18 +01:00

Author

Member

Copy Link

In #92709#1250688, @ideasman42 wrote:
Documentation may include details or descriptions of subtle behavior, if the text documenting this is stored elsewhere it's less obvious when changes to the code end up contradicting statements made in those doc-strings.

This is an observation of what happened in the past, while not exactly the same as C/C++ header comments in 2.4x the Python documentation for e.g. continually got out of sync when the docs were maintained in separate files, since moving the doc-strings directly above the functions this has been much less of a problem.

I agree that this is something we'll have to keep an eye on.

Not sure what you mean, as long as C++ code uses headers and source files there will be two places that can be documented (unless you mean where implementations are in the header).

I was thinking about even more futuristic C++ and having "modules" instead of header+source file separation.

True, although doc-strings can include all sorts of non-obvious details about arguments which I wouldn't necessarily be able to know without looking at the doc-string if the doc-string needs updating.

It's my hope that simply checking whether documentation is still correct becomes normal, especially when we move to a situation where documenting the public interface is normal.

The point is I believe maintaining docs in separate files requires some additional discipline (in an area we have not been all that disciplined).

Yup, important point. I still think the location of documentation should be primarily chosen for the benefit of the reader of the code, though, and not the writer. To me this is also about having the code-base be welcoming to new developers (or old developers looking into an area of Blender new to them).

Seems fine, another issue: would we expect doc-strings to be above static function declarations too? (where static functions are forward declared before they are defined within a file)

Good question. Personally I don't feel particularly strong about this, because those are typically for internal use only. Actually, documenting those at the implementation level could be beneficial; it's probably easier to understand the code when you first read through the high-level implementations, and only then get to the (documented) lower-level helper functions. The documentation for low-level helper functions will be hard to understand without the context of the high-level implementation.

The static function doc can simply refer to the public function, IMO. [...] What a function does, and in which situations it's expected to work, is part of its public interface documentation. The fact that it defers a big part of its code to some internal function, that's an implementation detail. If that static, internal function is at the same abstraction level as the public functions, yet cannot be documented in terms of those public functions, again I see it as a code design issue, and not as a reason to avoid documenting the public interface.

Perhaps this can be made int a guide-line then (even as part of this proposal), that static functions should refer to public functions docs instead of duplicating them.

Sounds good to me.

There still remains the awkward issue of the static function containing arguments not found in the public function but I don't think there is a good way around this.

These additional arguments can be documented without any conflict, right?

Clean Code by Robert Martin suggests that high-level functions preceed low-level functions. [...]

Not against this but seems like it could cause a lot of code-churn if applied to the current code-base, so best make that part of a separate proposal (even if it's a guide-line for new code).

I agree, better to keep that for a separate proposal.

> In #92709#1250688, @ideasman42 wrote: > Documentation may include details or descriptions of subtle behavior, if the text documenting this is stored elsewhere it's less obvious when changes to the code end up contradicting statements made in those doc-strings. > > This is an observation of what happened in the past, while not exactly the same as C/C++ header comments in 2.4x the Python documentation for e.g. continually got out of sync when the docs were maintained in separate files, since moving the doc-strings directly above the functions this has been much less of a problem. I agree that this is something we'll have to keep an eye on. > Not sure what you mean, as long as C++ code uses headers and source files there will be two places that can be documented (unless you mean where implementations are in the header). I was thinking about even more futuristic C++ and having "modules" instead of header+source file separation. > True, although doc-strings can include all sorts of non-obvious details about arguments which I wouldn't necessarily be able to know without looking at the doc-string if the doc-string needs updating. It's my hope that simply checking whether documentation is still correct becomes normal, especially when we move to a situation where documenting the public interface is normal. > The point is I believe maintaining docs in separate files requires some additional discipline (in an area we have not been all that disciplined). Yup, important point. I still think the location of documentation should be primarily chosen for the benefit of the reader of the code, though, and not the writer. To me this is also about having the code-base be welcoming to new developers (or old developers looking into an area of Blender new to them). > Seems fine, another issue: would we expect doc-strings to be above static function declarations too? (where static functions are forward declared before they are defined within a file) Good question. Personally I don't feel particularly strong about this, because those are typically for internal use only. Actually, documenting those at the implementation level could be beneficial; it's probably easier to understand the code when you first read through the high-level implementations, and only then get to the (documented) lower-level helper functions. The documentation for low-level helper functions will be hard to understand without the context of the high-level implementation. >> The static function doc can simply refer to the public function, IMO. [...] What a function does, and in which situations it's expected to work, is part of its public interface documentation. The fact that it defers a big part of its code to some internal function, that's an implementation detail. If that static, internal function is at the same abstraction level as the public functions, yet cannot be documented in terms of those public functions, again I see it as a code design issue, and not as a reason to avoid documenting the public interface. > > Perhaps this can be made int a guide-line then (even as part of this proposal), that static functions should refer to public functions docs instead of duplicating them. Sounds good to me. > There still remains the awkward issue of the static function containing arguments not found in the public function but I don't think there is a good way around this. These additional arguments can be documented without any conflict, right? >> Clean Code by Robert Martin suggests that high-level functions preceed low-level functions. [...] > > Not against this but seems like it could cause a lot of code-churn if applied to the current code-base, so best make that part of a separate proposal (even if it's a guide-line for new code). I agree, better to keep that for a separate proposal.

Campbell Barton commented 2021-11-09 15:10:07 +01:00

Owner

Copy Link

In #92709#1250941, @dr.sybren wrote:

In #92709#1250688, @ideasman42 wrote:
Seems fine, another issue: would we expect doc-strings to be above static function declarations too? (where static functions are forward declared before they are defined within a file)

Good question. Personally I don't feel particularly strong about this, because those are typically for internal use only. Actually, documenting those at the implementation level could be beneficial; it's probably easier to understand the code when you first read through the high-level implementations, and only then get to the (documented) lower-level helper functions. The documentation for low-level helper functions will be hard to understand without the context of the high-level implementation.

Having an exception for static functions seems reasonable, especially when these forward declarations may be added based on static function order in the file.

There still remains the awkward issue of the static function containing arguments not found in the public function but I don't think there is a good way around this.

These additional arguments can be documented without any conflict, right?

Right, it's just awkward when only some arguments are documented (makes it read as if they're missing).
Although an explicit reference to the public function may be OK in this case.

---

So the next step to come up with proposed text for https://wiki.blender.org/wiki/Style_Guide/C_Cpp#API_Docs I'd suppose.

> In #92709#1250941, @dr.sybren wrote: >> In #92709#1250688, @ideasman42 wrote: >> Seems fine, another issue: would we expect doc-strings to be above static function declarations too? (where static functions are forward declared before they are defined within a file) > > Good question. Personally I don't feel particularly strong about this, because those are typically for internal use only. Actually, documenting those at the implementation level could be beneficial; it's probably easier to understand the code when you first read through the high-level implementations, and only then get to the (documented) lower-level helper functions. The documentation for low-level helper functions will be hard to understand without the context of the high-level implementation. Having an exception for static functions seems reasonable, especially when these forward declarations may be added based on static function order in the file. >> There still remains the awkward issue of the static function containing arguments not found in the public function but I don't think there is a good way around this. > > These additional arguments can be documented without any conflict, right? Right, it's just awkward when only some arguments are documented (makes it read as if they're missing). Although an explicit reference to the public function may be OK in this case. ---- So the next step to come up with proposed text for https://wiki.blender.org/wiki/Style_Guide/C_Cpp#API_Docs I'd suppose.

Sybren A. Stüvel commented 2021-11-11 11:53:44 +01:00

Author

Member

Copy Link

In #92709#1251043, @ideasman42 wrote:

There still remains the awkward issue of the static function containing arguments not found in the public function but I don't think there is a good way around this.

These additional arguments can be documented without any conflict, right?

Right, it's just awkward when only some arguments are documented (makes it read as if they're missing).
Although an explicit reference to the public function may be OK in this case.

Yes, I would envision something like "The first 4 parameters are the same as in #the_public_function".

So the next step to come up with proposed text for https://wiki.blender.org/wiki/Style_Guide/C_Cpp#API_Docs I'd suppose.

I think the beginning of the text (about the doxygen) can be kept, up to and including the "Note that this is just the typical paragraph style used in blender with an extra leading '*'. " line.

The code example should change so that it's a function declaration and not a definition.

For the rest of the text, how's this? The emphasis is to aid in scannability of the text, to quickly understand its structure.

---

As for placement of documentation, follow these guidelines:

• Symbols (functions, constants, structs, classes, etc.) that are declared in a header file are considered part of the module's public interface, and should be documented in the header file. This makes it possible to document & organise the header file in a way that makes sense to the reader, to document groups of symbols together, and to read through the available functionality without being hindered by implementation details and internal code. This documentation should describe the public interface, but not internal implementation details that are irrelevant to calling code.
• Symbols that are internal to a file (static, anonymous namespace) should be documented at the implementation. This allows forward-declaring such functions in the implementation file, then listing the higher-level public functions, and only then have the lower-level internal/helper functions with their documentation. The documentation can be more to the point when the higher-level concepts are already known to the reader (when reading top-to-bottom throught he file).
• Implementation details that are irrelevant to the calling code should be documented at the definition/implementation of the symbol. Sometimes such information can even go inside a function, when it applies only to a part of its internals.

When there is overlap between internal and public functions, for example when two public functions actually call an internal function with some additional parameters, the internal function's documentation can refer to the public function. That way documentation doesn't have to be copied between those.

In Summary:

• Try to make it possible for developers to use a module by only reading its header file. In other words, improve black-boxing by documenting the public functions in the header file.
• Optionally use doxygen comments for detailed docs.
• Keep comments about implementation details close to the implementation.

> In #92709#1251043, @ideasman42 wrote: >>> There still remains the awkward issue of the static function containing arguments not found in the public function but I don't think there is a good way around this. >> >> These additional arguments can be documented without any conflict, right? > > Right, it's just awkward when only some arguments are documented (makes it read as if they're missing). > Although an explicit reference to the public function may be OK in this case. Yes, I would envision something like "The first 4 parameters are the same as in #the_public_function". > So the next step to come up with proposed text for https://wiki.blender.org/wiki/Style_Guide/C_Cpp#API_Docs I'd suppose. I think the beginning of the text (about the doxygen) can be kept, up to and including the "Note that this is just the typical paragraph style used in blender with an extra leading '*'. " line. The code example should change so that it's a function declaration and not a definition. For the rest of the text, how's this? The emphasis is to aid in scannability of the text, to quickly understand its structure. ------------------- As for placement of documentation, follow these guidelines: - **Symbols (functions, constants, structs, classes, etc.) that are declared in a header file** are considered part of the module's *public interface*, and should be documented in the header file. This makes it possible to document & organise the header file in a way that makes sense to the reader, to document groups of symbols together, and to read through the available functionality without being hindered by implementation details and internal code. This documentation should describe the public interface, but not internal implementation details that are irrelevant to calling code. - Symbols that are **internal to a file** (static, anonymous namespace) should be documented at the implementation. This allows forward-declaring such functions in the implementation file, then listing the higher-level public functions, and only then have the lower-level internal/helper functions with their documentation. The documentation can be more to the point when the higher-level concepts are already known to the reader (when reading top-to-bottom throught he file). - **Implementation details** that are irrelevant to the calling code should be documented at the definition/implementation of the symbol. Sometimes such information can even go inside a function, when it applies only to a part of its internals. When there is overlap between internal and public functions, for example when two public functions actually call an internal function with some additional parameters, the internal function's documentation can refer to the public function. That way documentation doesn't have to be copied between those. In Summary: - Try to make it possible for developers to use a module by only reading its header file. In other words, improve [black-boxing](https://en.wikipedia.org/wiki/Black_box) by documenting the public functions in the header file. - Optionally use doxygen comments for detailed docs. - Keep comments about implementation details close to the implementation.

Campbell Barton commented 2021-11-11 12:36:42 +01:00

Owner

Copy Link

LGTM, worth mentioning some details.

• Header & implementation doc-strings should not duplicate contents.
• These guidelines also apply to *_internal.h headers.
• Limit formal parameter and return doc-strings (\param and \return) to public doc-strings

LGTM, worth mentioning some details. - Header & implementation doc-strings should not duplicate contents. - These guidelines also apply to `*_internal.h` headers. - Limit formal parameter and return doc-strings (`\param` and `\return`) to public doc-strings

Sybren A. Stüvel commented 2021-11-16 10:51:43 +01:00

Author

Member

Copy Link

In #92709#1252292, @ideasman42 wrote:

• Limit formal parameter and return doc-strings (\param and \return) to public doc-strings

I think this only applies when there are two comment blocks (interface & implementation) on the same symbol. Personally I don't see any reason to forbid using \param and \return when documenting an internal function.

> In #92709#1252292, @ideasman42 wrote: > - Limit formal parameter and return doc-strings (`\param` and `\return`) to public doc-strings I think this only applies when there are two comment blocks (interface & implementation) on the same symbol. Personally I don't see any reason to forbid using `\param` and `\return` when documenting an internal function.

James Partsafas commented 2021-11-18 17:24:36 +01:00

Copy Link

Added subscriber: @JamesThePlant

Added subscriber: @JamesThePlant

Campbell Barton commented 2021-11-19 07:17:34 +01:00

Owner

Copy Link

Edit the updated proposal addresses this issue.

---

In #92709#1254293, @dr.sybren wrote:

In #92709#1252292, @ideasman42 wrote:

• Limit formal parameter and return doc-strings (\param and \return) to public doc-strings

I think this only applies when there are two comment blocks (interface & implementation) on the same symbol. Personally I don't see any reason to forbid using \param and \return when documenting an internal function.

Ah that was poorly worded, the point remains though, that doubling up on \param and \return in the case there are both public and private doc-strings should be avoided (doxygen reports a warning in this case), although, as noted in a previous comment, I don't think there is much of a need for this for the implementation doc-string.

**Edit** the updated proposal addresses this issue. --- > In #92709#1254293, @dr.sybren wrote: >> In #92709#1252292, @ideasman42 wrote: >> - Limit formal parameter and return doc-strings (`\param` and `\return`) to public doc-strings > > I think this only applies when there are two comment blocks (interface & implementation) on the same symbol. Personally I don't see any reason to forbid using `\param` and `\return` when documenting an internal function. Ah that was poorly worded, the point remains though, that doubling up on `\param` and `\return` in the case there are both public and private doc-strings should be avoided (doxygen reports a warning in this case), although, as noted in a previous comment, I don't think there is much of a need for this for the implementation doc-string.

Sybren A. Stüvel commented 2021-11-30 13:10:33 +01:00

Author

Member

Copy Link

Changed status from 'Confirmed' to: 'Resolved'

Changed status from 'Confirmed' to: 'Resolved'

Sybren A. Stüvel closed this issue 2021-11-30 13:10:33 +01:00

Sybren A. Stüvel self-assigned this 2021-11-30 13:10:33 +01:00

Sybren A. Stüvel commented 2021-11-30 13:10:33 +01:00

Author

Member

Copy Link

Resolved by updating https://wiki.blender.org/wiki/Style_Guide/C_Cpp#API_Docs

Resolved by updating https://wiki.blender.org/wiki/Style_Guide/C_Cpp#API_Docs

blender-admin commented 2021-12-03 09:29:35 +01:00

Copy Link

This issue was referenced by c4e041da23

This issue was referenced by c4e041da23b9c45273fcd4874308c536b6a315d1

blender-admin commented 2021-12-07 07:19:15 +01:00

Copy Link

This issue was referenced by ffc4c126f5

This issue was referenced by ffc4c126f5416b04a01653e7a03451797b98aba4

blender-admin commented 2021-12-08 07:12:33 +01:00

Copy Link

This issue was referenced by 00f3957b8e

This issue was referenced by 00f3957b8edd74f901cb52af1d2353e5b8df7437

blender-admin commented 2021-12-08 07:12:35 +01:00

Copy Link

This issue was referenced by d6c3ea9e7a

This issue was referenced by d6c3ea9e7aaa2246bb8ffbce895f97e05beeba98

blender-admin commented 2021-12-08 07:12:37 +01:00

Copy Link

This issue was referenced by 2545119112

This issue was referenced by 2545119112875a098d8d807c84b7f7a3e1bcd338

blender-admin commented 2021-12-08 07:12:39 +01:00

Copy Link

This issue was referenced by da67a19ed9

This issue was referenced by da67a19ed9b5e8c5f61926d88653c05ebd3cdf45

blender-admin commented 2021-12-08 07:12:40 +01:00

Copy Link

This issue was referenced by db795a4727

This issue was referenced by db795a4727b3a2ad56a11147181d5e3d60ca0466

blender-admin commented 2021-12-08 07:12:41 +01:00

Copy Link

This issue was referenced by 7e92717439

This issue was referenced by 7e927174396443cdc4c5544dad13faca7299d183

blender-admin commented 2021-12-08 07:12:43 +01:00

Copy Link

This issue was referenced by 93ba5e2375

This issue was referenced by 93ba5e237546f58819cdff838334470e30ec0294

blender-admin commented 2021-12-08 07:37:44 +01:00

Copy Link

This issue was referenced by 4f48b2992b

This issue was referenced by 4f48b2992bdfa2926c61457b364b75900d7416b0

blender-admin commented 2021-12-08 07:54:22 +01:00

Copy Link

This issue was referenced by 07726ef1b6

This issue was referenced by 07726ef1b609d4420ad09fb1c7273667c00b1383

blender-admin commented 2021-12-08 08:02:27 +01:00

Copy Link

This issue was referenced by 2c0ccb0159

This issue was referenced by 2c0ccb0159d17d807888af34d6f8d95a332b6c0c

blender-admin commented 2021-12-08 08:03:10 +01:00

Copy Link

This issue was referenced by a46ff1dd38

This issue was referenced by a46ff1dd38a2d23a1c0ac05cad3155398fc19e92

blender-admin commented 2021-12-08 10:29:39 +01:00

Copy Link

This issue was referenced by e89d42ddff

This issue was referenced by e89d42ddffef8892a8eb52d29e2a7329ad381204

blender-admin commented 2021-12-08 11:02:29 +01:00

Copy Link

This issue was referenced by 32b1a13fa1

This issue was referenced by 32b1a13fa1e427bc5b2cce2bf179fd274679e08f

blender-admin commented 2021-12-08 14:55:11 +01:00

Copy Link

This issue was referenced by 61776befc3

This issue was referenced by 61776befc3f88c373e47ccbdf8c75e2ca0f4e987

blender-admin commented 2021-12-09 10:01:44 +01:00

Copy Link

This issue was referenced by 9e365069af

This issue was referenced by 9e365069afe156f33fadfad9705e1325f894cd54

blender-admin commented 2021-12-09 10:01:45 +01:00

Copy Link

This issue was referenced by 9f546d6908

This issue was referenced by 9f546d690899e05b25a6ef764cc8cf2f5db918b0

blender-admin commented 2021-12-09 10:01:47 +01:00

Copy Link

This issue was referenced by 7c76bdca1b

This issue was referenced by 7c76bdca1b7195720a769c4911678d85825907fe

blender-admin commented 2021-12-09 10:01:49 +01:00

Copy Link

This issue was referenced by 8aed5dbcf8

This issue was referenced by 8aed5dbcf8e05675be8128b6197026e480622254

blender-admin commented 2021-12-09 10:11:29 +01:00

Copy Link

This issue was referenced by 15a428eab0

This issue was referenced by 15a428eab0e0a0ca63953d71e8fc719e4a06c612

blender-admin commented 2021-12-09 10:23:10 +01:00

Copy Link

This issue was referenced by 3647a1e621

This issue was referenced by 3647a1e62199f6187c6ce2c0f7a5eac5ea6b428e

blender-admin commented 2021-12-09 10:31:05 +01:00

Copy Link

This issue was referenced by cd4a7be5b2

This issue was referenced by cd4a7be5b2ec458712bd9e0a028913dcb03512d9

blender-admin commented 2021-12-09 10:58:39 +01:00

Copy Link

This issue was referenced by bc01003673

This issue was referenced by bc010036739e76315a2bf2ee19ee891debc8a7aa

blender-admin commented 2021-12-09 11:09:54 +01:00

Copy Link

This issue was referenced by 65de17ece4

This issue was referenced by 65de17ece4626ac38ee990948548fbe9bff2e283

blender-admin commented 2021-12-09 11:16:25 +01:00

Copy Link

This issue was referenced by 7f4878ac7f

This issue was referenced by 7f4878ac7f5612082a1235cf6083dc490cbfc212

blender-admin commented 2021-12-09 12:25:45 +01:00

Copy Link

This issue was referenced by 50f378e5c8

This issue was referenced by 50f378e5c8cc6aeda7e3e6d1adee2ba787a438a7

blender-admin commented 2021-12-09 12:32:42 +01:00

Copy Link

This issue was referenced by 74e57efb2d

This issue was referenced by 74e57efb2d1dee73cdef6f31131ca4b3bbc6e81c

blender-admin commented 2021-12-09 12:46:40 +01:00

Copy Link

This issue was referenced by d812e46f40

This issue was referenced by d812e46f40bce18f37b192f423e61360239a295c

blender-admin commented 2021-12-10 02:11:43 +01:00

Copy Link

This issue was referenced by 566a458950

This issue was referenced by 566a458950ee30f60400a7624746342d19cf9695

blender-admin commented 2021-12-10 11:40:30 +01:00

Copy Link

This issue was referenced by 3060217d39

This issue was referenced by 3060217d39747589d66bc4501ceaf30f59923cdc

blender-admin commented 2021-12-10 11:40:53 +01:00

Copy Link

This issue was referenced by 63f8d18c0f

This issue was referenced by 63f8d18c0fbc8bce12c65eb6bd49ec28eef703e4

blender-admin commented 2021-12-10 11:42:06 +01:00

Copy Link

This issue was referenced by bd2b48e98d

This issue was referenced by bd2b48e98d77c8437b8b0c77582084a3c984e45c

blender-admin commented 2022-04-04 23:44:36 +02:00

Copy Link

This issue was referenced by 744369c114

This issue was referenced by 744369c1144ec4c4685732e6fb06ba7b9513de41

Sign in to join this conversation.

No Branch/Tag Specified

Branches Tags

main

brush-assets-project

blender-v4.1-release

blender-v3.3-release

blender-v3.6-release

universal-scene-description

blender-v3.6-temp_wmoss_animrig_public

temp-sculpt-dyntopo

gpencil-next

anim/animation-id-113594

blender-v4.0-release

blender-projects-basics

bridge-curves

sculpt-blender

asset-browser-frontend-split

asset-shelf

tmp-usd-python-mtl

tmp-usd-3.6

blender-v3.5-release

blender-v2.93-release

realtime-clock

sculpt-dev

bevelv2

xr-dev

v4.1.1

v3.6.11

v3.3.18

v4.1.0

v3.3.17

v3.6.10

v3.6.9

v3.3.16

v3.6.8

v3.6.7

v3.3.14

v4.0.2

v4.0.1

v4.0.0

v3.6.5

v3.3.12

v3.6.4

v3.6.3

v3.3.11

v3.6.2

v3.3.10

v3.6.1

v3.3.9

v3.6.0

v3.3.8

v3.3.7

v2.93.18

v3.5.1

v3.3.6

v2.93.17

v3.5.0

v2.93.16

v3.3.5

v3.3.4

v2.93.15

v2.93.14

v3.3.3

v2.93.13

v2.93.12

v3.4.1

v3.3.2

v3.4.0

v3.3.1

v2.93.11

v3.3.0

v3.2.2

v2.93.10

v3.2.1

v3.2.0

v2.83.20

v2.93.9

v3.1.2

v3.1.1

v3.1.0

v2.83.19

v2.93.8

v3.0.1

v2.93.7

v3.0.0

v2.93.6

v2.93.5

v2.83.18

v2.93.4

v2.93.3

v2.83.17

v2.93.2

v2.93.1

v2.83.16

v2.93.0

v2.83.15

v2.83.14

v2.83.13

v2.92.0

v2.83.12

v2.91.2

v2.83.10

v2.91.0

v2.83.9

v2.83.8

v2.83.7

v2.90.1

v2.83.6.1

v2.83.6

v2.90.0

v2.83.5

v2.83.4

v2.83.3

v2.83.2

v2.83.1

v2.83

v2.82a

v2.82

v2.81a

v2.81

v2.80

v2.80-rc3

v2.80-rc2

v2.80-rc1

v2.79b

v2.79a

v2.79

v2.79-rc2

v2.79-rc1

v2.78c

v2.78b

v2.78a

v2.78

v2.78-rc2

v2.78-rc1

v2.77a

v2.77

v2.77-rc2

v2.77-rc1

v2.76b

v2.76a

v2.76

v2.76-rc3

v2.76-rc2

v2.76-rc1

v2.75a

v2.75

v2.75-rc2

v2.75-rc1

v2.74

v2.74-rc4

v2.74-rc3

v2.74-rc2

v2.74-rc1

v2.73a

v2.73

v2.73-rc1

v2.72b

2.72b

v2.72a

v2.72

v2.72-rc1

v2.71

v2.71-rc2

v2.71-rc1

v2.70a

v2.70

v2.70-rc2

v2.70-rc

v2.69

v2.68a

v2.68

v2.67b

v2.67a

v2.67

v2.66a

v2.66

v2.65a

v2.65

v2.64a

v2.64

v2.63a

v2.63

v2.61

v2.60a

v2.60

v2.59

v2.58a

v2.58

v2.57b

v2.57a

v2.57

v2.56a

v2.56

v2.55

v2.54

v2.53

v2.52

v2.51

v2.50

v2.49b

v2.49a

v2.49

v2.48a

v2.48

v2.47

v2.46

v2.45

v2.44

v2.43

v2.42a

v2.42

v2.41

v2.40

v2.37a

v2.37

v2.36

v2.35a

v2.35

v2.34

v2.33a

v2.33

v2.32

v2.31a

v2.31

v2.30

v2.28c

v2.28a

v2.28

v2.27

v2.26

v2.25

Labels

Clear labels   

Interest

Alembic

  

Interest

Animation & Rigging

  

Interest

Asset Browser

  

Interest

Asset Browser Project Overview

  

Interest

Audio

  

Interest

Automated Testing

  

Interest

Blender Asset Bundle

  

Interest

BlendFile

  

Interest

Collada

  

Interest

Compatibility

This issue affects/is about backward or forward compatibility

  

Interest

Compositing

  

Interest

Core

  

Interest

Cycles

  

Interest

Dependency Graph

  

Interest

Development Management

  

Interest

EEVEE

  

Interest

EEVEE & Viewport

  

Interest

Freestyle

  

Interest

Geometry Nodes

  

Interest

Grease Pencil

  

Interest

ID Management

  

Interest

Images & Movies

  

Interest

Import Export

  

Interest

Line Art

  

Interest

Masking

  

Interest

Metal

  

Interest

Modeling

  

Interest

Modifiers

  

Interest

Motion Tracking

  

Interest

Nodes & Physics

  

Interest

OpenGL

  

Interest

Overlay

  

Interest

Overrides

  

Interest

Performance

  

Interest

Physics

  

Interest

Pipeline, Assets & IO

  

Interest

Platforms, Builds & Tests

  

Interest

Python API

  

Interest

Render & Cycles

  

Interest

Render Pipeline

  

Interest

Sculpt, Paint & Texture

  

Interest

Text Editor

  

Interest

Translations

  

Interest

Triaging

  

Interest

Undo

  

Interest

USD

  

Interest

User Interface

  

Interest

UV Editing

  

Interest

VFX & Video

  

Interest

Video Sequencer

  

Interest

Virtual Reality

  

Interest

Vulkan

  

Interest

Wayland

Issues relating to Wayland windowing support.

  

Interest

Workbench

  

Interest: X11

Issues relating to Xorg/X11 support.

  

Legacy

Blender 2.8 Project

  

Legacy

Milestone 1: Basic, Local Asset Browser

  

Legacy

OpenGL Error

  

Meta

Good First Issue

  

Meta

Papercut

  

Meta

Retrospective

  

Meta

Security

Issues relating to security: https://wiki.blender.org/wiki/Process/Vulnerability_Reports

  

Module

Animation & Rigging

  

Module

Core

  

Module

Development Management

  

Module

EEVEE & Viewport

  

Module

Grease Pencil

  

Module

Modeling

  

Module

Nodes & Physics

  

Module

Pipeline, Assets & IO

  

Module

Platforms, Builds & Tests

  

Module

Python API

  

Module

Render & Cycles

  

Module

Sculpt, Paint & Texture

  

Module

Triaging

  

Module

User Interface

  

Module

VFX & Video

  

Platform

FreeBSD

  

Platform

Linux

  

Platform

macOS

  

Platform

Windows

  

Priority

High

  

Priority

Low

  

Priority

Normal

  

Priority

Unbreak Now!

  

Status

Archived

  

Status

Confirmed

  

Status

Duplicate

  

Status

Needs Info from Developers

  

Status

Needs Information from User

  

Status

Needs Triage

  

Status

Resolved

  

Type

Bug

  

Type

Design

  

Type

Known Issue

  

Type

Patch

  

Type

Report

  

Type

To Do

No Label

Interest

Alembic

Interest

Animation & Rigging

Interest

Asset Browser

Interest

Asset Browser Project Overview

Interest

Audio

Interest

Automated Testing

Interest

Blender Asset Bundle

Interest

BlendFile

Interest

Collada

Interest

Compatibility

Interest

Compositing

Interest

Core

Interest

Cycles

Interest

Dependency Graph

Interest

Development Management

Interest

EEVEE

Interest

EEVEE & Viewport

Interest

Freestyle

Interest

Geometry Nodes

Interest

Grease Pencil

Interest

ID Management

Interest

Images & Movies

Interest

Import Export

Interest

Line Art

Interest

Masking

Interest

Metal

Interest

Modeling

Interest

Modifiers

Interest

Motion Tracking

Interest

Nodes & Physics

Interest

OpenGL

Interest

Overlay

Interest

Overrides

Interest

Performance

Interest

Physics

Interest

Pipeline, Assets & IO

Interest

Platforms, Builds & Tests

Interest

Python API

Interest

Render & Cycles

Interest

Render Pipeline

Interest

Sculpt, Paint & Texture

Interest

Text Editor

Interest

Translations

Interest

Triaging

Interest

Undo

Interest

USD

Interest

User Interface

Interest

UV Editing

Interest

VFX & Video

Interest

Video Sequencer

Interest

Virtual Reality

Interest

Vulkan

Interest

Wayland

Interest

Workbench

Interest: X11

Legacy

Blender 2.8 Project

Legacy

Milestone 1: Basic, Local Asset Browser

Legacy

OpenGL Error

Meta

Good First Issue

Meta

Papercut

Meta

Retrospective

Meta

Security

Module

Animation & Rigging

Module

Core

Module

Development Management

Module

EEVEE & Viewport

Module

Grease Pencil

Module

Modeling

Module

Nodes & Physics

Module

Pipeline, Assets & IO

Module

Platforms, Builds & Tests

Module

Python API

Module

Render & Cycles

Module

Sculpt, Paint & Texture

Module

Triaging

Module

User Interface

Module

VFX & Video

Platform

FreeBSD

Platform

Linux

Platform

macOS

Platform

Windows

Priority

High

Priority

Low

Priority

Normal

Priority

Unbreak Now!

Status

Archived

Status

Confirmed

Status

Duplicate

Status

Needs Info from Developers

Status

Needs Information from User

Status

Needs Triage

Status

Resolved

Type

Bug

Type

Design

Type

Known Issue

Type

Patch

Type

Report

Type

To Do

Milestone

Clear milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

No Assignees

Sybren A. Stüvel

8 Participants

Notifications

Subscribe

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#92709

No description provided.