Run clang-tidy as part of CI. #921
Comments
|
I'm not sure what clang-tidy does but we should run as part of check/fix format flows. |
|
Despite the name, clang-tidy isn't a formatter -- it's a static analyzer that catches common correctness and performance issues. For example, see #1023 where it flags all the times our loop variables could have been const references but were instead causing unnecessary copies. I agree this should be part of CI but offhand I don't think it's simple to do with bazel -- clang-tidy relies on a "compilation database" to understand how each file is built, and at least on a cursory look I can't find a good way to get bazel to emit one. Some folks have written their own janky-looking scripts, and I'll see if we can adapt one or produce a janky script of our own. (In the meantime, thanks to some Google-internal infrastructure, we get clang-tidy findings when we import the Envoy code into our source repo, so until CI does this properly we can at least push those fixes back upstream pretty easily.) |
|
FYI as part of getting warmed up on envoy I sort of accidentally started working on this while setting up YouCompleteMe support. Got Bazel emitting compile_commands.json (just some modifications to some aforementioned janky script) but I'm a little stuck getting include paths working right when running clang tooling from the root of the repo. Basically, it all works fine if you find and cd into the bazel exec root, but for incremental checks where you want to work live on the repo I have to figure out how to get compile_commands pointed at all the right spot. E.g. |
|
p.s., literally the first file i ran clang-tidy on i (rather, it) found a nasty bug waiting to happen, so we should definitely get this available! for devs locally and eventually in CI |
|
Did anyone try this recently? I was able to generate compilation database with https://github.com/grailbio/bazel-compilation-database and then run clang-tidy with it. |
|
@lizan for compilation database, there is also Kythe, which supports extracting compilations using Bazel, and which might be potentially slightly more useful for us in the future. |
|
@lizan if you can make this work that would be awesome. Do you want to own this one? |
|
Sure I will take this one if no one is working on that. @PiotrSikora I tried Kythe for a bit but it requires too many Java dependencies to generate compilation db. |
Add a script to generate compile database, this can be used for auto completion for editors or clang-tidy. Part of #921 Risk Level: Low Testing: manual Docs Changes: N/A Release Notes: N/A Signed-off-by: Lizan Zhou <lizan@tetrate.io>
Add a script to generate compile database, this can be used for auto completion for editors or clang-tidy. Part of envoyproxy#921 Risk Level: Low Testing: manual Docs Changes: N/A Release Notes: N/A Signed-off-by: Lizan Zhou <lizan@tetrate.io> Signed-off-by: Aaltan Ahmad <aa@stripe.com>
Fixes the following warnings when compiling with Xcode 11.5:
```
INFO: From Compiling Swift module Envoy:
library/swift/src/grpc/GRPCStream.swift:56:23: warning: initialization of 'UnsafeBufferPointer<UInt32>' results in a dangling buffer pointer
prefixData.append(UnsafeBufferPointer(start: &length, count: 1))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
library/swift/src/grpc/GRPCStream.swift:56:50: note: implicit argument conversion from 'UInt32' to 'UnsafePointer<UInt32>?' produces a pointer valid only for the duration of the call to 'init(start:count:)'
prefixData.append(UnsafeBufferPointer(start: &length, count: 1))
^~~~~~~
library/swift/src/grpc/GRPCStream.swift:56:50: note: use 'withUnsafePointer' in order to explicitly convert argument to pointer valid for a defined scope
prefixData.append(UnsafeBufferPointer(start: &length, count: 1))
^
```
Signed-off-by: Michael Rebello <me@michaelrebello.com>
Signed-off-by: JP Simard <jp@jpsim.com>
Fixes the following warnings when compiling with Xcode 11.5:
```
INFO: From Compiling Swift module Envoy:
library/swift/src/grpc/GRPCStream.swift:56:23: warning: initialization of 'UnsafeBufferPointer<UInt32>' results in a dangling buffer pointer
prefixData.append(UnsafeBufferPointer(start: &length, count: 1))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
library/swift/src/grpc/GRPCStream.swift:56:50: note: implicit argument conversion from 'UInt32' to 'UnsafePointer<UInt32>?' produces a pointer valid only for the duration of the call to 'init(start:count:)'
prefixData.append(UnsafeBufferPointer(start: &length, count: 1))
^~~~~~~
library/swift/src/grpc/GRPCStream.swift:56:50: note: use 'withUnsafePointer' in order to explicitly convert argument to pointer valid for a defined scope
prefixData.append(UnsafeBufferPointer(start: &length, count: 1))
^
```
Signed-off-by: Michael Rebello <me@michaelrebello.com>
Signed-off-by: JP Simard <jp@jpsim.com>
Now, that we have Clang in CI, it would be good idea to catch common mistakes with
clang-tidy.@htuch
The text was updated successfully, but these errors were encountered: