Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get include-what-you-use work for Envoy #12853

Open
foreseeable opened this issue Aug 27, 2020 · 2 comments
Open

Get include-what-you-use work for Envoy #12853

foreseeable opened this issue Aug 27, 2020 · 2 comments
Labels

Comments

@foreseeable
Copy link
Contributor

Title: Get include-what-you-use work for Envoy

Description:
There are lots of include-what-you-use violations in Envoy:

  1. over-inclusion: including too many unused headers slows down compilation.
  2. transitive-inclusion: some of the missing explicit includes will make further development difficult.

include-what-you-use is powerful to address those violations and fix them.

@htuch mentioned at #10917 that we need to get it work for Envoy.

I tried on my own development environment and it works.

Here are steps to get it work:

  1. follow instructions at [Instructions for developers] (https://github.com/include-what-you-use/include-what-you-use/blob/master/README.md) to build iwyu
  2. generate compilation database by
tools/gen_compilation_database.py --include_headers
  1. replace -stdlib=libc++ -stdlib=libstdc++ in the generated json file compile_commands.json

  2. run iwyu_tool.py -p . at Envoy root directory. (Please refer to section "Using with a compilation database" from [Instructions for developers] (https://github.com/include-what-you-use/include-what-you-use/blob/master/README.md) for details)

The output shall look like this

...
source/common/grpc/google_async_client_impl.cc should add these lines:
#include <grpc/impl/codegen/gpr_types.h>            // for GPR_CLOCK_REALTIME
#include <grpc/support/time.h>                      // for gpr_inf_future
#include <grpcpp/channel.h>                         // for Channel
#include <grpcpp/impl/codegen/async_stream_impl.h>  // for ClientAsyncReader...
#include <grpcpp/impl/codegen/call_op_set.h>        // for WriteOptions
#include <grpcpp/impl/codegen/string_ref.h>         // for string_ref
#include <chrono>                                   // for duration
#include <tuple>                                    // for tie, tuple
#include "absl/strings/match.h"                     // for EndsWith
#include "common/grpc/stat_names.h"                 // for StatNames
#include "common/http/header_map_impl.h"            // for RequestHeaderMapImpl
#include "common/protobuf/utility.h"                // for PROTOBUF_GET_WRAP...
#include "common/stats/symbol_table_impl.h"         // for StatName
#include "envoy/api/api.h"                          // for Api
#include "envoy/common/time.h"                      // for TimeSource
#include "envoy/stats/stats.h"                      // for Counter

source/common/grpc/google_async_client_impl.cc should remove these lines:
- #include "common/config/datasource.h"  // lines 9-9
- #include "common/grpc/common.h"  // lines 10-10
- #include "common/grpc/google_grpc_creds_impl.h"  // lines 11-11
- #include "grpcpp/support/proto_buffer_reader.h"  // lines 15-15

The full include-list for source/common/grpc/google_async_client_impl.cc:
#include "common/grpc/google_async_client_impl.h"
#include <grpc/impl/codegen/gpr_types.h>            // for GPR_CLOCK_REALTIME
#include <grpc/support/time.h>                      // for gpr_inf_future
#include <grpcpp/channel.h>                         // for Channel
#include <grpcpp/impl/codegen/async_stream_impl.h>  // for ClientAsyncReader...
#include <grpcpp/impl/codegen/call_op_set.h>        // for WriteOptions
#include <grpcpp/impl/codegen/string_ref.h>         // for string_ref
...

It will be useful if we can apply the iwyu check to every pull request to check iwyu violations.

@mattklein123
Copy link
Member

This would be awesome!

@chadr123
Copy link
Contributor

I think if it is possible to remove the unused includes automatically, it would be great if it works with 'fix_format'.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants