From 5e5f714065f5d3414f00bad0c3adf36e49746de4 Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Wed, 25 Dec 2019 15:30:49 -0500 Subject: [PATCH 1/3] build: add API_NO_BOOST* annotations. These allow specific types, expressions and files to be excluded from API boosting via annotation. API_NO_BOOST_FILE (anywhere in the file) will skip boosting of the file, API_NO_BOOST() wrapped around an expression or type will bypass boosting of that text. The idea is that there are some types that we do not ever want to upgrade as they are relevant to the cross-version wire impedance matching, or testing v2 compat when the tree is v3alpha. The sites identified in this PR were taken from my WiP in which I have the entire tree boosted and tests passing. It's possible we might need to tune some more later, but this should go a reasonable way towards reducing the amount of manual fixups required after boosting to get tests passing again. Risk level: Low (macros are nops) Testing: bazel test //test/..., new api_booster golden tests. Signed-off-by: Harvey Tuch --- source/common/config/BUILD | 5 ++++ source/common/config/api_version.h | 7 ++++++ source/common/config/type_to_endpoint.cc | 2 ++ source/common/router/BUILD | 3 +++ source/common/router/rds_impl.cc | 4 ++- source/common/router/scoped_rds.cc | 6 +++-- source/common/router/vhds.cc | 4 ++- source/common/runtime/BUILD | 1 + source/common/runtime/runtime_impl.cc | 4 ++- source/common/secret/BUILD | 1 + source/common/secret/sds_api.cc | 6 +++-- source/common/upstream/BUILD | 2 ++ source/common/upstream/cds_api_impl.cc | 4 ++- source/common/upstream/eds.cc | 3 ++- source/server/BUILD | 1 + source/server/lds_api.cc | 4 ++- test/common/config/BUILD | 3 +++ .../config/delta_subscription_impl_test.cc | 3 ++- test/common/config/grpc_mux_impl_test.cc | 3 ++- .../config/grpc_subscription_test_harness.h | 3 ++- test/common/router/BUILD | 1 + test/common/router/scoped_rds_test.cc | 14 ++++++----- test/integration/BUILD | 5 ++++ test/integration/header_integration_test.cc | 9 ++++--- test/integration/integration.cc | 7 +++--- test/integration/integration.h | 5 ++-- .../scoped_rds_integration_test.cc | 7 +++--- .../sds_dynamic_integration_test.cc | 3 ++- .../integration/tcp_proxy_integration_test.cc | 25 ++++++++++--------- tools/api_boost/api_boost.py | 7 ++++++ tools/api_boost/api_boost_test.py | 1 + tools/api_boost/testdata/BUILD | 6 +++++ tools/api_boost/testdata/decl_ref_expr.cc | 6 +++++ .../api_boost/testdata/decl_ref_expr.cc.gold | 6 +++++ tools/api_boost/testdata/no_boost_file.cc | 12 +++++++++ .../api_boost/testdata/no_boost_file.cc.gold | 12 +++++++++ tools/clang_tools/api_booster/main.cc | 16 +++++++++--- 37 files changed, 164 insertions(+), 47 deletions(-) create mode 100644 source/common/config/api_version.h create mode 100644 tools/api_boost/testdata/no_boost_file.cc create mode 100644 tools/api_boost/testdata/no_boost_file.cc.gold diff --git a/source/common/config/BUILD b/source/common/config/BUILD index c8bbe5a647813..9b61f27df4ca7 100644 --- a/source/common/config/BUILD +++ b/source/common/config/BUILD @@ -20,6 +20,11 @@ envoy_cc_library( ], ) +envoy_cc_library( + name = "api_version_lib", + hdrs = ["api_version.h"], +) + envoy_cc_library( name = "config_provider_lib", srcs = ["config_provider_impl.cc"], diff --git a/source/common/config/api_version.h b/source/common/config/api_version.h new file mode 100644 index 0000000000000..fddef009aec0e --- /dev/null +++ b/source/common/config/api_version.h @@ -0,0 +1,7 @@ +#pragma once + +// Use this to force a specific version of a given config proto, preventing API +// boosting from modifying it. E.g. API_NO_BOOST(envoy::api::v2::Cluster). +#define API_NO_BOOST(x) x + +namespace Envoy {} diff --git a/source/common/config/type_to_endpoint.cc b/source/common/config/type_to_endpoint.cc index e1b0170f7a2f1..15e95d6d47eb6 100644 --- a/source/common/config/type_to_endpoint.cc +++ b/source/common/config/type_to_endpoint.cc @@ -11,6 +11,8 @@ #include "common/grpc/common.h" +// API_NO_BOOST_FILE + namespace Envoy { namespace Config { diff --git a/source/common/router/BUILD b/source/common/router/BUILD index 1034f252c0395..4495396cfa43b 100644 --- a/source/common/router/BUILD +++ b/source/common/router/BUILD @@ -131,6 +131,7 @@ envoy_cc_library( "//include/envoy/thread_local:thread_local_interface", "//source/common/common:assert_lib", "//source/common/common:minimal_logger_lib", + "//source/common/config:api_version_lib", "//source/common/config:utility_lib", "//source/common/init:target_lib", "//source/common/protobuf:utility_lib", @@ -160,6 +161,7 @@ envoy_cc_library( "//source/common/common:callback_impl_lib", "//source/common/common:cleanup_lib", "//source/common/common:minimal_logger_lib", + "//source/common/config:api_version_lib", "//source/common/config:subscription_factory_lib", "//source/common/config:utility_lib", "//source/common/init:manager_lib", @@ -205,6 +207,7 @@ envoy_cc_library( "//source/common/common:assert_lib", "//source/common/common:cleanup_lib", "//source/common/common:minimal_logger_lib", + "//source/common/config:api_version_lib", "//source/common/config:config_provider_lib", "//source/common/init:manager_lib", "//source/common/init:watcher_lib", diff --git a/source/common/router/rds_impl.cc b/source/common/router/rds_impl.cc index 98ef892517924..96c9952eac170 100644 --- a/source/common/router/rds_impl.cc +++ b/source/common/router/rds_impl.cc @@ -13,6 +13,7 @@ #include "common/common/assert.h" #include "common/common/fmt.h" +#include "common/config/api_version.h" #include "common/config/utility.h" #include "common/protobuf/utility.h" #include "common/router/config_impl.h" @@ -73,7 +74,8 @@ RdsRouteConfigSubscription::RdsRouteConfigSubscription( subscription_ = factory_context.clusterManager().subscriptionFactory().subscriptionFromConfigSource( rds.config_source(), - Grpc::Common::typeUrl(envoy::api::v2::RouteConfiguration().GetDescriptor()->full_name()), + Grpc::Common::typeUrl( + API_NO_BOOST(envoy::api::v2::RouteConfiguration)().GetDescriptor()->full_name()), *scope_, *this); config_update_info_ = std::make_unique(factory_context.timeSource(), validator); diff --git a/source/common/router/scoped_rds.cc b/source/common/router/scoped_rds.cc index 83bb0c0e411aa..ec1564fe802d1 100644 --- a/source/common/router/scoped_rds.cc +++ b/source/common/router/scoped_rds.cc @@ -13,6 +13,7 @@ #include "common/common/cleanup.h" #include "common/common/logger.h" #include "common/common/utility.h" +#include "common/config/api_version.h" #include "common/config/resources.h" #include "common/init/manager_impl.h" #include "common/init/watcher_impl.h" @@ -102,8 +103,9 @@ ScopedRdsConfigSubscription::ScopedRdsConfigSubscription( subscription_ = factory_context.clusterManager().subscriptionFactory().subscriptionFromConfigSource( scoped_rds.scoped_rds_config_source(), - Grpc::Common::typeUrl( - envoy::api::v2::ScopedRouteConfiguration().GetDescriptor()->full_name()), + Grpc::Common::typeUrl(API_NO_BOOST(envoy::api::v2::ScopedRouteConfiguration)() + .GetDescriptor() + ->full_name()), *scope_, *this); initialize([scope_key_builder]() -> Envoy::Config::ConfigProvider::ConfigConstSharedPtr { diff --git a/source/common/router/vhds.cc b/source/common/router/vhds.cc index 11efea2d49015..ac5fa000e24a1 100644 --- a/source/common/router/vhds.cc +++ b/source/common/router/vhds.cc @@ -11,6 +11,7 @@ #include "common/common/assert.h" #include "common/common/fmt.h" +#include "common/config/api_version.h" #include "common/config/utility.h" #include "common/protobuf/utility.h" #include "common/router/config_impl.h" @@ -42,7 +43,8 @@ VhdsSubscription::VhdsSubscription(RouteConfigUpdatePtr& config_update_info, subscription_ = factory_context.clusterManager().subscriptionFactory().subscriptionFromConfigSource( config_update_info_->routeConfiguration().vhds().config_source(), - Grpc::Common::typeUrl(envoy::api::v2::route::VirtualHost().GetDescriptor()->full_name()), + Grpc::Common::typeUrl( + API_NO_BOOST(envoy::api::v2::route::VirtualHost)().GetDescriptor()->full_name()), *scope_, *this); } diff --git a/source/common/runtime/BUILD b/source/common/runtime/BUILD index d92492af69e2b..e2abe6597a7e2 100644 --- a/source/common/runtime/BUILD +++ b/source/common/runtime/BUILD @@ -32,6 +32,7 @@ envoy_cc_library( "//source/common/common:minimal_logger_lib", "//source/common/common:thread_lib", "//source/common/common:utility_lib", + "//source/common/config:api_version_lib", "//source/common/filesystem:directory_lib", "//source/common/grpc:common_lib", "//source/common/init:target_lib", diff --git a/source/common/runtime/runtime_impl.cc b/source/common/runtime/runtime_impl.cc index 7b51f36935b4c..7dc5ed7bf8439 100644 --- a/source/common/runtime/runtime_impl.cc +++ b/source/common/runtime/runtime_impl.cc @@ -17,6 +17,7 @@ #include "common/common/assert.h" #include "common/common/fmt.h" #include "common/common/utility.h" +#include "common/config/api_version.h" #include "common/filesystem/directory.h" #include "common/grpc/common.h" #include "common/protobuf/message_validator_impl.h" @@ -590,7 +591,8 @@ void RtdsSubscription::start() { // instantiated in the server instance. subscription_ = parent_.cm_->subscriptionFactory().subscriptionFromConfigSource( config_source_, - Grpc::Common::typeUrl(envoy::service::discovery::v2::Runtime().GetDescriptor()->full_name()), + Grpc::Common::typeUrl( + API_NO_BOOST(envoy::service::discovery::v2::Runtime)().GetDescriptor()->full_name()), store_, *this); subscription_->start({resource_name_}); } diff --git a/source/common/secret/BUILD b/source/common/secret/BUILD index db35adccc31a2..45b4f11f5d398 100644 --- a/source/common/secret/BUILD +++ b/source/common/secret/BUILD @@ -53,6 +53,7 @@ envoy_cc_library( "//include/envoy/stats:stats_interface", "//source/common/common:callback_impl_lib", "//source/common/common:cleanup_lib", + "//source/common/config:api_version_lib", "//source/common/config:resources_lib", "//source/common/config:utility_lib", "//source/common/init:target_lib", diff --git a/source/common/secret/sds_api.cc b/source/common/secret/sds_api.cc index 53d495f9d84a6..8ecea2b50bf16 100644 --- a/source/common/secret/sds_api.cc +++ b/source/common/secret/sds_api.cc @@ -7,6 +7,7 @@ #include "envoy/api/v2/core/config_source.pb.h" #include "envoy/api/v2/discovery.pb.h" +#include "common/config/api_version.h" #include "common/config/resources.h" #include "common/protobuf/utility.h" @@ -81,8 +82,9 @@ void SdsApi::validateUpdateSize(int num_resources) { void SdsApi::initialize() { subscription_ = subscription_factory_.subscriptionFromConfigSource( sds_config_, - Grpc::Common::typeUrl(envoy::api::v2::auth::Secret().GetDescriptor()->full_name()), stats_, - *this); + Grpc::Common::typeUrl( + API_NO_BOOST(envoy::api::v2::auth::Secret)().GetDescriptor()->full_name()), + stats_, *this); subscription_->start({sds_config_name_}); } diff --git a/source/common/upstream/BUILD b/source/common/upstream/BUILD index 6993b0d28fa91..ad02bc359511b 100644 --- a/source/common/upstream/BUILD +++ b/source/common/upstream/BUILD @@ -18,6 +18,7 @@ envoy_cc_library( "//include/envoy/local_info:local_info_interface", "//source/common/common:cleanup_lib", "//source/common/common:minimal_logger_lib", + "//source/common/config:api_version_lib", "//source/common/config:resources_lib", "//source/common/config:utility_lib", "//source/common/protobuf:utility_lib", @@ -347,6 +348,7 @@ envoy_cc_library( "//include/envoy/secret:secret_manager_interface", "//include/envoy/upstream:cluster_factory_interface", "//include/envoy/upstream:locality_lib", + "//source/common/config:api_version_lib", "//source/common/config:metadata_lib", "//source/common/config:subscription_factory_lib", "//source/common/config:utility_lib", diff --git a/source/common/upstream/cds_api_impl.cc b/source/common/upstream/cds_api_impl.cc index 26bb6de76f64a..2b9b25db9b85f 100644 --- a/source/common/upstream/cds_api_impl.cc +++ b/source/common/upstream/cds_api_impl.cc @@ -10,6 +10,7 @@ #include "common/common/cleanup.h" #include "common/common/utility.h" +#include "common/config/api_version.h" #include "common/config/resources.h" #include "common/config/utility.h" #include "common/protobuf/utility.h" @@ -30,7 +31,8 @@ CdsApiImpl::CdsApiImpl(const envoy::api::v2::core::ConfigSource& cds_config, Clu : cm_(cm), scope_(scope.createScope("cluster_manager.cds.")), validation_visitor_(validation_visitor) { subscription_ = cm_.subscriptionFactory().subscriptionFromConfigSource( - cds_config, Grpc::Common::typeUrl(envoy::api::v2::Cluster().GetDescriptor()->full_name()), + cds_config, + Grpc::Common::typeUrl(API_NO_BOOST(envoy::api::v2::Cluster)().GetDescriptor()->full_name()), *scope_, *this); } diff --git a/source/common/upstream/eds.cc b/source/common/upstream/eds.cc index 285e9ea158ce1..2c8dd16c19a08 100644 --- a/source/common/upstream/eds.cc +++ b/source/common/upstream/eds.cc @@ -7,6 +7,7 @@ #include "envoy/api/v2/eds.pb.validate.h" #include "common/common/utility.h" +#include "common/config/api_version.h" namespace Envoy { namespace Upstream { @@ -35,7 +36,7 @@ EdsClusterImpl::EdsClusterImpl( factory_context.clusterManager().subscriptionFactory().subscriptionFromConfigSource( eds_config, Grpc::Common::typeUrl( - envoy::api::v2::ClusterLoadAssignment().GetDescriptor()->full_name()), + API_NO_BOOST(envoy::api::v2::ClusterLoadAssignment)().GetDescriptor()->full_name()), info_->statsScope(), *this); } diff --git a/source/server/BUILD b/source/server/BUILD index 5e4cc5a667194..88a73a5f69cee 100644 --- a/source/server/BUILD +++ b/source/server/BUILD @@ -253,6 +253,7 @@ envoy_cc_library( "//include/envoy/init:manager_interface", "//include/envoy/server:listener_manager_interface", "//source/common/common:cleanup_lib", + "//source/common/config:api_version_lib", "//source/common/config:resources_lib", "//source/common/config:utility_lib", "//source/common/init:target_lib", diff --git a/source/server/lds_api.cc b/source/server/lds_api.cc index 3fadc71662bce..df19f72fc6dd1 100644 --- a/source/server/lds_api.cc +++ b/source/server/lds_api.cc @@ -10,6 +10,7 @@ #include "envoy/stats/scope.h" #include "common/common/cleanup.h" +#include "common/config/api_version.h" #include "common/config/resources.h" #include "common/config/utility.h" #include "common/protobuf/utility.h" @@ -27,7 +28,8 @@ LdsApiImpl::LdsApiImpl(const envoy::api::v2::core::ConfigSource& lds_config, init_target_("LDS", [this]() { subscription_->start({}); }), validation_visitor_(validation_visitor) { subscription_ = cm.subscriptionFactory().subscriptionFromConfigSource( - lds_config, Grpc::Common::typeUrl(envoy::api::v2::Listener().GetDescriptor()->full_name()), + lds_config, + Grpc::Common::typeUrl(API_NO_BOOST(envoy::api::v2::Listener)().GetDescriptor()->full_name()), *scope_, *this); init_manager.add(init_target_); } diff --git a/test/common/config/BUILD b/test/common/config/BUILD index 6e3369de52bed..8490bd6923b59 100644 --- a/test/common/config/BUILD +++ b/test/common/config/BUILD @@ -25,6 +25,7 @@ envoy_cc_test( srcs = ["delta_subscription_impl_test.cc"], deps = [ ":delta_subscription_test_harness", + "//source/common/config:api_version_lib", "//source/common/config:delta_subscription_lib", "//source/common/stats:isolated_store_lib", "//test/mocks:common_lib", @@ -89,6 +90,7 @@ envoy_cc_test( name = "grpc_mux_impl_test", srcs = ["grpc_mux_impl_test.cc"], deps = [ + "//source/common/config:api_version_lib", "//source/common/config:grpc_mux_lib", "//source/common/config:protobuf_link_hacks", "//source/common/config:resources_lib", @@ -157,6 +159,7 @@ envoy_cc_test_library( deps = [ ":subscription_test_harness", "//source/common/common:hash_lib", + "//source/common/config:api_version_lib", "//source/common/config:grpc_subscription_lib", "//source/common/config:resources_lib", "//test/mocks/config:config_mocks", diff --git a/test/common/config/delta_subscription_impl_test.cc b/test/common/config/delta_subscription_impl_test.cc index 4a3694d903ff8..8f92ff09bf09b 100644 --- a/test/common/config/delta_subscription_impl_test.cc +++ b/test/common/config/delta_subscription_impl_test.cc @@ -3,6 +3,7 @@ #include "envoy/api/v2/eds.pb.h" #include "common/buffer/zero_copy_input_stream_impl.h" +#include "common/config/api_version.h" #include "test/common/config/delta_subscription_test_harness.h" @@ -109,7 +110,7 @@ TEST_F(DeltaSubscriptionImplTest, PauseQueuesAcks) { // All ACK sendMessage()s will happen upon calling resume(). EXPECT_CALL(async_stream_, sendMessageRaw_(_, _)) .WillRepeatedly(Invoke([this](Buffer::InstancePtr& buffer, bool) { - envoy::api::v2::DeltaDiscoveryRequest message; + API_NO_BOOST(envoy::api::v2::DeltaDiscoveryRequest) message; EXPECT_TRUE(Grpc::Common::parseBufferInstance(std::move(buffer), message)); const std::string nonce = message.response_nonce(); if (!nonce.empty()) { diff --git a/test/common/config/grpc_mux_impl_test.cc b/test/common/config/grpc_mux_impl_test.cc index c9fb873d61581..2cb73cdda08bd 100644 --- a/test/common/config/grpc_mux_impl_test.cc +++ b/test/common/config/grpc_mux_impl_test.cc @@ -4,6 +4,7 @@ #include "envoy/api/v2/eds.pb.h" #include "common/common/empty_string.h" +#include "common/config/api_version.h" #include "common/config/grpc_mux_impl.h" #include "common/config/protobuf_link_hacks.h" #include "common/config/resources.h" @@ -68,7 +69,7 @@ class GrpcMuxImplTestBase : public testing::Test { bool first = false, const std::string& nonce = "", const Protobuf::int32 error_code = Grpc::Status::WellKnownGrpcStatus::Ok, const std::string& error_message = "") { - envoy::api::v2::DiscoveryRequest expected_request; + API_NO_BOOST(envoy::api::v2::DiscoveryRequest) expected_request; if (first) { expected_request.mutable_node()->CopyFrom(local_info_.node()); } diff --git a/test/common/config/grpc_subscription_test_harness.h b/test/common/config/grpc_subscription_test_harness.h index 25dbff0c3fb3c..08ac0165891ec 100644 --- a/test/common/config/grpc_subscription_test_harness.h +++ b/test/common/config/grpc_subscription_test_harness.h @@ -7,6 +7,7 @@ #include "envoy/api/v2/eds.pb.h" #include "common/common/hash.h" +#include "common/config/api_version.h" #include "common/config/grpc_subscription_impl.h" #include "common/config/resources.h" @@ -62,7 +63,7 @@ class GrpcSubscriptionTestHarness : public SubscriptionTestHarness { bool expect_node, const Protobuf::int32 error_code, const std::string& error_message) { UNREFERENCED_PARAMETER(expect_node); - envoy::api::v2::DiscoveryRequest expected_request; + API_NO_BOOST(envoy::api::v2::DiscoveryRequest) expected_request; if (expect_node) { expected_request.mutable_node()->CopyFrom(node_); } diff --git a/test/common/router/BUILD b/test/common/router/BUILD index 66d7228729996..69243a8495361 100644 --- a/test/common/router/BUILD +++ b/test/common/router/BUILD @@ -104,6 +104,7 @@ envoy_cc_test( deps = [ "//include/envoy/config:subscription_interface", "//include/envoy/init:manager_interface", + "//source/common/config:api_version_lib", "//source/common/config:utility_lib", "//source/common/http:message_lib", "//source/common/json:json_loader_lib", diff --git a/test/common/router/scoped_rds_test.cc b/test/common/router/scoped_rds_test.cc index 796e2a795b24e..7d50efa32eeeb 100644 --- a/test/common/router/scoped_rds_test.cc +++ b/test/common/router/scoped_rds_test.cc @@ -11,6 +11,7 @@ #include "envoy/init/manager.h" #include "envoy/stats/scope.h" +#include "common/config/api_version.h" #include "common/config/grpc_mux_impl.h" #include "common/router/scoped_rds.h" @@ -122,12 +123,13 @@ class ScopedRdsTest : public ScopedRoutesTestBase { subscriptionFromConfigSource(_, _, _, _)) .Times(AnyNumber()); // rds subscription - EXPECT_CALL(server_factory_context_.cluster_manager_.subscription_factory_, - subscriptionFromConfigSource( - _, - Eq(Grpc::Common::typeUrl( - envoy::api::v2::RouteConfiguration().GetDescriptor()->full_name())), - _, _)) + EXPECT_CALL( + server_factory_context_.cluster_manager_.subscription_factory_, + subscriptionFromConfigSource( + _, + Eq(Grpc::Common::typeUrl( + API_NO_BOOST(envoy::api::v2::RouteConfiguration)().GetDescriptor()->full_name())), + _, _)) .Times(AnyNumber()) .WillRepeatedly(Invoke([this](const envoy::api::v2::core::ConfigSource&, absl::string_view, Stats::Scope&, diff --git a/test/integration/BUILD b/test/integration/BUILD index 3c3d24948e369..6d01661a7e7cf 100644 --- a/test/integration/BUILD +++ b/test/integration/BUILD @@ -217,6 +217,7 @@ envoy_cc_test( ], deps = [ ":http_integration_lib", + "//source/common/config:api_version_lib", "//source/common/protobuf", "//test/test_common:utility_lib", "@envoy_api//envoy/api/v2:pkg_cc_proto", @@ -482,6 +483,7 @@ envoy_cc_test_library( "//source/common/buffer:zero_copy_input_stream_lib", "//source/common/common:assert_lib", "//source/common/common:minimal_logger_lib", + "//source/common/config:api_version_lib", "//source/common/event:dispatcher_lib", "//source/common/grpc:codec_lib", "//source/common/grpc:common_lib", @@ -769,6 +771,7 @@ envoy_cc_test( ], deps = [ ":http_integration_lib", + "//source/common/config:api_version_lib", "//source/common/config:protobuf_link_hacks", "//source/common/config:resources_lib", "//source/common/event:dispatcher_includes", @@ -801,6 +804,7 @@ envoy_cc_test( ], deps = [ ":integration_lib", + "//source/common/config:api_version_lib", "//source/common/event:dispatcher_includes", "//source/common/event:dispatcher_lib", "//source/common/network:utility_lib", @@ -956,6 +960,7 @@ envoy_cc_test( ], deps = [ ":http_integration_lib", + "//source/common/config:api_version_lib", "//source/common/config:resources_lib", "//source/common/event:dispatcher_includes", "//source/common/event:dispatcher_lib", diff --git a/test/integration/header_integration_test.cc b/test/integration/header_integration_test.cc index 4c1a311e9a3f4..0292571dcf75a 100644 --- a/test/integration/header_integration_test.cc +++ b/test/integration/header_integration_test.cc @@ -6,6 +6,7 @@ #include "envoy/config/filter/http/router/v2/router.pb.h" #include "envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.pb.h" +#include "common/config/api_version.h" #include "common/config/metadata.h" #include "common/config/resources.h" #include "common/http/exception.h" @@ -368,16 +369,16 @@ class HeaderIntegrationTest RELEASE_ASSERT(result, result.message()); eds_stream_->startGrpcStream(); - envoy::api::v2::DiscoveryRequest discovery_request; + API_NO_BOOST(envoy::api::v2::DiscoveryRequest) discovery_request; result = eds_stream_->waitForGrpcMessage(*dispatcher_, discovery_request); RELEASE_ASSERT(result, result.message()); - envoy::api::v2::DiscoveryResponse discovery_response; + API_NO_BOOST(envoy::api::v2::DiscoveryResponse) discovery_response; discovery_response.set_version_info("1"); discovery_response.set_type_url(Config::TypeUrl::get().ClusterLoadAssignment); - envoy::api::v2::ClusterLoadAssignment cluster_load_assignment = - TestUtility::parseYaml(fmt::format( + auto cluster_load_assignment = + TestUtility::parseYaml(fmt::format( R"EOF( cluster_name: cluster_0 endpoints: diff --git a/test/integration/integration.cc b/test/integration/integration.cc index f41111d8c048c..728c32e8a2288 100644 --- a/test/integration/integration.cc +++ b/test/integration/integration.cc @@ -21,6 +21,7 @@ #include "common/common/assert.h" #include "common/common/fmt.h" #include "common/common/stack_array.h" +#include "common/config/api_version.h" #include "common/event/dispatcher_impl.h" #include "common/event/libevent.h" #include "common/network/connection_impl.h" @@ -335,7 +336,7 @@ void BaseIntegrationTest::createEnvoy() { if (use_lds_) { // After the config has been finalized, write the final listener config to the lds file. const std::string lds_path = config_helper_.bootstrap().dynamic_resources().lds_config().path(); - envoy::api::v2::DiscoveryResponse lds; + API_NO_BOOST(envoy::api::v2::DiscoveryResponse) lds; lds.set_version_info("0"); for (auto& listener : config_helper_.bootstrap().static_resources().listeners()) { ProtobufWkt::Any* resource = lds.add_resources(); @@ -582,7 +583,7 @@ AssertionResult BaseIntegrationTest::compareSotwDiscoveryRequest( const std::string& expected_type_url, const std::string& expected_version, const std::vector& expected_resource_names, bool expect_node, const Protobuf::int32 expected_error_code, const std::string& expected_error_substring) { - envoy::api::v2::DiscoveryRequest discovery_request; + API_NO_BOOST(envoy::api::v2::DiscoveryRequest) discovery_request; VERIFY_ASSERTION(xds_stream_->waitForGrpcMessage(*dispatcher_, discovery_request)); if (expect_node) { @@ -640,7 +641,7 @@ AssertionResult BaseIntegrationTest::compareDeltaDiscoveryRequest( const std::vector& expected_resource_subscriptions, const std::vector& expected_resource_unsubscriptions, FakeStreamPtr& xds_stream, const Protobuf::int32 expected_error_code, const std::string& expected_error_substring) { - envoy::api::v2::DeltaDiscoveryRequest request; + API_NO_BOOST(envoy::api::v2::DeltaDiscoveryRequest) request; VERIFY_ASSERTION(xds_stream->waitForGrpcMessage(*dispatcher_, request)); // Verify all we care about node. diff --git a/test/integration/integration.h b/test/integration/integration.h index b3839198b3a8a..1297379c50429 100644 --- a/test/integration/integration.h +++ b/test/integration/integration.h @@ -9,6 +9,7 @@ #include "envoy/api/v2/endpoint/endpoint.pb.h" #include "envoy/server/process_context.h" +#include "common/config/api_version.h" #include "common/http/codec_client.h" #include "test/common/grpc/grpc_client_integration.h" @@ -271,7 +272,7 @@ class BaseIntegrationTest : protected Logger::Loggable { template void sendSotwDiscoveryResponse(const std::string& type_url, const std::vector& messages, const std::string& version) { - envoy::api::v2::DiscoveryResponse discovery_response; + API_NO_BOOST(envoy::api::v2::DiscoveryResponse) discovery_response; discovery_response.set_version_info(version); discovery_response.set_type_url(type_url); for (const auto& message : messages) { @@ -293,7 +294,7 @@ class BaseIntegrationTest : protected Logger::Loggable { const std::vector& added_or_updated, const std::vector& removed, const std::string& version, FakeStreamPtr& stream) { - envoy::api::v2::DeltaDiscoveryResponse response; + API_NO_BOOST(envoy::api::v2::DeltaDiscoveryResponse) response; response.set_system_version_info("system_version_info_this_is_a_test"); response.set_type_url(type_url); for (const auto& message : added_or_updated) { diff --git a/test/integration/scoped_rds_integration_test.cc b/test/integration/scoped_rds_integration_test.cc index c1ac2d9c9f09a..e59342544aa33 100644 --- a/test/integration/scoped_rds_integration_test.cc +++ b/test/integration/scoped_rds_integration_test.cc @@ -6,6 +6,7 @@ #include "envoy/config/bootstrap/v2/bootstrap.pb.h" #include "envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.pb.h" +#include "common/config/api_version.h" #include "common/config/resources.h" #include "test/common/grpc/grpc_client_integration.h" @@ -160,7 +161,7 @@ class ScopedRdsIntegrationTest : public HttpIntegrationTest, } void sendRdsResponse(const std::string& route_config, const std::string& version) { - envoy::api::v2::DiscoveryResponse response; + API_NO_BOOST(envoy::api::v2::DiscoveryResponse) response; response.set_version_info(version); response.set_type_url(Config::TypeUrl::get().RouteConfiguration); auto route_configuration = @@ -187,7 +188,7 @@ class ScopedRdsIntegrationTest : public HttpIntegrationTest, const std::string& version) { ASSERT(scoped_rds_upstream_info_.stream_by_resource_name_[srds_config_name_] != nullptr); - envoy::api::v2::DeltaDiscoveryResponse response; + API_NO_BOOST(envoy::api::v2::DeltaDiscoveryResponse) response; response.set_system_version_info(version); response.set_type_url(Config::TypeUrl::get().ScopedRouteConfiguration); @@ -210,7 +211,7 @@ class ScopedRdsIntegrationTest : public HttpIntegrationTest, const std::string& version) { ASSERT(scoped_rds_upstream_info_.stream_by_resource_name_[srds_config_name_] != nullptr); - envoy::api::v2::DiscoveryResponse response; + API_NO_BOOST(envoy::api::v2::DiscoveryResponse) response; response.set_version_info(version); response.set_type_url(Config::TypeUrl::get().ScopedRouteConfiguration); diff --git a/test/integration/sds_dynamic_integration_test.cc b/test/integration/sds_dynamic_integration_test.cc index 1027dd95304f6..fe91152ef23b9 100644 --- a/test/integration/sds_dynamic_integration_test.cc +++ b/test/integration/sds_dynamic_integration_test.cc @@ -7,6 +7,7 @@ #include "envoy/config/bootstrap/v2/bootstrap.pb.h" #include "envoy/service/discovery/v2/sds.pb.h" +#include "common/config/api_version.h" #include "common/config/resources.h" #include "common/event/dispatcher_impl.h" #include "common/network/connection_impl.h" @@ -117,7 +118,7 @@ class SdsDynamicIntegrationBaseTest : public Grpc::GrpcClientIntegrationParamTes } void sendSdsResponse(const envoy::api::v2::auth::Secret& secret) { - envoy::api::v2::DiscoveryResponse discovery_response; + API_NO_BOOST(envoy::api::v2::DiscoveryResponse) discovery_response; discovery_response.set_version_info("1"); discovery_response.set_type_url(Config::TypeUrl::get().Secret); discovery_response.add_resources()->PackFrom(secret); diff --git a/test/integration/tcp_proxy_integration_test.cc b/test/integration/tcp_proxy_integration_test.cc index 068530969862a..1fa01b9469727 100644 --- a/test/integration/tcp_proxy_integration_test.cc +++ b/test/integration/tcp_proxy_integration_test.cc @@ -8,6 +8,7 @@ #include "envoy/config/bootstrap/v2/bootstrap.pb.h" #include "envoy/config/filter/network/tcp_proxy/v2/tcp_proxy.pb.h" +#include "common/config/api_version.h" #include "common/network/utility.h" #include "extensions/transport_sockets/tls/context_manager_impl.h" @@ -240,10 +241,10 @@ TEST_P(TcpProxyIntegrationTest, AccessLog) { auto* filter_chain = listener->mutable_filter_chains(0); auto* config_blob = filter_chain->mutable_filters(0)->mutable_typed_config(); - ASSERT_TRUE(config_blob->Is()); - auto tcp_proxy_config = - MessageUtil::anyConvert( - *config_blob); + ASSERT_TRUE( + config_blob->Is()); + auto tcp_proxy_config = MessageUtil::anyConvert(*config_blob); auto* access_log = tcp_proxy_config.add_access_log(); access_log->set_name("envoy.file_access_log"); @@ -329,10 +330,10 @@ TEST_P(TcpProxyIntegrationTest, TestIdletimeoutWithNoData) { auto* filter_chain = listener->mutable_filter_chains(0); auto* config_blob = filter_chain->mutable_filters(0)->mutable_typed_config(); - ASSERT_TRUE(config_blob->Is()); - auto tcp_proxy_config = - MessageUtil::anyConvert( - *config_blob); + ASSERT_TRUE( + config_blob->Is()); + auto tcp_proxy_config = MessageUtil::anyConvert(*config_blob); tcp_proxy_config.mutable_idle_timeout()->set_nanos( std::chrono::duration_cast(std::chrono::milliseconds(100)) .count()); @@ -352,10 +353,10 @@ TEST_P(TcpProxyIntegrationTest, TestIdletimeoutWithLargeOutstandingData) { auto* filter_chain = listener->mutable_filter_chains(0); auto* config_blob = filter_chain->mutable_filters(0)->mutable_typed_config(); - ASSERT_TRUE(config_blob->Is()); - auto tcp_proxy_config = - MessageUtil::anyConvert( - *config_blob); + ASSERT_TRUE( + config_blob->Is()); + auto tcp_proxy_config = MessageUtil::anyConvert(*config_blob); tcp_proxy_config.mutable_idle_timeout()->set_nanos( std::chrono::duration_cast(std::chrono::milliseconds(500)) .count()); diff --git a/tools/api_boost/api_boost.py b/tools/api_boost/api_boost.py index 2d0fc7d4ad6a2..983380c1e2bfb 100755 --- a/tools/api_boost/api_boost.py +++ b/tools/api_boost/api_boost.py @@ -32,6 +32,10 @@ def PrefixDirectory(path_prefix): # Update a C++ file to the latest API. def ApiBoostFile(llvm_include_path, debug_log, path): print('Processing %s' % path) + if 'API_NO_BOOST_FILE' in pathlib.Path(path).read_text(): + if debug_log: + print('Not boosting %s due to API_NO_BOOST_FILE\n' % path) + return None # Run the booster try: result = sp.run([ @@ -57,6 +61,9 @@ def ApiBoostFile(llvm_include_path, debug_log, path): # below, we have more control over special casing as well, so ¯\_(ツ)_/¯. def RewriteIncludes(args): path, api_includes = args + # Files with API_NO_BOOST_FILE will have None returned by ApiBoostFile. + if api_includes is None: + return # We just dump the inferred API header includes at the start of the #includes # in the file and remove all the present API header includes. This does not # match Envoy style; we rely on later invocations of fix_format.sh to take diff --git a/tools/api_boost/api_boost_test.py b/tools/api_boost/api_boost_test.py index abb2d6dcd6d48..7d18abe9fc27d 100755 --- a/tools/api_boost/api_boost_test.py +++ b/tools/api_boost/api_boost_test.py @@ -25,6 +25,7 @@ ('elaborated_type', 'ElaboratedTypeLoc type upgrades'), ('using_decl', 'UsingDecl upgrades for named types'), ('decl_ref_expr', 'DeclRefExpr upgrades for named constants'), + ('no_boost_file', 'API_NO_BOOST_FILE annotations'), ])) TESTDATA_PATH = 'tools/api_boost/testdata' diff --git a/tools/api_boost/testdata/BUILD b/tools/api_boost/testdata/BUILD index 78a4479fc9e7a..fe19b715ce364 100644 --- a/tools/api_boost/testdata/BUILD +++ b/tools/api_boost/testdata/BUILD @@ -20,6 +20,12 @@ envoy_cc_library( deps = ["@envoy_api//envoy/config/overload/v2alpha:pkg_cc_proto"], ) +envoy_cc_library( + name = "no_boost_file", + srcs = ["no_boost_file.cc"], + deps = ["@envoy_api//envoy/config/overload/v2alpha:pkg_cc_proto"], +) + envoy_cc_library( name = "using_decl", srcs = ["using_decl.cc"], diff --git a/tools/api_boost/testdata/decl_ref_expr.cc b/tools/api_boost/testdata/decl_ref_expr.cc index 1d5adc0920666..4c5996dc064b7 100644 --- a/tools/api_boost/testdata/decl_ref_expr.cc +++ b/tools/api_boost/testdata/decl_ref_expr.cc @@ -1,5 +1,8 @@ #include "envoy/config/overload/v2alpha/overload.pb.h" +#define API_NO_BOOST(x) x +#define BAR(x) x + using envoy::config::overload::v2alpha::Trigger; class ThresholdTriggerImpl { @@ -17,5 +20,8 @@ class ThresholdTriggerImpl { default: break; } + API_NO_BOOST(envoy::config::overload::v2alpha::Trigger) foo; + BAR(API_NO_BOOST(envoy::config::overload::v2alpha::Trigger)) bar; + BAR(envoy::config::overload::v2alpha::Trigger) baz; } }; diff --git a/tools/api_boost/testdata/decl_ref_expr.cc.gold b/tools/api_boost/testdata/decl_ref_expr.cc.gold index 1b774c4fee41c..22900a017a4af 100644 --- a/tools/api_boost/testdata/decl_ref_expr.cc.gold +++ b/tools/api_boost/testdata/decl_ref_expr.cc.gold @@ -1,5 +1,8 @@ #include "envoy/config/overload/v3alpha/overload.pb.h" +#define API_NO_BOOST(x) x +#define BAR(x) x + using envoy::config::overload::v3alpha::Trigger; class ThresholdTriggerImpl { @@ -17,5 +20,8 @@ public: default: break; } + API_NO_BOOST(envoy::config::overload::v2alpha::Trigger) foo; + BAR(API_NO_BOOST(envoy::config::overload::v2alpha::Trigger)) bar; + BAR(envoy::config::overload::v3alpha::Trigger) baz; } }; diff --git a/tools/api_boost/testdata/no_boost_file.cc b/tools/api_boost/testdata/no_boost_file.cc new file mode 100644 index 0000000000000..82d11a26410b0 --- /dev/null +++ b/tools/api_boost/testdata/no_boost_file.cc @@ -0,0 +1,12 @@ +#include "envoy/config/overload/v2alpha/overload.pb.h" + +// API_NO_BOOST_FILE + +using envoy::config::overload::v2alpha::ThresholdTrigger; +using SomePtrAlias = std::unique_ptr; + +class ThresholdTriggerImpl { +public: + ThresholdTriggerImpl(const ThresholdTrigger& /*config*/) {} + ThresholdTriggerImpl(SomePtrAlias /*config*/) {} +}; diff --git a/tools/api_boost/testdata/no_boost_file.cc.gold b/tools/api_boost/testdata/no_boost_file.cc.gold new file mode 100644 index 0000000000000..82d11a26410b0 --- /dev/null +++ b/tools/api_boost/testdata/no_boost_file.cc.gold @@ -0,0 +1,12 @@ +#include "envoy/config/overload/v2alpha/overload.pb.h" + +// API_NO_BOOST_FILE + +using envoy::config::overload::v2alpha::ThresholdTrigger; +using SomePtrAlias = std::unique_ptr; + +class ThresholdTriggerImpl { +public: + ThresholdTriggerImpl(const ThresholdTrigger& /*config*/) {} + ThresholdTriggerImpl(SomePtrAlias /*config*/) {} +}; diff --git a/tools/clang_tools/api_booster/main.cc b/tools/clang_tools/api_booster/main.cc index e110079ce0a03..4337e91272121 100644 --- a/tools/clang_tools/api_booster/main.cc +++ b/tools/clang_tools/api_booster/main.cc @@ -212,9 +212,19 @@ class ApiBooster : public clang::ast_matchers::MatchFinder::MatchCallback, return; } // Add corresponding replacement. - const clang::SourceLocation start = source_range->getBegin(); - const clang::SourceLocation end = - clang::Lexer::getLocForEndOfToken(source_range->getEnd(), 0, source_manager, lexer_lopt_); + const clang::SourceLocation non_spelling_start = source_range->getBegin(); + if (non_spelling_start.isMacroID()) { + DEBUG_LOG("macro"); + auto macro_name = + clang::Lexer::getImmediateMacroName(non_spelling_start, source_manager, lexer_lopt_); + if (macro_name.str() == "API_NO_BOOST") { + DEBUG_LOG("Skipping replacement due to API_NO_BOOST"); + return; + } + } + const clang::SourceLocation start = source_manager.getSpellingLoc(source_range->getBegin()); + const clang::SourceLocation end = clang::Lexer::getLocForEndOfToken( + source_manager.getSpellingLoc(source_range->getEnd()), 0, source_manager, lexer_lopt_); const size_t length = source_manager.getFileOffset(end) - source_manager.getFileOffset(start); clang::tooling::Replacement type_replacement( source_manager, start, length, ProtoCxxUtils::protoToCxxType(latest_type_info->type_name_)); From 508ef52af85de7b8f022cdba3ab9acc9361c2131 Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Wed, 25 Dec 2019 22:29:17 -0500 Subject: [PATCH 2/3] Fix handling of includes for skipped types. Signed-off-by: Harvey Tuch --- .../api_boost/testdata/decl_ref_expr.cc.gold | 1 + tools/clang_tools/api_booster/main.cc | 40 +++++++++---------- tools/type_whisperer/api_type_db.cc | 9 +++++ tools/type_whisperer/api_type_db.h | 1 + 4 files changed, 31 insertions(+), 20 deletions(-) diff --git a/tools/api_boost/testdata/decl_ref_expr.cc.gold b/tools/api_boost/testdata/decl_ref_expr.cc.gold index 22900a017a4af..b77f18dce2b4e 100644 --- a/tools/api_boost/testdata/decl_ref_expr.cc.gold +++ b/tools/api_boost/testdata/decl_ref_expr.cc.gold @@ -1,3 +1,4 @@ +#include "envoy/config/overload/v2alpha/overload.pb.h" #include "envoy/config/overload/v3alpha/overload.pb.h" #define API_NO_BOOST(x) x diff --git a/tools/clang_tools/api_booster/main.cc b/tools/clang_tools/api_booster/main.cc index 4337e91272121..afb10147b5be5 100644 --- a/tools/clang_tools/api_booster/main.cc +++ b/tools/clang_tools/api_booster/main.cc @@ -195,39 +195,38 @@ class ApiBooster : public clang::ast_matchers::MatchFinder::MatchCallback, void tryBoostType(const std::string& type_name, absl::optional source_range, const clang::SourceManager& source_manager, absl::string_view debug_description, bool validation_required = false) { - const auto latest_type_info = getLatestTypeInformationFromCType(type_name); + bool is_skip_macro = false; + if (source_range && source_range->getBegin().isMacroID()) { + DEBUG_LOG("macro"); + auto macro_name = clang::Lexer::getImmediateMacroName(source_range->getBegin(), + source_manager, lexer_lopt_); + if (macro_name.str() == "API_NO_BOOST") { + DEBUG_LOG("Skipping replacement due to API_NO_BOOST"); + is_skip_macro = true; + } + } + const auto type_info = getTypeInformationFromCType(type_name, !is_skip_macro); // If this isn't a known API type, our work here is done. - if (!latest_type_info) { + if (!type_info) { return; } DEBUG_LOG(absl::StrCat("Matched type '", type_name, "' (", debug_description, ")")); // Track corresponding imports. - source_api_proto_paths_.insert(adjustProtoSuffix(latest_type_info->proto_path_, ".pb.h")); + source_api_proto_paths_.insert(adjustProtoSuffix(type_info->proto_path_, ".pb.h")); if (validation_required) { - source_api_proto_paths_.insert( - adjustProtoSuffix(latest_type_info->proto_path_, ".pb.validate.h")); + source_api_proto_paths_.insert(adjustProtoSuffix(type_info->proto_path_, ".pb.validate.h")); } // Not all AST matchers know how to do replacements (yet?). - if (!source_range) { + if (!source_range || is_skip_macro) { return; } // Add corresponding replacement. - const clang::SourceLocation non_spelling_start = source_range->getBegin(); - if (non_spelling_start.isMacroID()) { - DEBUG_LOG("macro"); - auto macro_name = - clang::Lexer::getImmediateMacroName(non_spelling_start, source_manager, lexer_lopt_); - if (macro_name.str() == "API_NO_BOOST") { - DEBUG_LOG("Skipping replacement due to API_NO_BOOST"); - return; - } - } const clang::SourceLocation start = source_manager.getSpellingLoc(source_range->getBegin()); const clang::SourceLocation end = clang::Lexer::getLocForEndOfToken( source_manager.getSpellingLoc(source_range->getEnd()), 0, source_manager, lexer_lopt_); const size_t length = source_manager.getFileOffset(end) - source_manager.getFileOffset(start); clang::tooling::Replacement type_replacement( - source_manager, start, length, ProtoCxxUtils::protoToCxxType(latest_type_info->type_name_)); + source_manager, start, length, ProtoCxxUtils::protoToCxxType(type_info->type_name_)); llvm::Error error = replacements_[type_replacement.getFilePath()].add(type_replacement); if (error) { std::cerr << " Replacement insertion error: " << llvm::toString(std::move(error)) @@ -246,8 +245,8 @@ class ApiBooster : public clang::ast_matchers::MatchFinder::MatchCallback, // Obtain the latest type information for a given from C++ type, e.g. envoy:config::v2::Cluster, // from the API type database. - absl::optional - getLatestTypeInformationFromCType(const std::string& c_type_name) { + absl::optional getTypeInformationFromCType(const std::string& c_type_name, + bool latest) { // Ignore compound or non-API types. // TODO(htuch): this is all super hacky and not really right, we should be // removing qualifiers etc. to get to the underlying type name. @@ -258,7 +257,8 @@ class ApiBooster : public clang::ast_matchers::MatchFinder::MatchCallback, const std::string proto_type_name = ProtoCxxUtils::cxxToProtoType(type_name); // Use API type database to map from proto type to path. - auto result = ApiTypeDb::getLatestTypeInformation(proto_type_name); + auto result = latest ? ApiTypeDb::getLatestTypeInformation(proto_type_name) + : ApiTypeDb::getExistingTypeInformation(proto_type_name); if (result) { // Remove the .proto extension. return result; diff --git a/tools/type_whisperer/api_type_db.cc b/tools/type_whisperer/api_type_db.cc index bd5f5932011fc..2ec3fd834667f 100644 --- a/tools/type_whisperer/api_type_db.cc +++ b/tools/type_whisperer/api_type_db.cc @@ -27,6 +27,15 @@ const tools::type_whisperer::TypeDb& getApiTypeDb() { } // namespace +absl::optional +ApiTypeDb::getExistingTypeInformation(const std::string& type_name) { + auto it = getApiTypeDb().types().find(type_name); + if (it == getApiTypeDb().types().end()) { + return {}; + } + return absl::make_optional(type_name, it->second.proto_path()); +} + absl::optional ApiTypeDb::getLatestTypeInformation(const std::string& type_name) { absl::optional result; std::string current_type_name = type_name; diff --git a/tools/type_whisperer/api_type_db.h b/tools/type_whisperer/api_type_db.h index 8647990b6843e..da55e80776676 100644 --- a/tools/type_whisperer/api_type_db.h +++ b/tools/type_whisperer/api_type_db.h @@ -24,6 +24,7 @@ struct TypeInformation { // libtool binaries). class ApiTypeDb { public: + static absl::optional getExistingTypeInformation(const std::string& type_name); static absl::optional getLatestTypeInformation(const std::string& type_name); }; From 96744204818524606667a1fbd0c456ebc1ba2537 Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Wed, 25 Dec 2019 22:59:07 -0500 Subject: [PATCH 3/3] Resolve merge conflicts. Signed-off-by: Harvey Tuch --- tools/api_boost/testdata/decl_ref_expr.cc.gold | 7 +------ tools/clang_tools/api_booster/main.cc | 12 ++++++------ tools/type_whisperer/api_type_db.cc | 2 +- 3 files changed, 8 insertions(+), 13 deletions(-) diff --git a/tools/api_boost/testdata/decl_ref_expr.cc.gold b/tools/api_boost/testdata/decl_ref_expr.cc.gold index e9d28b0c24afe..5b363fe9f770c 100644 --- a/tools/api_boost/testdata/decl_ref_expr.cc.gold +++ b/tools/api_boost/testdata/decl_ref_expr.cc.gold @@ -1,12 +1,10 @@ -#include "envoy/config/overload/v3alpha/overload.pb.h" - #include "envoy/api/v3alpha/cds.pb.h" #include "envoy/config/overload/v2alpha/overload.pb.h" #include "envoy/config/overload/v3alpha/overload.pb.h" -#define ASSERT(x) static_cast(x) #define API_NO_BOOST(x) x #define BAR(x) x +#define ASSERT(x) static_cast(x) using envoy::config::overload::v3alpha::Trigger; @@ -25,15 +23,12 @@ public: default: break; } -<<<<<<< HEAD API_NO_BOOST(envoy::config::overload::v2alpha::Trigger) foo; BAR(API_NO_BOOST(envoy::config::overload::v2alpha::Trigger)) bar; BAR(envoy::config::overload::v3alpha::Trigger) baz; -======= envoy::config::overload::v3alpha::ThresholdTrigger::default_instance(); ASSERT(envoy::config::overload::v3alpha::Trigger::kThreshold == Trigger::kThreshold); envoy::api::v3alpha::Cluster_LbPolicy_Name(0); static_cast(envoy::api::v3alpha::Cluster::ORIGINAL_DST_LB); ->>>>>>> upstream/master } }; diff --git a/tools/clang_tools/api_booster/main.cc b/tools/clang_tools/api_booster/main.cc index 313e2e901ffad..62e4dae00aa75 100644 --- a/tools/clang_tools/api_booster/main.cc +++ b/tools/clang_tools/api_booster/main.cc @@ -220,7 +220,7 @@ class ApiBooster : public clang::ast_matchers::MatchFinder::MatchCallback, const clang::SourceManager& source_manager) { const std::string type_name = member_call_expr.getObjectType().getCanonicalType().getUnqualifiedType().getAsString(); - const auto latest_type_info = getLatestTypeInformationFromCType(type_name); + const auto latest_type_info = getTypeInformationFromCType(type_name, true); // If this isn't a known API type, our work here is done. if (!latest_type_info) { return; @@ -265,7 +265,7 @@ class ApiBooster : public clang::ast_matchers::MatchFinder::MatchCallback, const clang::SourceManager& source_manager, absl::string_view debug_description, bool requires_enum_truncation, bool validation_required = false) { if (source_range) { - tryBoostType(type_name, source_manager.getSpellingLoc(source_range->getBegin()), + tryBoostType(type_name, source_range->getBegin(), sourceRangeLength(*source_range, source_manager), source_manager, debug_description, requires_enum_truncation, validation_required); } else { @@ -280,8 +280,7 @@ class ApiBooster : public clang::ast_matchers::MatchFinder::MatchCallback, bool is_skip_macro = false; if (begin_loc.isMacroID()) { DEBUG_LOG("macro"); - auto macro_name = clang::Lexer::getImmediateMacroName(begin_loc, - source_manager, lexer_lopt_); + auto macro_name = clang::Lexer::getImmediateMacroName(begin_loc, source_manager, lexer_lopt_); if (macro_name.str() == "API_NO_BOOST") { DEBUG_LOG("Skipping replacement due to API_NO_BOOST"); is_skip_macro = true; @@ -303,13 +302,14 @@ class ApiBooster : public clang::ast_matchers::MatchFinder::MatchCallback, if (length == -1 || is_skip_macro) { return; } + const clang::SourceLocation spelling_begin = source_manager.getSpellingLoc(begin_loc); // We need to look at the text we're replacing to decide whether we should // use the qualified C++'ified proto name. const bool qualified = - getSourceText(begin_loc, length, source_manager).find("::") != std::string::npos; + getSourceText(spelling_begin, length, source_manager).find("::") != std::string::npos; // Add corresponding replacement. const clang::tooling::Replacement type_replacement( - source_manager, begin_loc, length, + source_manager, spelling_begin, length, ProtoCxxUtils::protoToCxxType(type_info->type_name_, qualified, type_info->enum_type_ && requires_enum_truncation)); insertReplacement(type_replacement); diff --git a/tools/type_whisperer/api_type_db.cc b/tools/type_whisperer/api_type_db.cc index 1b21ce32a2aa9..fbb5b843c0794 100644 --- a/tools/type_whisperer/api_type_db.cc +++ b/tools/type_whisperer/api_type_db.cc @@ -55,7 +55,7 @@ ApiTypeDb::getExistingTypeInformation(const std::string& type_name) { if (it == getApiTypeDb().types().end()) { return {}; } - return absl::make_optional(type_name, it->second.proto_path()); + return absl::make_optional(type_name, it->second.proto_path(), false); } absl::optional ApiTypeDb::getLatestTypeInformation(const std::string& type_name) {