treewide: move closing part of header guard to end of file #20905
Conversation
mguetschow
requested review from
miri64,
leandrolanzieri,
aabadie,
MichelRottleuthner,
kaspar030,
bergzand,
basilfx,
PeterKietzmann,
maribu,
jue89,
jia200x,
MrKevinWeiss,
gschorcht,
jeandudey,
benpicco,
vincent-d,
dylad,
biboc,
keestux,
nandojve and
kYc0o
as code owners
github-actions
bot
added
Platform: native
github-actions
bot
added
Area: drivers
| /** @} */ | ||
| #endif /* GPIO_PARAMS_H */ |
There was a problem hiding this comment.
Here specifically the @{ and the @} would now be unbalanced, as one of them is within and the other outside of the part covered by the include guard. I'm nit sure if I am just pedantic, or if this really could cause issues.
I think we wouod be better off with #pragma once.
There was a problem hiding this comment.
Yeah, as mentioned in the initial message, that's the case everywhere.
I think we wouod be better off with
#pragma once.
I tend to agree. Let's maybe discuss #pragma once and the required coding convention changes next friday (I've already added it to the agenda).
There was a problem hiding this comment.
I second this. This unbalance was the first thing to come to my mind, upon opening this PR.
Also, I would be in support of a move to #pragma once. I moved to it years ago. It is just one less thing to think about with each new header. Reduction of cognitive load is always nice. And no worries of the include guard name colliding if you have headers of the same name in different locations, or when files get renamed, on top of all the other benefits.
There was a problem hiding this comment.
We discussed the question of C language extensions in RIOT during the weekly today, and postponed it to the VMA: https://forum.riot-os.org/t/weekly-coordination-call-friday-at-10am-cet/3068/363
|
I'll have a look at coccinelle to retain proper nesting between doxygen and include guards. |
Try to test this with a few files first and see if it has any influence on the doxygen builds... |
|
@miri64 quoting myself from the PR description:
|
|
Unfortunately, it seems like matching against preprocessor directives and comments is not supported by coccinelle: https://coccinelle.gitlabpages.inria.fr/website/docs/main_grammar012.html |
|
(ノ-_-)ノ ミ ┴┴ |
Contribution description
While reviewing #20865 (comment), I noticed that in many cases, the closing
#endifof the header guard is not the last line in the file, leading to clang-format not recognizing it as a header guard. @maribu mentioned that this could also hinder some preprocessor optimations.To produce the changes in this PR, I used the following search-replace pattern in VSCode:
(#endif .*_H \*/)\n(/\*\* @\} \*/)and$2\n$1.As a result, the preprocessor directives and doxygen groupings are not properly nested anymore, but at least to my tests, that didn't cause any harm. In general I'd say that it would make sense to have the header guard directly after the copyright and before the documentation, but that's probably something for another time.
Testing procedure
Quick glance a the changes. CI should pass and documentation should not look different (or be missing stuff).
Issues/PRs references
Noticed while reviewing #20865