docs: Add documentation for all cimple linters. #226
Conversation
iphydf
changed the title
iphydf
force-pushed
the
docs
branch
2 times, most recently
from
a1d31ad
to
f3c2931
Compare
iphydf
force-pushed
the
docs
branch
3 times, most recently
from
a86d223
to
a7e2bec
Compare
| **Reason:** boolean returns using `bool` (or an `enum` type) are clearer than | ||
| ones returning an `int` that happens to only have 2 possible values. | ||
|
|
||
| ## `-Wbooleans` |
There was a problem hiding this comment.
I think this one is available in clang-tidy as well: https://releases.llvm.org/14.0.0/tools/clang/tools/extra/docs/clang-tidy/checks/readability-simplify-boolean-expr.html
There was a problem hiding this comment.
That's good, but apparently it doesn't always trigger correctly. We have "*" enabled in clang-tidy, but I wrote this despite clang-tidy existing, being in use, and yet we had these. All linters written here are things we found that no other tool found, and thus wanted to fix everywhere.
| **Reason:** compound literals aren't needed in initialisations. Without them, | ||
| the code is clearer. | ||
|
|
||
| ## `-Wconstness` |
There was a problem hiding this comment.
clang-tidy has https://clang.llvm.org/extra/clang-tidy/checks/misc/const-correctness.html which I think matches this check.
There was a problem hiding this comment.
Again I implemented this because clang-tidy didn't catch it, or not in the way we wanted it. I'll have a look at both of these to see if anything has improved since I did this.
There was a problem hiding this comment.
I see this one is newer than our clang-tidy version (14). I'll upgrade and see if it works.
There was a problem hiding this comment.
I tested it. I removed const here and ran only misc-const-correctness, and got no warnings. Let me know if I'm doing something wrong.
| unexported implementation detail, and there is no compiler that can check | ||
| whether our declaration matches the implementation's definition. | ||
|
|
||
| ## `-Wlarge-struct-params` |
There was a problem hiding this comment.
clang-tidy has https://clang.llvm.org/extra/clang-tidy/checks/performance/unnecessary-value-param.html which should be able to detect this.
There was a problem hiding this comment.
Not quite:
The check is only applied to parameters of types that are expensive to copy which means they are not trivially copyable or have a non-trivial copy constructor or destructor.
All our types are trivially copyable. That doesn't mean they are cheap to copy.
| stack overflows. Since we can't currently measure the size, we avoid any struct | ||
| passing altogether apart from some well-known exemptions. | ||
|
|
||
| ## `-Wlogger-calls` |
There was a problem hiding this comment.
-Wformat-nonliteral could probably do this too?
There was a problem hiding this comment.
Not quite. We actually want it to be a true string literal, not a thing composed of macros and strings and not a bunch of string literals concatenated.
|
I didn't know toktok-releaser merged even with unresolved conversations, but oh well. |
Fixed that. It won't do that again. |
This change is