From e602169fa0ff436f37e5843bf340f45aebdfc18a Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 5 Nov 2021 12:05:13 -0700 Subject: [PATCH] First pass IWYU tooling (#27857) * First pass IWYU tooling * fix * Automated change: Fix sanity tests * Update iwyu.sh * Update iwyu.sh * Update Dockerfile.template * Update iwyu.sh * Automated change: Fix sanity tests Co-authored-by: ctiller --- .gitignore | 5 ++ BUILD | 3 + src/core/lib/promise/activity.cc | 2 + src/core/lib/promise/activity.h | 14 +++++ src/core/lib/promise/context.h | 1 + src/core/lib/promise/detail/basic_join.h | 7 +++ src/core/lib/promise/detail/basic_seq.h | 7 +++ src/core/lib/promise/detail/promise_factory.h | 4 ++ src/core/lib/promise/detail/promise_like.h | 2 + src/core/lib/promise/detail/status.h | 2 + .../lib/promise/exec_ctx_wakeup_scheduler.h | 3 + src/core/lib/promise/for_each.h | 4 ++ src/core/lib/promise/if.h | 3 + src/core/lib/promise/intra_activity_waiter.h | 1 + src/core/lib/promise/join.h | 2 + src/core/lib/promise/loop.h | 5 ++ src/core/lib/promise/map.h | 4 ++ src/core/lib/promise/observable.h | 9 +++ src/core/lib/promise/pipe.h | 13 ++++- src/core/lib/promise/poll.h | 2 + src/core/lib/promise/promise.h | 2 + src/core/lib/promise/race.h | 3 + src/core/lib/promise/seq.h | 2 + src/core/lib/promise/try_join.h | 7 +++ src/core/lib/promise/try_seq.h | 2 + src/core/lib/promise/wait_set.h | 3 + .../dockerfile/grpc_iwyu/Dockerfile.template | 35 ++++++++++++ tools/distrib/iwyu.sh | 55 +++++++++++++++++++ tools/dockerfile/grpc_iwyu/Dockerfile | 33 +++++++++++ tools/dockerfile/grpc_iwyu/iwyu.sh | 43 +++++++++++++++ 30 files changed, 275 insertions(+), 3 deletions(-) create mode 100644 templates/tools/dockerfile/grpc_iwyu/Dockerfile.template create mode 100755 tools/distrib/iwyu.sh create mode 100644 tools/dockerfile/grpc_iwyu/Dockerfile create mode 100755 tools/dockerfile/grpc_iwyu/iwyu.sh diff --git a/.gitignore b/.gitignore index 14cdcebe439..d0d363e1a06 100644 --- a/.gitignore +++ b/.gitignore @@ -157,3 +157,8 @@ BenchmarkDotNet.Artifacts/ # clang JSON compilation database file compile_commands.json + +# IWYU byproducts +compile_commands_for_iwyu.json +iwyu.out +iwyu_files.txt diff --git a/BUILD b/BUILD index d4d708d4585..020da669833 100644 --- a/BUILD +++ b/BUILD @@ -1246,6 +1246,9 @@ grpc_cc_library( srcs = [ "src/core/lib/promise/activity.cc", ], + external_deps = [ + "absl/base:core_headers", + ], language = "c++", public_hdrs = [ "src/core/lib/promise/activity.h", diff --git a/src/core/lib/promise/activity.cc b/src/core/lib/promise/activity.cc index 979d085967e..48a5cf30f80 100644 --- a/src/core/lib/promise/activity.cc +++ b/src/core/lib/promise/activity.cc @@ -16,6 +16,8 @@ #include "src/core/lib/promise/activity.h" +#include "absl/base/attributes.h" + #include "src/core/lib/gprpp/atomic_utils.h" namespace grpc_core { diff --git a/src/core/lib/promise/activity.h b/src/core/lib/promise/activity.h index b1d62f41419..5403aa8d368 100644 --- a/src/core/lib/promise/activity.h +++ b/src/core/lib/promise/activity.h @@ -17,10 +17,24 @@ #include +#include +#include + +#include +#include #include +#include +#include + +#include "absl/base/thread_annotations.h" +#include "absl/status/status.h" +#include "absl/types/optional.h" +#include "absl/types/variant.h" +#include "absl/utility/utility.h" #include +#include "src/core/lib/gpr/tls.h" #include "src/core/lib/gprpp/construct_destruct.h" #include "src/core/lib/gprpp/sync.h" #include "src/core/lib/promise/context.h" diff --git a/src/core/lib/promise/context.h b/src/core/lib/promise/context.h index b9862a9cf6f..dfee3d28a6b 100644 --- a/src/core/lib/promise/context.h +++ b/src/core/lib/promise/context.h @@ -17,6 +17,7 @@ #include +#include #include #include "src/core/lib/gpr/tls.h" diff --git a/src/core/lib/promise/detail/basic_join.h b/src/core/lib/promise/detail/basic_join.h index 6fe6679de55..17c5e42bd15 100644 --- a/src/core/lib/promise/detail/basic_join.h +++ b/src/core/lib/promise/detail/basic_join.h @@ -17,14 +17,21 @@ #include +#include +#include + +#include #include +#include #include +#include "absl/types/variant.h" #include "absl/utility/utility.h" #include "src/core/lib/gprpp/bitset.h" #include "src/core/lib/gprpp/construct_destruct.h" #include "src/core/lib/promise/detail/promise_factory.h" +#include "src/core/lib/promise/detail/promise_like.h" #include "src/core/lib/promise/poll.h" namespace grpc_core { diff --git a/src/core/lib/promise/detail/basic_seq.h b/src/core/lib/promise/detail/basic_seq.h index 991cd46e4cb..e6ea1a9d1d5 100644 --- a/src/core/lib/promise/detail/basic_seq.h +++ b/src/core/lib/promise/detail/basic_seq.h @@ -17,8 +17,13 @@ #include +#include #include +#include +#include +#include +#include "absl/meta/type_traits.h" #include "absl/types/variant.h" #include "absl/utility/utility.h" @@ -29,6 +34,8 @@ namespace grpc_core { namespace promise_detail { +template +class PromiseLike; // Helper for SeqState to evaluate some common types to all partial // specializations. diff --git a/src/core/lib/promise/detail/promise_factory.h b/src/core/lib/promise/detail/promise_factory.h index 2da6a8bd357..567bfccee02 100644 --- a/src/core/lib/promise/detail/promise_factory.h +++ b/src/core/lib/promise/detail/promise_factory.h @@ -17,6 +17,9 @@ #include +#include +#include + #include "absl/meta/type_traits.h" #include "src/core/lib/promise/detail/promise_like.h" @@ -65,6 +68,7 @@ struct IsVoidCallable()())>> { // Given F(A,B,C,...), what's the return type? template struct ResultOfT; + template struct ResultOfT>()( diff --git a/src/core/lib/promise/detail/promise_like.h b/src/core/lib/promise/detail/promise_like.h index b2af650c617..c0b32f1ef0e 100644 --- a/src/core/lib/promise/detail/promise_like.h +++ b/src/core/lib/promise/detail/promise_like.h @@ -19,6 +19,8 @@ #include +#include "absl/meta/type_traits.h" + #include "src/core/lib/promise/poll.h" // A Promise is a callable object that returns Poll for some T. diff --git a/src/core/lib/promise/detail/status.h b/src/core/lib/promise/detail/status.h index 92a980f0390..79b070e8d22 100644 --- a/src/core/lib/promise/detail/status.h +++ b/src/core/lib/promise/detail/status.h @@ -17,6 +17,8 @@ #include +#include + #include "absl/status/status.h" #include "absl/status/statusor.h" diff --git a/src/core/lib/promise/exec_ctx_wakeup_scheduler.h b/src/core/lib/promise/exec_ctx_wakeup_scheduler.h index 5af59de26b2..168cbcc4ceb 100644 --- a/src/core/lib/promise/exec_ctx_wakeup_scheduler.h +++ b/src/core/lib/promise/exec_ctx_wakeup_scheduler.h @@ -17,6 +17,9 @@ #include +#include "src/core/lib/gprpp/debug_location.h" +#include "src/core/lib/iomgr/closure.h" +#include "src/core/lib/iomgr/error.h" #include "src/core/lib/iomgr/exec_ctx.h" namespace grpc_core { diff --git a/src/core/lib/promise/for_each.h b/src/core/lib/promise/for_each.h index 42c70839a65..f9540757c20 100644 --- a/src/core/lib/promise/for_each.h +++ b/src/core/lib/promise/for_each.h @@ -17,6 +17,9 @@ #include +#include +#include + #include "absl/status/status.h" #include "absl/types/variant.h" @@ -45,6 +48,7 @@ Poll FinishIteration(absl::Status* r, Reader* reader, // easily. template struct Done; + template <> struct Done { static absl::Status Make() { return absl::OkStatus(); } diff --git a/src/core/lib/promise/if.h b/src/core/lib/promise/if.h index 04fcf5e5cb5..89f11479b7a 100644 --- a/src/core/lib/promise/if.h +++ b/src/core/lib/promise/if.h @@ -17,10 +17,13 @@ #include +#include + #include "absl/status/statusor.h" #include "absl/types/variant.h" #include "src/core/lib/promise/detail/promise_factory.h" +#include "src/core/lib/promise/detail/promise_like.h" #include "src/core/lib/promise/poll.h" namespace grpc_core { diff --git a/src/core/lib/promise/intra_activity_waiter.h b/src/core/lib/promise/intra_activity_waiter.h index b0eb299a066..889323bf3ec 100644 --- a/src/core/lib/promise/intra_activity_waiter.h +++ b/src/core/lib/promise/intra_activity_waiter.h @@ -18,6 +18,7 @@ #include #include "src/core/lib/promise/activity.h" +#include "src/core/lib/promise/poll.h" namespace grpc_core { diff --git a/src/core/lib/promise/join.h b/src/core/lib/promise/join.h index b936901d3d9..d8088d0bda9 100644 --- a/src/core/lib/promise/join.h +++ b/src/core/lib/promise/join.h @@ -17,6 +17,8 @@ #include +#include "absl/meta/type_traits.h" + #include "src/core/lib/promise/detail/basic_join.h" namespace grpc_core { diff --git a/src/core/lib/promise/loop.h b/src/core/lib/promise/loop.h index 6d6f4af4b03..847b7d654fc 100644 --- a/src/core/lib/promise/loop.h +++ b/src/core/lib/promise/loop.h @@ -17,9 +17,13 @@ #include +#include +#include + #include "absl/types/variant.h" #include "src/core/lib/promise/detail/promise_factory.h" +#include "src/core/lib/promise/poll.h" namespace grpc_core { @@ -36,6 +40,7 @@ namespace promise_detail { template struct LoopTraits; + template struct LoopTraits> { using Result = T; diff --git a/src/core/lib/promise/map.h b/src/core/lib/promise/map.h index 7d43de09d18..d4090beb69d 100644 --- a/src/core/lib/promise/map.h +++ b/src/core/lib/promise/map.h @@ -17,7 +17,11 @@ #include +#include + #include +#include +#include #include "absl/types/variant.h" diff --git a/src/core/lib/promise/observable.h b/src/core/lib/promise/observable.h index 1509851fe31..4c3be986577 100644 --- a/src/core/lib/promise/observable.h +++ b/src/core/lib/promise/observable.h @@ -17,11 +17,20 @@ #include +#include + #include +#include +#include +#include "absl/base/thread_annotations.h" +#include "absl/synchronization/mutex.h" #include "absl/types/optional.h" +#include "absl/types/variant.h" #include "src/core/lib/promise/activity.h" +#include "src/core/lib/promise/detail/promise_like.h" +#include "src/core/lib/promise/poll.h" #include "src/core/lib/promise/wait_set.h" namespace grpc_core { diff --git a/src/core/lib/promise/pipe.h b/src/core/lib/promise/pipe.h index d0f472eece4..fb53ff17476 100644 --- a/src/core/lib/promise/pipe.h +++ b/src/core/lib/promise/pipe.h @@ -17,9 +17,19 @@ #include +#include +#include +#include + +#include +#include +#include + +#include "absl/container/inlined_vector.h" #include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/types/optional.h" +#include "absl/types/variant.h" #include "src/core/lib/promise/activity.h" #include "src/core/lib/promise/detail/promise_factory.h" @@ -30,10 +40,8 @@ namespace grpc_core { template struct Pipe; - template class PipeSender; - template class PipeReceiver; @@ -41,7 +49,6 @@ namespace pipe_detail { template class Push; - template class Next; diff --git a/src/core/lib/promise/poll.h b/src/core/lib/promise/poll.h index b7cd470f50b..16fb74e4dfd 100644 --- a/src/core/lib/promise/poll.h +++ b/src/core/lib/promise/poll.h @@ -17,6 +17,8 @@ #include +#include + #include "absl/types/variant.h" namespace grpc_core { diff --git a/src/core/lib/promise/promise.h b/src/core/lib/promise/promise.h index ecba7588fbf..5ae27d42ee7 100644 --- a/src/core/lib/promise/promise.h +++ b/src/core/lib/promise/promise.h @@ -18,8 +18,10 @@ #include #include +#include #include "absl/types/optional.h" +#include "absl/types/variant.h" #include "src/core/lib/promise/detail/promise_like.h" #include "src/core/lib/promise/poll.h" diff --git a/src/core/lib/promise/race.h b/src/core/lib/promise/race.h index 4b55bcc0f5d..4ecb359a82a 100644 --- a/src/core/lib/promise/race.h +++ b/src/core/lib/promise/race.h @@ -18,6 +18,9 @@ #include #include +#include + +#include "absl/types/variant.h" #include "src/core/lib/promise/poll.h" diff --git a/src/core/lib/promise/seq.h b/src/core/lib/promise/seq.h index a6342a088b0..250c6eb39ff 100644 --- a/src/core/lib/promise/seq.h +++ b/src/core/lib/promise/seq.h @@ -17,6 +17,8 @@ #include +#include + #include "absl/types/variant.h" #include "src/core/lib/promise/detail/basic_seq.h" diff --git a/src/core/lib/promise/try_join.h b/src/core/lib/promise/try_join.h index 9239d93999f..095fc1a9444 100644 --- a/src/core/lib/promise/try_join.h +++ b/src/core/lib/promise/try_join.h @@ -17,8 +17,15 @@ #include +#include + +#include "absl/meta/type_traits.h" +#include "absl/status/status.h" +#include "absl/status/statusor.h" + #include "src/core/lib/promise/detail/basic_join.h" #include "src/core/lib/promise/detail/status.h" +#include "src/core/lib/promise/poll.h" namespace grpc_core { diff --git a/src/core/lib/promise/try_seq.h b/src/core/lib/promise/try_seq.h index 3e142a94bd8..ee41e144531 100644 --- a/src/core/lib/promise/try_seq.h +++ b/src/core/lib/promise/try_seq.h @@ -18,7 +18,9 @@ #include #include +#include +#include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/types/variant.h" diff --git a/src/core/lib/promise/wait_set.h b/src/core/lib/promise/wait_set.h index bdfc85012c6..79a0e0f2ea0 100644 --- a/src/core/lib/promise/wait_set.h +++ b/src/core/lib/promise/wait_set.h @@ -17,9 +17,12 @@ #include +#include + #include "absl/container/flat_hash_set.h" #include "src/core/lib/promise/activity.h" +#include "src/core/lib/promise/poll.h" namespace grpc_core { diff --git a/templates/tools/dockerfile/grpc_iwyu/Dockerfile.template b/templates/tools/dockerfile/grpc_iwyu/Dockerfile.template new file mode 100644 index 00000000000..f31c4b658e2 --- /dev/null +++ b/templates/tools/dockerfile/grpc_iwyu/Dockerfile.template @@ -0,0 +1,35 @@ +%YAML 1.2 +--- | + # Copyright 2021 gRPC authors. + # + # Licensed under the Apache License, Version 2.0 (the "License"); + # you may not use this file except in compliance with the License. + # You may obtain a copy of the License at + # + # http://www.apache.org/licenses/LICENSE-2.0 + # + # Unless required by applicable law or agreed to in writing, software + # distributed under the License is distributed on an "AS IS" BASIS, + # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + # See the License for the specific language governing permissions and + # limitations under the License. + + FROM debian:bullseye + + RUN apt-get update && apt-get install -y clang-11 llvm-11-dev libclang-11-dev clang-format-11 jq git cmake python + + ENV CLANG_FORMAT=clang-format-11 + + RUN git clone https://github.com/include-what-you-use/include-what-you-use.git /iwyu + # latest commit on the clang 11 branch + RUN cd /iwyu && git checkout 5db414ac448004fe019871c977905cb7c2cff23f + + RUN mkdir /iwyu_build && cd /iwyu_build && cmake -G "Unix Makefiles" -DCMAKE_PREFIX_PATH=/usr/lib/llvm-11 /iwyu && make + + ADD iwyu.sh / + + # When running locally, we'll be impersonating the current user, so we need + # to make the script runnable by everyone. + RUN chmod a+rx /iwyu.sh + + CMD ["echo 'Run with tools/distrib/iwyu.sh'"] diff --git a/tools/distrib/iwyu.sh b/tools/distrib/iwyu.sh new file mode 100755 index 00000000000..6ca9b7c21fd --- /dev/null +++ b/tools/distrib/iwyu.sh @@ -0,0 +1,55 @@ +#!/bin/bash +# Copyright 2021 gRPC authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +echo "NOTE: to automagically apply fixes, invoke with --fix" + +set -ex + +# change to root directory +cd $(dirname $0)/../.. +REPO_ROOT=$(pwd) + +# grep targets with manual tag, which is not included in a result of bazel build using ... +# let's get a list of them using query command and pass it to gen_compilation_database.py +export MANUAL_TARGETS=$(bazel query 'attr("tags", "manual", tests(//test/cpp/...))' | grep -v _on_ios) + +# generate a clang compilation database for all C/C++ sources in the repo. +tools/distrib/gen_compilation_database.py \ + --include_headers \ + --ignore_system_headers \ + --dedup_targets \ + "//:*" \ + "//src/core/..." \ + "//src/compiler/..." \ + "//test/core/..." \ + "//test/cpp/..." \ + $MANUAL_TARGETS + +# build clang-tidy docker image +docker build -t grpc_iwyu tools/dockerfile/grpc_iwyu + +# run clang-tidy against the checked out codebase +# when modifying the checked-out files, the current user will be impersonated +# so that the updated files don't end up being owned by "root". +docker run \ + -e TEST="$TEST" \ + -e CHANGED_FILES="$CHANGED_FILES" \ + -e IWYU_ROOT="/local-code" \ + --rm=true \ + -v "${REPO_ROOT}":/local-code \ + -v "${HOME/.cache/bazel}":"${HOME/.cache/bazel}" \ + --user "$(id -u):$(id -g)" \ + -t grpc_iwyu /iwyu.sh "$@" + diff --git a/tools/dockerfile/grpc_iwyu/Dockerfile b/tools/dockerfile/grpc_iwyu/Dockerfile new file mode 100644 index 00000000000..c6b9a75b3b6 --- /dev/null +++ b/tools/dockerfile/grpc_iwyu/Dockerfile @@ -0,0 +1,33 @@ +# Copyright 2021 gRPC authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +FROM debian:bullseye + +RUN apt-get update && apt-get install -y clang-11 llvm-11-dev libclang-11-dev clang-format-11 jq git cmake python + +ENV CLANG_FORMAT=clang-format-11 + +RUN git clone https://github.com/include-what-you-use/include-what-you-use.git /iwyu +# latest commit on the clang 11 branch +RUN cd /iwyu && git checkout 5db414ac448004fe019871c977905cb7c2cff23f + +RUN mkdir /iwyu_build && cd /iwyu_build && cmake -G "Unix Makefiles" -DCMAKE_PREFIX_PATH=/usr/lib/llvm-11 /iwyu && make + +ADD iwyu.sh / + +# When running locally, we'll be impersonating the current user, so we need +# to make the script runnable by everyone. +RUN chmod a+rx /iwyu.sh + +CMD ["echo 'Run with tools/distrib/iwyu.sh'"] diff --git a/tools/dockerfile/grpc_iwyu/iwyu.sh b/tools/dockerfile/grpc_iwyu/iwyu.sh new file mode 100755 index 00000000000..05a8804d83a --- /dev/null +++ b/tools/dockerfile/grpc_iwyu/iwyu.sh @@ -0,0 +1,43 @@ +#!/bin/sh +# Copyright 2017 gRPC authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +set -ex + +cd ${IWYU_ROOT} + +export PATH=${PATH}:/iwyu_build/bin + +cat compile_commands.json | sed "s,\"file\": \",\"file\": \"${IWYU_ROOT}/,g" > compile_commands_for_iwyu.json + +# figure out which files to include +cat compile_commands.json | jq -r '.[].file' \ + | grep -E "^src/core/lib/promise/" \ + | grep -v -E "/upb-generated/|/upbdefs-generated/" \ + | sort \ + | tee iwyu_files.txt + +# run iwyu, filtering out changes to port_platform.h +xargs -a iwyu_files.txt /iwyu/iwyu_tool.py -p compile_commands_for_iwyu.json -j 16 \ + | grep -v -E "port_platform.h" \ + | tee iwyu.out + +# apply the suggested changes +/iwyu/fix_includes.py --nocomments < iwyu.out || true + +# reformat sources, since iwyu gets this wrong +xargs -a iwyu_files.txt $CLANG_FORMAT -i + +# TODO(ctiller): expand this to match the clang-tidy directories: +# | grep -E "(^include/|^src/core/|^src/cpp/|^test/core/|^test/cpp/)"