Skip to content

Commit

Permalink
build: use clang-6.0. (envoyproxy#4168)
Browse files Browse the repository at this point in the history
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
  • Loading branch information
PiotrSikora authored and Matt Klein committed Aug 16, 2018
1 parent 01f403e commit 05c0d52
Show file tree
Hide file tree
Showing 198 changed files with 265 additions and 281 deletions.
2 changes: 1 addition & 1 deletion .circleci/config.yml
@@ -1,6 +1,6 @@
references:
envoy-build-image: &envoy-build-image
envoyproxy/envoy-build:1ef23d481a4701ad4a414d1ef98036bd2ed322e7
envoyproxy/envoy-build:e994c1c0b1cdc9a9470cff728311ff7c995685e6

version: 2
jobs:
Expand Down
4 changes: 2 additions & 2 deletions bazel/README.md
Expand Up @@ -33,7 +33,7 @@ On Ubuntu, run the following commands:
apt-get install libtool
apt-get install cmake
apt-get install realpath
apt-get install clang-format-5.0
apt-get install clang-format-6.0
apt-get install automake
apt-get install ninja-build
apt-get install curl
Expand Down Expand Up @@ -231,7 +231,7 @@ bazel test -c dbg --config=asan //test/...

The ASAN failure stack traces include line numbers as a result of running ASAN with a `dbg` build above.

If you have clang-5.0, additional checks are provided with:
If you have clang-5.0 or newer, additional checks are provided with:

```
bazel test -c dbg --config=clang-asan //test/...
Expand Down
12 changes: 6 additions & 6 deletions ci/README.md
Expand Up @@ -26,7 +26,7 @@ Currently there are three build images:
* `envoyproxy/envoy-build-ubuntu` &mdash; based on Ubuntu 16.04 (Xenial) which uses the GCC 5.4 compiler.
* `envoyproxy/envoy-build-centos` &mdash; based on CentOS 7 which uses the GCC 5.3.1 compiler (devtoolset-4).

We also install and use the clang-5.0 compiler for some sanitizing runs.
We also install and use the clang-6.0 compiler for some sanitizing runs.

# Building and running tests as a developer

Expand Down Expand Up @@ -77,8 +77,8 @@ The build artifact can be found in `/tmp/envoy-docker-build/envoy/source/exe/env

The `./ci/run_envoy_docker.sh './ci/do_ci.sh <TARGET>'` targets are:

* `bazel.api` &mdash; build and run API tests under `-c fastbuild` with clang-5.0.
* `bazel.asan` &mdash; build and run tests under `-c dbg --config=clang-asan` with clang-5.0.
* `bazel.api` &mdash; build and run API tests under `-c fastbuild` with clang-6.0.
* `bazel.asan` &mdash; build and run tests under `-c dbg --config=clang-asan` with clang-6.0.
* `bazel.debug` &mdash; build Envoy static binary and run tests under `-c dbg`.
* `bazel.debug.server_only` &mdash; build Envoy static binary under `-c dbg`.
* `bazel.dev` &mdash; build Envoy static binary and run tests under `-c fastbuild` with gcc.
Expand All @@ -87,9 +87,9 @@ The `./ci/run_envoy_docker.sh './ci/do_ci.sh <TARGET>'` targets are:
* `bazel.release.server_only` &mdash; build Envoy static binary under `-c opt` with gcc.
* `bazel.coverage` &mdash; build and run tests under `-c dbg` with gcc, generating coverage information in `$ENVOY_DOCKER_BUILD_DIR/envoy/generated/coverage/coverage.html`.
* `bazel.coverity` &mdash; build Envoy static binary and run Coverity Scan static analysis.
* `bazel.tsan` &mdash; build and run tests under `-c dbg --config=clang-tsan` with clang-5.0.
* `check_format`&mdash; run `clang-format` 5.0 and `buildifier` on entire source tree.
* `fix_format`&mdash; run and enforce `clang-format` 5.0 and `buildifier` on entire source tree.
* `bazel.tsan` &mdash; build and run tests under `-c dbg --config=clang-tsan` with clang-6.0.
* `check_format`&mdash; run `clang-format-6.0` and `buildifier` on entire source tree.
* `fix_format`&mdash; run and enforce `clang-format-6.0` and `buildifier` on entire source tree.
* `docs`&mdash; build documentation tree in `generated/docs`.

# Testing changes to the build image as a developer
Expand Down
6 changes: 3 additions & 3 deletions ci/build_setup.sh
Expand Up @@ -17,9 +17,9 @@ function setup_gcc_toolchain() {
}

function setup_clang_toolchain() {
export CC=clang-5.0
export CXX=clang++-5.0
export ASAN_SYMBOLIZER_PATH=/usr/lib/llvm-5.0/bin/llvm-symbolizer
export CC=clang-6.0
export CXX=clang++-6.0
export ASAN_SYMBOLIZER_PATH=/usr/lib/llvm-6.0/bin/llvm-symbolizer
echo "$CC/$CXX toolchain configured"
}

Expand Down
2 changes: 1 addition & 1 deletion include/envoy/common/pure.h
Expand Up @@ -5,4 +5,4 @@ namespace Envoy {
* Friendly name for a pure virtual routine.
*/
#define PURE = 0
} // Envoy
} // namespace Envoy
2 changes: 1 addition & 1 deletion include/envoy/stats/stats_macros.h
Expand Up @@ -40,4 +40,4 @@ namespace Envoy {
#define POOL_COUNTER(POOL) POOL_COUNTER_PREFIX(POOL, "")
#define POOL_GAUGE(POOL) POOL_GAUGE_PREFIX(POOL, "")
#define POOL_HISTOGRAM(POOL) POOL_HISTOGRAM_PREFIX(POOL, "")
} // Envoy
} // namespace Envoy
2 changes: 1 addition & 1 deletion include/envoy/upstream/host_description.h
Expand Up @@ -112,5 +112,5 @@ class HostDescription {

typedef std::shared_ptr<const HostDescription> HostDescriptionConstSharedPtr;

} // Upstream
} // namespace Upstream
} // namespace Envoy
2 changes: 1 addition & 1 deletion source/common/common/assert.h
Expand Up @@ -66,4 +66,4 @@ namespace Envoy {
// after a switch (some_enum) with all enum values included in the cases. The macro name includes
// "GCOVR_EXCL_LINE" to exclude the macro's usage from code coverage reports.
#define NOT_REACHED_GCOVR_EXCL_LINE PANIC("not reached")
} // Envoy
} // namespace Envoy
2 changes: 1 addition & 1 deletion source/common/common/compiler_requirements.h
Expand Up @@ -17,4 +17,4 @@ namespace Envoy {
"ENVOY_IGNORE_GLIBCXX_USE_CXX11_ABI_ERROR=1 in your build."
#endif

} // Envoy
} // namespace Envoy
2 changes: 1 addition & 1 deletion source/common/common/logger.cc
Expand Up @@ -88,5 +88,5 @@ void Registry::setLogFormat(const std::string& log_format) {
}
}

} // Logger
} // namespace Logger
} // namespace Envoy
2 changes: 1 addition & 1 deletion source/common/common/logger.h
Expand Up @@ -240,7 +240,7 @@ template <Id id> class Loggable {
}
};

} // Logger
} // namespace Logger

// Convert the line macro to a string literal for concatenation in log macros.
#define DO_STRINGIZE(x) STRINGIZE(x)
Expand Down
2 changes: 1 addition & 1 deletion source/common/common/macros.h
Expand Up @@ -44,4 +44,4 @@ namespace Envoy {
#define FALLTHRU
#endif

} // Envoy
} // namespace Envoy
5 changes: 3 additions & 2 deletions source/common/config/filter_json.cc
Expand Up @@ -161,8 +161,9 @@ void FilterJson::translateHttpConnectionManager(
json_filter->getString("name")));
JSON_UTIL_SET_STRING(*json_filter, *filter->mutable_deprecated_v1(), type);

const std::string deprecated_config = "{\"deprecated_v1\": true, \"value\": " +
json_filter->getObject("config")->asJsonString() + "}";
const std::string deprecated_config =
"{\"deprecated_v1\": true, \"value\": " + json_filter->getObject("config")->asJsonString() +
"}";

const auto status =
Protobuf::util::JsonStringToMessage(deprecated_config, filter->mutable_config());
Expand Down
5 changes: 3 additions & 2 deletions source/common/config/lds_json.cc
Expand Up @@ -41,8 +41,9 @@ void LdsJson::translateListener(const Json::Object& json_listener,
json_filter->getString("name")));
JSON_UTIL_SET_STRING(*json_filter, *filter->mutable_deprecated_v1(), type);

const std::string json_config = "{\"deprecated_v1\": true, \"value\": " +
json_filter->getObject("config")->asJsonString() + "}";
const std::string json_config =
"{\"deprecated_v1\": true, \"value\": " + json_filter->getObject("config")->asJsonString() +
"}";

const auto status = Protobuf::util::JsonStringToMessage(json_config, filter->mutable_config());
// JSON schema has already validated that this is a valid JSON object.
Expand Down
10 changes: 5 additions & 5 deletions source/common/filesystem/filesystem_impl.h
Expand Up @@ -132,10 +132,10 @@ class FileImpl : public File {
Thread::CondVar flush_event_;
std::atomic<bool> flush_thread_exit_{};
std::atomic<bool> reopen_file_{};
Buffer::OwnedImpl flush_buffer_
GUARDED_BY(write_lock_); // This buffer is used by multiple threads. It gets filled and
// then flushed either when max size is reached or when a timer
// fires.
Buffer::OwnedImpl
flush_buffer_ GUARDED_BY(write_lock_); // This buffer is used by multiple threads. It gets
// filled and then flushed either when max size is
// reached or when a timer fires.
// TODO(jmarantz): this should be GUARDED_BY(flush_lock_) but the analysis cannot poke through
// the std::make_unique assignment. I do not believe it's possible to annotate this properly now
// due to limitations in the clang thread annotation analysis.
Expand All @@ -153,4 +153,4 @@ class FileImpl : public File {
};

} // namespace Filesystem
} // Envoy
} // namespace Envoy
2 changes: 1 addition & 1 deletion source/common/http/conn_manager_config.h
Expand Up @@ -293,5 +293,5 @@ class ConnectionManagerConfig {
*/
virtual const Http::Http1Settings& http1Settings() const PURE;
};
}
} // namespace Http
} // namespace Envoy
2 changes: 1 addition & 1 deletion source/common/http/header_map_impl.h
Expand Up @@ -209,5 +209,5 @@ class HeaderMapImpl : public HeaderMap, NonCopyable {

typedef std::unique_ptr<HeaderMapImpl> HeaderMapImplPtr;

} // Http
} // namespace Http
} // namespace Envoy
1 change: 0 additions & 1 deletion source/common/http/http2/codec_impl.cc
Expand Up @@ -673,7 +673,6 @@ ConnectionImpl::Http2Callbacks::Http2Callbacks() {
callbacks_,
[](nghttp2_session*, const nghttp2_frame* frame, const uint8_t* raw_name, size_t name_length,
const uint8_t* raw_value, size_t value_length, uint8_t, void* user_data) -> int {

// TODO PERF: Can reference count here to avoid copies.
HeaderString name;
name.setCopy(reinterpret_cast<const char*>(raw_name), name_length);
Expand Down
2 changes: 1 addition & 1 deletion source/common/http/user_agent.h
Expand Up @@ -68,4 +68,4 @@ class UserAgent {
};

} // namespace Http
} // Envoy
} // namespace Envoy
16 changes: 8 additions & 8 deletions source/common/router/router.cc
Expand Up @@ -222,14 +222,14 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool e
direct_response->rewritePathHeader(headers, !config_.suppress_envoy_headers_);
callbacks_->sendLocalReply(
direct_response->responseCode(), direct_response->responseBody(),
[ this, direct_response, &request_headers = headers ](Http::HeaderMap & response_headers)
->void {
const auto new_path = direct_response->newPath(request_headers);
if (!new_path.empty()) {
response_headers.addReferenceKey(Http::Headers::get().Location, new_path);
}
direct_response->finalizeResponseHeaders(response_headers, callbacks_->requestInfo());
});
[this, direct_response,
&request_headers = headers](Http::HeaderMap& response_headers) -> void {
const auto new_path = direct_response->newPath(request_headers);
if (!new_path.empty()) {
response_headers.addReferenceKey(Http::Headers::get().Location, new_path);
}
direct_response->finalizeResponseHeaders(response_headers, callbacks_->requestInfo());
});
return Http::FilterHeadersStatus::StopIteration;
}

Expand Down
2 changes: 1 addition & 1 deletion source/common/runtime/runtime_impl.cc
Expand Up @@ -315,7 +315,7 @@ std::unique_ptr<SnapshotImpl> LoaderImpl::createNewSnapshot() {

void LoaderImpl::loadNewSnapshot() {
ThreadLocal::ThreadLocalObjectSharedPtr ptr = createNewSnapshot();
tls_->set([ptr = std::move(ptr)](Event::Dispatcher&)->ThreadLocal::ThreadLocalObjectSharedPtr {
tls_->set([ptr = std::move(ptr)](Event::Dispatcher&) -> ThreadLocal::ThreadLocalObjectSharedPtr {
return ptr;
});
}
Expand Down
2 changes: 1 addition & 1 deletion source/common/stats/thread_local_store.cc
Expand Up @@ -144,7 +144,7 @@ void ThreadLocalStoreImpl::releaseScopeCrossThread(ScopeImpl* scope) {
// cache flush operation.
if (!shutting_down_ && main_thread_dispatcher_) {
main_thread_dispatcher_->post(
[ this, scope_id = scope->scope_id_ ]()->void { clearScopeFromCaches(scope_id); });
[this, scope_id = scope->scope_id_]() -> void { clearScopeFromCaches(scope_id); });
}
}

Expand Down
15 changes: 6 additions & 9 deletions source/common/tcp_proxy/tcp_proxy.cc
Expand Up @@ -466,17 +466,14 @@ void Filter::onUpstreamEvent(Network::ConnectionEvent event) {
// The idle_timer_ can be moved to a Drainer, so related callbacks call into
// the UpstreamCallbacks, which has the same lifetime as the timer, and can dispatch
// the call to either TcpProxy or to Drainer, depending on the current state.
idle_timer_ =
read_callbacks_->connection().dispatcher().createTimer([upstream_callbacks =
upstream_callbacks_]() {
upstream_callbacks->onIdleTimeout();
});
idle_timer_ = read_callbacks_->connection().dispatcher().createTimer(
[upstream_callbacks = upstream_callbacks_]() { upstream_callbacks->onIdleTimeout(); });
resetIdleTimer();
read_callbacks_->connection().addBytesSentCallback([this](uint64_t) { resetIdleTimer(); });
upstream_conn_data_->connection().addBytesSentCallback([upstream_callbacks =
upstream_callbacks_](uint64_t) {
upstream_callbacks->onBytesSent();
});
upstream_conn_data_->connection().addBytesSentCallback(
[upstream_callbacks = upstream_callbacks_](uint64_t) {
upstream_callbacks->onBytesSent();
});
}
}
}
Expand Down
57 changes: 26 additions & 31 deletions source/common/upstream/cluster_manager_impl.cc
Expand Up @@ -521,28 +521,24 @@ bool ClusterManagerImpl::addOrUpdateCluster(const envoy::api::v2::Cluster& clust
}

void ClusterManagerImpl::createOrUpdateThreadLocalCluster(ClusterData& cluster) {
tls_->runOnAllThreads(
[
this, new_cluster = cluster.cluster_->info(),
thread_aware_lb_factory = cluster.loadBalancerFactory()
]()
->void {
ThreadLocalClusterManagerImpl& cluster_manager =
tls_->getTyped<ThreadLocalClusterManagerImpl>();

if (cluster_manager.thread_local_clusters_.count(new_cluster->name()) > 0) {
ENVOY_LOG(debug, "updating TLS cluster {}", new_cluster->name());
} else {
ENVOY_LOG(debug, "adding TLS cluster {}", new_cluster->name());
}

auto thread_local_cluster = new ThreadLocalClusterManagerImpl::ClusterEntry(
cluster_manager, new_cluster, thread_aware_lb_factory);
cluster_manager.thread_local_clusters_[new_cluster->name()].reset(thread_local_cluster);
for (auto& cb : cluster_manager.update_callbacks_) {
cb->onClusterAddOrUpdate(*thread_local_cluster);
}
});
tls_->runOnAllThreads([this, new_cluster = cluster.cluster_->info(),
thread_aware_lb_factory = cluster.loadBalancerFactory()]() -> void {
ThreadLocalClusterManagerImpl& cluster_manager =
tls_->getTyped<ThreadLocalClusterManagerImpl>();

if (cluster_manager.thread_local_clusters_.count(new_cluster->name()) > 0) {
ENVOY_LOG(debug, "updating TLS cluster {}", new_cluster->name());
} else {
ENVOY_LOG(debug, "adding TLS cluster {}", new_cluster->name());
}

auto thread_local_cluster = new ThreadLocalClusterManagerImpl::ClusterEntry(
cluster_manager, new_cluster, thread_aware_lb_factory);
cluster_manager.thread_local_clusters_[new_cluster->name()].reset(thread_local_cluster);
for (auto& cb : cluster_manager.update_callbacks_) {
cb->onClusterAddOrUpdate(*thread_local_cluster);
}
});
}

