Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions STYLE.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@
* `using BarSharedPtr = std::shared_ptr<Bar>;`
* `using BlahConstSharedPtr = std::shared_ptr<const Blah>;`
* Regular pointers (e.g. `int* foo`) should not be type aliased.
* `absl::optional<std::reference_wrapper<T>> is type aliased:
* `using FooOptRef = absl::optional<std::reference_wrapper<T>>;`
* `using FooOptConstRef = absl::optional<std::reference_wrapper<T>>;`
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably expand the language on line 41 beyond "smart pointers".

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@derekargueta not sure what you mean? I see these two as different bullet points.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh sorry, missed the un-indentation on 46 to make it a new section 👍

* If move semantics are intended, prefer specifying function arguments with `&&`.
E.g., `void onHeaders(Http::HeaderMapPtr&& headers, ...)`. The rationale for this is that it
forces the caller to specify `std::move(...)` or pass a temporary and makes the intention at
Expand Down
2 changes: 1 addition & 1 deletion include/envoy/api/api.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class Api {
/**
* @return an optional reference to the ProcessContext
*/
virtual OptProcessContextRef processContext() PURE;
virtual ProcessContextOptRef processContext() PURE;
};

using ApiPtr = std::unique_ptr<Api>;
Expand Down
1 change: 1 addition & 0 deletions include/envoy/network/listen_socket.h
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ class Socket {

using SocketPtr = std::unique_ptr<Socket>;
using SocketSharedPtr = std::shared_ptr<Socket>;
using SocketOptRef = absl::optional<std::reference_wrapper<Socket>>;

/**
* A socket passed to a connection. For server connections this represents the accepted socket, and
Expand Down
2 changes: 1 addition & 1 deletion include/envoy/network/listener.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class ListenSocketFactory {
/**
* @return the socket shared by worker threads if any; otherwise return null.
*/
virtual absl::optional<std::reference_wrapper<Socket>> sharedSocket() const PURE;
virtual SocketOptRef sharedSocket() const PURE;
};

using ListenSocketFactorySharedPtr = std::shared_ptr<ListenSocketFactory>;
Expand Down
4 changes: 2 additions & 2 deletions include/envoy/server/filter_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -195,10 +195,10 @@ class FactoryContext : public virtual CommonFactoryContext {
virtual Grpc::Context& grpcContext() PURE;

/**
* @return OptProcessContextRef an optional reference to the
* @return ProcessContextOptRef an optional reference to the
* process context. Will be unset when running in validation mode.
*/
virtual OptProcessContextRef processContext() PURE;
virtual ProcessContextOptRef processContext() PURE;

/**
* @return ProtobufMessage::ValidationVisitor& validation visitor for filter configuration
Expand Down
2 changes: 1 addition & 1 deletion include/envoy/server/instance.h
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ class Instance {
/**
* @return the server-wide process context.
*/
virtual OptProcessContextRef processContext() PURE;
virtual ProcessContextOptRef processContext() PURE;

/**
* @return ThreadLocal::Instance& the thread local storage engine for the server. This is used to
Expand Down
4 changes: 3 additions & 1 deletion include/envoy/server/process_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ class ProcessObject {
virtual ~ProcessObject() = default;
};

using ProcessObjectOptRef = absl::optional<std::reference_wrapper<ProcessObject>>;

/**
* Context passed to filters to access resources from non-Envoy parts of the
* process.
Expand All @@ -28,6 +30,6 @@ class ProcessContext {
virtual ProcessObject& get() const PURE;
};

using OptProcessContextRef = absl::optional<std::reference_wrapper<ProcessContext>>;
using ProcessContextOptRef = absl::optional<std::reference_wrapper<ProcessContext>>;

} // namespace Envoy
12 changes: 6 additions & 6 deletions include/envoy/stats/scope.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ class Histogram;
class Scope;
class NullGaugeImpl;

using OptionalCounter = absl::optional<std::reference_wrapper<const Counter>>;
using OptionalGauge = absl::optional<std::reference_wrapper<const Gauge>>;
using OptionalHistogram = absl::optional<std::reference_wrapper<const Histogram>>;
using CounterOptConstRef = absl::optional<std::reference_wrapper<const Counter>>;
using GaugeOptConstRef = absl::optional<std::reference_wrapper<const Gauge>>;
using HistogramOptConstRef = absl::optional<std::reference_wrapper<const Histogram>>;
using ScopePtr = std::unique_ptr<Scope>;
using ScopeSharedPtr = std::shared_ptr<Scope>;

Expand Down Expand Up @@ -98,20 +98,20 @@ class Scope {
* @param The name of the stat, obtained from the SymbolTable.
* @return a reference to a counter within the scope's namespace, if it exists.
*/
virtual OptionalCounter findCounter(StatName name) const PURE;
virtual CounterOptConstRef findCounter(StatName name) const PURE;

/**
* @param The name of the stat, obtained from the SymbolTable.
* @return a reference to a gauge within the scope's namespace, if it exists.
*/
virtual OptionalGauge findGauge(StatName name) const PURE;
virtual GaugeOptConstRef findGauge(StatName name) const PURE;

/**
* @param The name of the stat, obtained from the SymbolTable.
* @return a reference to a histogram within the scope's namespace, if it
* exists.
*/
virtual OptionalHistogram findHistogram(StatName name) const PURE;
virtual HistogramOptConstRef findHistogram(StatName name) const PURE;

/**
* @return a reference to the symbol table.
Expand Down
2 changes: 1 addition & 1 deletion source/common/api/api_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ namespace Api {

Impl::Impl(Thread::ThreadFactory& thread_factory, Stats::Store& store,
Event::TimeSystem& time_system, Filesystem::Instance& file_system,
const OptProcessContextRef& process_context)
const ProcessContextOptRef& process_context)
: thread_factory_(thread_factory), store_(store), time_system_(time_system),
file_system_(file_system), process_context_(process_context) {}

Expand Down
6 changes: 3 additions & 3 deletions source/common/api/api_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class Impl : public Api {
public:
Impl(Thread::ThreadFactory& thread_factory, Stats::Store& store, Event::TimeSystem& time_system,
Filesystem::Instance& file_system,
const OptProcessContextRef& process_context = absl::nullopt);
const ProcessContextOptRef& process_context = absl::nullopt);

// Api::Api
Event::DispatcherPtr allocateDispatcher() override;
Expand All @@ -27,14 +27,14 @@ class Impl : public Api {
Filesystem::Instance& fileSystem() override { return file_system_; }
TimeSource& timeSource() override { return time_system_; }
const Stats::Scope& rootScope() override { return store_; }
OptProcessContextRef processContext() override { return process_context_; }
ProcessContextOptRef processContext() override { return process_context_; }

private:
Thread::ThreadFactory& thread_factory_;
Stats::Store& store_;
Event::TimeSystem& time_system_;
Filesystem::Instance& file_system_;
OptProcessContextRef process_context_;
ProcessContextOptRef process_context_;
};

} // namespace Api
Expand Down
5 changes: 2 additions & 3 deletions source/common/network/addr_family_aware_socket_option_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,8 @@ absl::optional<Address::IpVersion> getVersionFromSocket(const Socket& socket) {
return absl::nullopt;
}

absl::optional<std::reference_wrapper<SocketOptionImpl>>
getOptionForSocket(const Socket& socket, SocketOptionImpl& ipv4_option,
SocketOptionImpl& ipv6_option) {
SocketOptionImplOptRef getOptionForSocket(const Socket& socket, SocketOptionImpl& ipv4_option,
SocketOptionImpl& ipv6_option) {
auto version = getVersionFromSocket(socket);
if (!version.has_value()) {
return absl::nullopt;
Expand Down
2 changes: 2 additions & 0 deletions source/common/network/socket_option_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -146,5 +146,7 @@ class SocketOptionImpl : public Socket::Option, Logger::Loggable<Logger::Id::con
const std::vector<uint8_t> value_;
};

using SocketOptionImplOptRef = absl::optional<std::reference_wrapper<SocketOptionImpl>>;

} // namespace Network
} // namespace Envoy
11 changes: 7 additions & 4 deletions source/common/stats/isolated_store_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ template <class Base> class IsolatedStatsCache {
using CounterAllocator = std::function<RefcountPtr<Base>(StatName name)>;
using GaugeAllocator = std::function<RefcountPtr<Base>(StatName, Gauge::ImportMode)>;
using HistogramAllocator = std::function<RefcountPtr<Base>(StatName, Histogram::Unit)>;
using BaseOptConstRef = absl::optional<std::reference_wrapper<const Base>>;

IsolatedStatsCache(CounterAllocator alloc) : counter_alloc_(alloc) {}
IsolatedStatsCache(GaugeAllocator alloc) : gauge_alloc_(alloc) {}
Expand Down Expand Up @@ -79,7 +80,7 @@ template <class Base> class IsolatedStatsCache {
private:
friend class IsolatedStoreImpl;

absl::optional<std::reference_wrapper<const Base>> find(StatName name) const {
BaseOptConstRef find(StatName name) const {
auto stat = stats_.find(name);
if (stat == stats_.end()) {
return absl::nullopt;
Expand Down Expand Up @@ -113,9 +114,11 @@ class IsolatedStoreImpl : public StoreImpl {
Histogram& histogram = histograms_.get(name, unit);
return histogram;
}
OptionalCounter findCounter(StatName name) const override { return counters_.find(name); }
OptionalGauge findGauge(StatName name) const override { return gauges_.find(name); }
OptionalHistogram findHistogram(StatName name) const override { return histograms_.find(name); }
CounterOptConstRef findCounter(StatName name) const override { return counters_.find(name); }
GaugeOptConstRef findGauge(StatName name) const override { return gauges_.find(name); }
HistogramOptConstRef findHistogram(StatName name) const override {
return histograms_.find(name);
}

// Stats::Store
std::vector<CounterSharedPtr> counters() const override { return counters_.toVector(); }
Expand Down
8 changes: 5 additions & 3 deletions source/common/stats/scope_prefixer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,13 @@ Histogram& ScopePrefixer::histogramFromStatName(StatName name, Histogram::Unit u
return scope_.histogramFromStatName(StatName(stat_name_storage.get()), unit);
}

OptionalCounter ScopePrefixer::findCounter(StatName name) const { return scope_.findCounter(name); }
CounterOptConstRef ScopePrefixer::findCounter(StatName name) const {
return scope_.findCounter(name);
}

OptionalGauge ScopePrefixer::findGauge(StatName name) const { return scope_.findGauge(name); }
GaugeOptConstRef ScopePrefixer::findGauge(StatName name) const { return scope_.findGauge(name); }

OptionalHistogram ScopePrefixer::findHistogram(StatName name) const {
HistogramOptConstRef ScopePrefixer::findHistogram(StatName name) const {
return scope_.findHistogram(name);
}

Expand Down
6 changes: 3 additions & 3 deletions source/common/stats/scope_prefixer.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ class ScopePrefixer : public Scope {
return histogramFromStatName(storage.statName(), unit);
}

OptionalCounter findCounter(StatName name) const override;
OptionalGauge findGauge(StatName name) const override;
OptionalHistogram findHistogram(StatName name) const override;
CounterOptConstRef findCounter(StatName name) const override;
GaugeOptConstRef findGauge(StatName name) const override;
HistogramOptConstRef findHistogram(StatName name) const override;

const SymbolTable& constSymbolTable() const override { return scope_.constSymbolTable(); }
SymbolTable& symbolTable() override { return scope_.symbolTable(); }
Expand Down
2 changes: 1 addition & 1 deletion source/common/stats/stat_merger.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ void StatMerger::mergeGauges(const Protobuf::Map<std::string, uint64_t>& gauges)

StatNameManagedStorage storage(gauge.first, temp_scope_->symbolTable());
StatName stat_name = storage.statName();
OptionalGauge gauge_opt = temp_scope_->findGauge(stat_name);
GaugeOptConstRef gauge_opt = temp_scope_->findGauge(stat_name);

Gauge::ImportMode import_mode = Gauge::ImportMode::Uninitialized;
if (gauge_opt) {
Expand Down
6 changes: 3 additions & 3 deletions source/common/stats/thread_local_store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -512,15 +512,15 @@ Histogram& ThreadLocalStoreImpl::ScopeImpl::histogramFromStatName(StatName name,
return **central_ref;
}

OptionalCounter ThreadLocalStoreImpl::ScopeImpl::findCounter(StatName name) const {
CounterOptConstRef ThreadLocalStoreImpl::ScopeImpl::findCounter(StatName name) const {
return findStatLockHeld<Counter>(name, central_cache_->counters_);
}

OptionalGauge ThreadLocalStoreImpl::ScopeImpl::findGauge(StatName name) const {
GaugeOptConstRef ThreadLocalStoreImpl::ScopeImpl::findGauge(StatName name) const {
return findStatLockHeld<Gauge>(name, central_cache_->gauges_);
}

OptionalHistogram ThreadLocalStoreImpl::ScopeImpl::findHistogram(StatName name) const {
HistogramOptConstRef ThreadLocalStoreImpl::ScopeImpl::findHistogram(StatName name) const {
auto iter = central_cache_->histograms_.find(name);
if (iter == central_cache_->histograms_.end()) {
return absl::nullopt;
Expand Down
18 changes: 9 additions & 9 deletions source/common/stats/thread_local_store.h
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,8 @@ class ThreadLocalStoreImpl : Logger::Loggable<Logger::Id::stats>, public StoreRo
const SymbolTable& constSymbolTable() const override { return alloc_.constSymbolTable(); }
SymbolTable& symbolTable() override { return alloc_.symbolTable(); }
const TagProducer& tagProducer() const { return *tag_producer_; }
OptionalCounter findCounter(StatName name) const override {
OptionalCounter found_counter;
CounterOptConstRef findCounter(StatName name) const override {
CounterOptConstRef found_counter;
Thread::LockGuard lock(lock_);
for (ScopeImpl* scope : scopes_) {
found_counter = scope->findCounter(name);
Expand All @@ -193,8 +193,8 @@ class ThreadLocalStoreImpl : Logger::Loggable<Logger::Id::stats>, public StoreRo
}
return absl::nullopt;
}
OptionalGauge findGauge(StatName name) const override {
OptionalGauge found_gauge;
GaugeOptConstRef findGauge(StatName name) const override {
GaugeOptConstRef found_gauge;
Thread::LockGuard lock(lock_);
for (ScopeImpl* scope : scopes_) {
found_gauge = scope->findGauge(name);
Expand All @@ -204,8 +204,8 @@ class ThreadLocalStoreImpl : Logger::Loggable<Logger::Id::stats>, public StoreRo
}
return absl::nullopt;
}
OptionalHistogram findHistogram(StatName name) const override {
OptionalHistogram found_histogram;
HistogramOptConstRef findHistogram(StatName name) const override {
HistogramOptConstRef found_histogram;
Thread::LockGuard lock(lock_);
for (ScopeImpl* scope : scopes_) {
found_histogram = scope->findHistogram(name);
Expand Down Expand Up @@ -306,9 +306,9 @@ class ThreadLocalStoreImpl : Logger::Loggable<Logger::Id::stats>, public StoreRo

// NOTE: The find methods assume that `name` is fully-qualified.
// Implementations will not add the scope prefix.
OptionalCounter findCounter(StatName name) const override;
OptionalGauge findGauge(StatName name) const override;
OptionalHistogram findHistogram(StatName name) const override;
CounterOptConstRef findCounter(StatName name) const override;
GaugeOptConstRef findGauge(StatName name) const override;
HistogramOptConstRef findHistogram(StatName name) const override;

template <class StatType>
using MakeStatFn = std::function<RefcountPtr<StatType>(Allocator&, StatName name,
Expand Down
2 changes: 1 addition & 1 deletion source/common/upstream/cluster_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1257,7 +1257,7 @@ ClusterManagerImpl::ThreadLocalClusterManagerImpl::ClusterEntry::connPool(

// Note: to simplify this, we assume that the factory is only called in the scope of this
// function. Otherwise, we'd need to capture a few of these variables by value.
ConnPoolsContainer::ConnPools::OptPoolRef pool =
ConnPoolsContainer::ConnPools::PoolOptRef pool =
container.pools_->getPool(priority, hash_key, [&]() {
return parent_.parent_.factory_.allocateConnPool(
parent_.thread_local_dispatcher_, host, priority, protocol,
Expand Down
4 changes: 2 additions & 2 deletions source/common/upstream/conn_pool_map.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ template <typename KEY_TYPE, typename POOL_TYPE> class ConnPoolMap {
public:
using PoolFactory = std::function<std::unique_ptr<POOL_TYPE>()>;
using DrainedCb = std::function<void()>;
using OptPoolRef = absl::optional<std::reference_wrapper<POOL_TYPE>>;
using PoolOptRef = absl::optional<std::reference_wrapper<POOL_TYPE>>;

ConnPoolMap(Event::Dispatcher& dispatcher, const HostConstSharedPtr& host,
ResourcePriority priority);
Expand All @@ -31,7 +31,7 @@ template <typename KEY_TYPE, typename POOL_TYPE> class ConnPoolMap {
* possible for this to fail if a limit on the number of pools allowed is reached.
* @return The pool corresponding to `key`, or `absl::nullopt`.
*/
OptPoolRef getPool(KEY_TYPE key, const PoolFactory& factory);
PoolOptRef getPool(KEY_TYPE key, const PoolFactory& factory);

/**
* @return the number of pools.
Expand Down
2 changes: 1 addition & 1 deletion source/common/upstream/conn_pool_map_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ template <typename KEY_TYPE, typename POOL_TYPE> ConnPoolMap<KEY_TYPE, POOL_TYPE
}

template <typename KEY_TYPE, typename POOL_TYPE>
typename ConnPoolMap<KEY_TYPE, POOL_TYPE>::OptPoolRef
typename ConnPoolMap<KEY_TYPE, POOL_TYPE>::PoolOptRef
ConnPoolMap<KEY_TYPE, POOL_TYPE>::getPool(KEY_TYPE key, const PoolFactory& factory) {
Common::AutoDebugRecursionChecker assert_not_in(recursion_checker_);
// TODO(klarose): Consider how we will change the connection pool's configuration in the future.
Expand Down
4 changes: 2 additions & 2 deletions source/common/upstream/priority_conn_pool_map.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ template <typename KEY_TYPE, typename POOL_TYPE> class PriorityConnPoolMap {
using ConnPoolMapType = ConnPoolMap<KEY_TYPE, POOL_TYPE>;
using PoolFactory = typename ConnPoolMapType::PoolFactory;
using DrainedCb = typename ConnPoolMapType::DrainedCb;
using OptPoolRef = typename ConnPoolMapType::OptPoolRef;
using PoolOptRef = typename ConnPoolMapType::PoolOptRef;

PriorityConnPoolMap(Event::Dispatcher& dispatcher, const HostConstSharedPtr& host);
~PriorityConnPoolMap();
Expand All @@ -26,7 +26,7 @@ template <typename KEY_TYPE, typename POOL_TYPE> class PriorityConnPoolMap {
* is reached.
* @return The pool corresponding to `key`, or `absl::nullopt`.
*/
OptPoolRef getPool(ResourcePriority priority, KEY_TYPE key, const PoolFactory& factory);
PoolOptRef getPool(ResourcePriority priority, KEY_TYPE key, const PoolFactory& factory);

/**
* @return the number of pools across all priorities.
Expand Down
2 changes: 1 addition & 1 deletion source/common/upstream/priority_conn_pool_map_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ template <typename KEY_TYPE, typename POOL_TYPE>
PriorityConnPoolMap<KEY_TYPE, POOL_TYPE>::~PriorityConnPoolMap() = default;

template <typename KEY_TYPE, typename POOL_TYPE>
typename PriorityConnPoolMap<KEY_TYPE, POOL_TYPE>::OptPoolRef
typename PriorityConnPoolMap<KEY_TYPE, POOL_TYPE>::PoolOptRef
PriorityConnPoolMap<KEY_TYPE, POOL_TYPE>::getPool(ResourcePriority priority, KEY_TYPE key,
const PoolFactory& factory) {
size_t index = static_cast<size_t>(priority);
Expand Down
2 changes: 1 addition & 1 deletion source/server/config_validation/server.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ class ValidationInstance final : Logger::Loggable<Logger::Id::main>,
Stats::Store& stats() override { return stats_store_; }
Grpc::Context& grpcContext() override { return grpc_context_; }
Http::Context& httpContext() override { return http_context_; }
OptProcessContextRef processContext() override { return absl::nullopt; }
ProcessContextOptRef processContext() override { return absl::nullopt; }
ThreadLocal::Instance& threadLocal() override { return thread_local_; }
const LocalInfo::LocalInfo& localInfo() const override { return *local_info_; }
TimeSource& timeSource() override { return api_->timeSource(); }
Expand Down
4 changes: 2 additions & 2 deletions source/server/filter_chain_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ ServerLifecycleNotifier& FilterChainFactoryContextImpl::lifecycleNotifier() {
return parent_context_.lifecycleNotifier();
}

OptProcessContextRef FilterChainFactoryContextImpl::processContext() {
ProcessContextOptRef FilterChainFactoryContextImpl::processContext() {
return parent_context_.processContext();
}

Expand Down Expand Up @@ -614,7 +614,7 @@ Api::Api& FactoryContextImpl::api() { return server_.api(); }
ServerLifecycleNotifier& FactoryContextImpl::lifecycleNotifier() {
return server_.lifecycleNotifier();
}
OptProcessContextRef FactoryContextImpl::processContext() { return server_.processContext(); }
ProcessContextOptRef FactoryContextImpl::processContext() { return server_.processContext(); }
Configuration::ServerFactoryContext& FactoryContextImpl::getServerFactoryContext() const {
return server_.serverFactoryContext();
}
Expand Down
Loading