envoyproxy / envoy

Cloud-native high-performance edge/middle/service proxy
https://www.envoyproxy.io
Apache License 2.0
24.96k stars 4.8k forks source link

fix clang-tidy reported errors on master #4863

Open lizan opened 6 years ago

lizan commented 6 years ago

Description: clang-tidy runs against diffs in CI, but it will caught all errors on the lines the PR author touched. Ideally we should make a full clang-tidy run error free.

zyfjeff commented 6 years ago

I am very interested, I can come to fix the issue

lizan commented 6 years ago

@zyfjeff Great, I think you can just run clang-tidy with -fix flag with the checks marked as error.

auni53 commented 6 years ago

Not directly relevant, but would be nice for instructions on clang-tidy to be added in the same places as instructions on fix_format, e.g. https://github.com/envoyproxy/envoy/blob/15706964a29e595c045a5d7ef0646d04f347dcc1/support/README.md

lizan commented 6 years ago

@auni53 Definitely, do you want to take that part?

auni53 commented 6 years ago

Can do :)

auni53 commented 6 years ago

I have no idea how to run the clang-tidy script locally. Documenting here to try and prevent others running into the same issue...


$ ./ci/run_envoy_docker.sh './ci/do_ci.sh bazel.clang_tidy'         

No remote cache bucket is set, skipping setup remote cache.
ENVOY_SRCDIR=/source
fatal: Not a git repository (or any parent up to mount point /build)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).```
auni53 commented 6 years ago

ah, rm -rf /tmp/envoy-docker-build did the trick.

auni53 commented 6 years ago

OK yeah that did as far as running the script, but I'm continually running into errors. I have no idea how any of this works, but here's what I understand:

lizan commented 6 years ago

@auni53 'ci/run_clang_tidy.sh' is written for running from do_ci.sh so it may depend on env vars from there. Usually I do tools/gen_compilation_database.py --run_bazel_build --include_header && run-clang-tidy-7, let me know (or ping me on Slack) when you face any issue.

auni53 commented 6 years ago

hah, took a day to realize that should be run-clang-tidy -7 (in case anyone else comes along this)

lizan commented 5 years ago

@auni53 no run-clang-tidy-7 (without space) is the right one if you're in envoy-build-docker or have llvm installed via official apt repository.

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or other activity occurs. Thank you for your contributions.

derekargueta commented 5 years ago

latest status of master errors that I'm getting with clang-tidy-8

dereka@dev-dereka-2:~/code/envoy-oss$ make clang-tidy 2>&1 | grep " error: " | grep -v "chromium_url"
/home/dereka/.cache/bazel/_bazel_dereka/b4e08c1eaa8adbb40db2e2ca7ea0cbf2/execroot/envoy/include/envoy/server/admin.h:124:53: error: invalid case style for parameter 'access_log_path_' [readability-identifier-naming,-warnings-as-errors]
/home/dereka/.cache/bazel/_bazel_dereka/b4e08c1eaa8adbb40db2e2ca7ea0cbf2/execroot/envoy/include/envoy/server/admin.h:124:53: error: invalid case style for parameter 'access_log_path_' [readability-identifier-naming,-warnings-as-errors]
bazel-out/k8-fastbuild/bin/source/extensions/quic_listeners/quiche/_virtual_includes/envoy_quic_server_session_lib/extensions/quic_listeners/quiche/envoy_quic_server_session.h:16:10: error: 'extensions/quic_listeners/quiche/quic_filter_manager_connection_impl.h' file not found [clang-diagnostic-error]
bazel-out/k8-fastbuild/bin/source/extensions/quic_listeners/quiche/_virtual_includes/envoy_quic_server_session_lib/extensions/quic_listeners/quiche/envoy_quic_server_session.h:16:10: error: 'extensions/quic_listeners/quiche/quic_filter_manager_connection_impl.h' file not found [clang-diagnostic-error]
source/extensions/quic_listeners/quiche/platform/quic_stack_trace_impl.h:16:22: error: implicit instantiation of undefined template 'std::__cxx11::basic_ostringstream<char, std::char_traits<char>, std::allocator<char> >' [clang-diagnostic-error]
bazel-out/k8-fastbuild/bin/source/extensions/quic_listeners/quiche/_virtual_includes/envoy_quic_server_session_lib/extensions/quic_listeners/quiche/envoy_quic_server_session.h:16:10: error: 'extensions/quic_listeners/quiche/quic_filter_manager_connection_impl.h' file not found [clang-diagnostic-error]
bazel-out/k8-fastbuild/bin/source/extensions/quic_listeners/quiche/_virtual_includes/envoy_quic_server_session_lib/extensions/quic_listeners/quiche/envoy_quic_server_session.h:16:10: error: 'extensions/quic_listeners/quiche/quic_filter_manager_connection_impl.h' file not found [clang-diagnostic-error]
source/extensions/quic_listeners/quiche/platform/quic_stack_trace_impl.h:16:22: error: implicit instantiation of undefined template 'std::__cxx11::basic_ostringstream<char, std::char_traits<char>, std::allocator<char> >' [clang-diagnostic-error]
bazel-out/k8-fastbuild/bin/source/extensions/quic_listeners/quiche/_virtual_includes/envoy_quic_server_session_lib/extensions/quic_listeners/quiche/envoy_quic_server_session.h:16:10: error: 'extensions/quic_listeners/quiche/quic_filter_manager_connection_impl.h' file not found [clang-diagnostic-error]
bazel-out/k8-fastbuild/bin/source/extensions/quic_listeners/quiche/_virtual_includes/envoy_quic_server_session_lib/extensions/quic_listeners/quiche/envoy_quic_server_session.h:16:10: error: 'extensions/quic_listeners/quiche/quic_filter_manager_connection_impl.h' file not found [clang-diagnostic-error]
test/server/filter_chain_benchmark_test.cc:174:20: error: initializer for base class 'Envoy::Server::FilterChainBenchmarkFixture' is redundant [readability-redundant-member-init,-warnings-as-errors]
test/server/filter_chain_benchmark_test.cc:183:20: error: initializer for base class 'Envoy::Server::FilterChainBenchmarkFixture' is redundant [readability-redundant-member-init,-warnings-as-errors]
/home/dereka/.cache/bazel/_bazel_dereka/b4e08c1eaa8adbb40db2e2ca7ea0cbf2/execroot/envoy/test/common/upstream/cluster_manager_impl_test.cc:49:16: error: using decl 'ByRef' is unused [misc-unused-using-decls,-warnings-as-errors]
dereka@dev-dereka-2:~/code/envoy-oss$
arvryna commented 3 years ago

hi, anyone working on this issue ?

gyohuangxin commented 2 years ago

Did anyone meet this issue when running full clang-tidy under docker?

$ ./ci/run_envoy_docker.sh "RUN_FULL_CLANG_TIDY=1 ./ci/do_ci.sh bazel.clang_tidy"
3de483a98c5e24973e710b4f97b2dabcd3cb621f: Pulling from envoyproxy/envoy-build-ubuntu
Digest: sha256:a0dfdec963cec68c0636578ec1587cc542f9839584f32bb56165284d64cbafe6
Status: Image is up to date for envoyproxy/envoy-build-ubuntu:3de483a98c5e24973e710b4f97b2dabcd3cb621f
docker.io/envoyproxy/envoy-build-ubuntu:3de483a98c5e24973e710b4f97b2dabcd3cb621f

real    0m3.934s
user    0m0.029s
sys     0m0.003s
No remote cache is set, skipping setup remote cache.
ENVOY_SRCDIR=/source
ENVOY_BUILD_TARGET=//source/exe:envoy-static
ENVOY_BUILD_ARCH=x86_64
$TEST_TMPDIR defined: output root default is '/build/tmp' and max_idle_secs default is '15'.
Starting local Bazel server and connecting to it...
WARNING: info command does not support starlark options. Ignoring options: [--@com_googlesource_googleurl//build_config:system_icu=0]
Skip setting up Envoy Filter Example.
building using 8 CPUs
building for x86_64
clang toolchain with libstdc++ configured
Generating compilation database...
$TEST_TMPDIR defined: output root default is '/build/tmp' and max_idle_secs default is '15'.
INFO: Analyzed 3758 targets (1466 packages loaded, 92970 targets configured).
INFO: Found 3758 targets...
INFO: Elapsed time: 36.237s, Critical Path: 0.58s
INFO: 1 process: 1 internal.
$TEST_TMPDIR defined: output root default is '/build/tmp' and max_idle_secs default is '15'.
WARNING: info command does not support starlark options. Ignoring options: [--@com_googlesource_googleurl//build_config:system_icu=0]
Running a full clang-tidy
python3: can't open file '/opt/llvm/share/clang/run-clang-tidy.py': [Errno 2] No such file or directory