bool ClusterManagerImpl::removeCluster(const std::string& cluster_name) {
Expand Down Expand Up @@ -693,15 +689,14 @@ void ClusterManagerImpl::postThreadLocalClusterUpdate(const Cluster& cluster, ui
HostsPerLocalityConstSharedPtr healthy_hosts_per_locality_copy =
host_set->healthyHostsPerLocality().clone();

tls_->runOnAllThreads([
this, name = cluster.info()->name(), priority, hosts_copy, healthy_hosts_copy,
hosts_per_locality_copy, healthy_hosts_per_locality_copy,
locality_weights = host_set->localityWeights(), hosts_added, hosts_removed
]() {
ThreadLocalClusterManagerImpl::updateClusterMembership(
name, priority, hosts_copy, healthy_hosts_copy, hosts_per_locality_copy,
healthy_hosts_per_locality_copy, locality_weights, hosts_added, hosts_removed, *tls_);
});
tls_->runOnAllThreads(
[this, name = cluster.info()->name(), priority, hosts_copy, healthy_hosts_copy,
hosts_per_locality_copy, healthy_hosts_per_locality_copy,
locality_weights = host_set->localityWeights(), hosts_added, hosts_removed]() {
ThreadLocalClusterManagerImpl::updateClusterMembership(
name, priority, hosts_copy, healthy_hosts_copy, hosts_per_locality_copy,
healthy_hosts_per_locality_copy, locality_weights, hosts_added, hosts_removed, *tls_);
});
}

void ClusterManagerImpl::postThreadLocalHealthFailure(const HostSharedPtr& host) {
Expand Down
2 changes: 1 addition & 1 deletion source/extensions/access_loggers/http_grpc/config.cc
Expand Up @@ -29,7 +29,7 @@ HttpGrpcAccessLogFactory::createAccessLogInstance(const Protobuf::Message& confi
std::shared_ptr<GrpcAccessLogStreamer> grpc_access_log_streamer =
context.singletonManager().getTyped<GrpcAccessLogStreamer>(
SINGLETON_MANAGER_REGISTERED_NAME(grpc_access_log_streamer),
[&context, grpc_service = proto_config.common_config().grpc_service() ] {
[&context, grpc_service = proto_config.common_config().grpc_service()] {
return std::make_shared<GrpcAccessLogStreamerImpl>(
context.clusterManager().grpcAsyncClientManager().factoryForGrpcService(
grpc_service, context.scope(), false),
Expand Down
2 changes: 1 addition & 1 deletion source/extensions/filters/common/lua/lua.h
Expand Up @@ -402,7 +402,7 @@ class LuaException : public EnvoyException {
public:
using EnvoyException::EnvoyException;
};
}
} // namespace Lua
} // namespace Common
} // namespace Filters
} // namespace Extensions
Expand Down
12 changes: 6 additions & 6 deletions source/extensions/filters/http/ext_authz/config.cc
Expand Up @@ -28,10 +28,10 @@ Http::FilterFactoryCb ExtAuthzFilterConfig::createFilterFactoryFromProtoTyped(
if (proto_config.has_http_service()) {
const uint32_t timeout_ms = PROTOBUF_GET_MS_OR_DEFAULT(proto_config.http_service().server_uri(),
timeout, DefaultTimeout);
return [
filter_config, timeout_ms, cluster_name = proto_config.http_service().server_uri().cluster(),
path_prefix = proto_config.http_service().path_prefix()
](Http::FilterChainFactoryCallbacks & callbacks) {
return [filter_config, timeout_ms,
cluster_name = proto_config.http_service().server_uri().cluster(),
path_prefix = proto_config.http_service().path_prefix()](
Http::FilterChainFactoryCallbacks& callbacks) {
auto client = std::make_unique<Filters::Common::ExtAuthz::RawHttpClientImpl>(
cluster_name, filter_config->cm(), std::chrono::milliseconds(timeout_ms), path_prefix,
filter_config->allowedAuthorizationHeaders(), filter_config->allowedRequestHeaders());
Expand All @@ -43,8 +43,8 @@ Http::FilterFactoryCb ExtAuthzFilterConfig::createFilterFactoryFromProtoTyped(
const uint32_t timeout_ms =
PROTOBUF_GET_MS_OR_DEFAULT(proto_config.grpc_service(), timeout, DefaultTimeout);

return [ grpc_service = proto_config.grpc_service(), &context, filter_config,
timeout_ms ](Http::FilterChainFactoryCallbacks & callbacks) {
return [grpc_service = proto_config.grpc_service(), &context, filter_config,
timeout_ms](Http::FilterChainFactoryCallbacks& callbacks) {
const auto async_client_factory =
context.clusterManager().grpcAsyncClientManager().factoryForGrpcService(
grpc_service, context.scope(), true);
Expand Down

0 comments on commit 05c0d52

Please sign in to comment.