[PATCH] D59963: [clang-tidy] Add a module for the Linux kernel.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst:10
+functions are marked with ``__attribute__((warn_unused_result))``, but
+the compiler warning for this attribute is not always enabled.
+
----------------
tmroeder wrote:
> gribozavr wrote:
> > IIRC it is possible to pass through compiler warnings through ClangTidy... WDYT about that instead of reimplementing the warning?
> > 
> > However we would lose the ability to "infer" `warn_unused_result` on functions that return `ERR_PTR`.  However, since the analysis is not cross-translation-unit, IDK how much value there is.
> I don't know exactly how to pass the warnings through, but I'd be interested in learning how to do that. I agree that that would be cleaner than this (partial) reimplementation.
> 
> Note that my code above does do something like that: I currently check that the unused-result attribute is set on the return from the call.
> I don't know exactly how to pass the warnings through, but I'd be interested in learning how to do that. I agree that that would be cleaner than this (partial) reimplementation.

It seems like it is done by default, and `-Wunused-result` is enabled by default:

```
$ cat /tmp/example.cpp
int __attribute__((warn_unused_result)) foo();

void bar() {
  foo();
}
$ ./bin/clang-tidy /tmp/example.cpp
1 warning generated.
/tmp/example.cpp:4:3: warning: ignoring return value of function declared with 'warn_unused_result' attribute [clang-diagnostic-unused-result]
  foo();
  ^
```

If it does not work for you, you can enable it on the command line:

```
clang-tidy -extra-arg=-Wunused-result -checks=clang-diagnostic-unused-result /tmp/example.cpp
```

> Note that my code above does do something like that: I currently check that the unused-result attribute is set on the return from the call.

Yes, I was saying that we would lose the ability to do that.  However, is it that valuable?  The analysis in ClangTidy does not cross translation unit boundaries, so unless the caller and the callee are in the same file, this inference does not buy us much.

You could also use an existing check, `bugprone-unused-return-value`, without even requiring the function to be annotated with the attribute:

```
$ cat /tmp/example.cpp
int foo();

void bar() {
  foo();
}
$ ./bin/clang-tidy /tmp/example.cpp -config="{Checks: 'bugprone-unused-return-value', CheckOptions: [{key: 'bugprone-unused-return-value.CheckedFunctions', value: 'foo'}]}"
1 warning generated.
/tmp/example.cpp:4:3: warning: the value returned by this function should be used [bugprone-unused-return-value]
  foo();
  ^
/tmp/example.cpp:4:3: note: cast the expression to void to silence this warning
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59963/new/

https://reviews.llvm.org/D59963