Skip to content

Commit 53f0da1

Browse files
authored
Switch workerd/util to idiomatic comment style (#901)
1 parent 724b7dd commit 53f0da1

16 files changed

+322
-319
lines changed

src/workerd/util/abortable.h

+12-12
Original file line numberDiff line numberDiff line change
@@ -42,16 +42,16 @@ class AbortableImpl final {
4242
RefcountedCanceler::Listener onCancel;
4343
};
4444

45+
// An InputStream that can be disconnected in response to RefcountedCanceler.
46+
// This is similar to NeuterableInputStream in global-scope.c++ but uses an
47+
// external kj::Canceler to trigger the disconnect.
48+
// This is currently only used in fetch() requests that use an AbortSignal.
49+
// The AbortableInputStream is created using a RefcountedCanceler,
50+
// which will be triggered when the AbortSignal is triggered.
51+
// TODO(later): It would be good to see if both this and NeuterableInputStream
52+
// could be combined into a single utility.
4553
class AbortableInputStream final: public kj::AsyncInputStream,
4654
public kj::Refcounted {
47-
// An InputStream that can be disconnected in response to RefcountedCanceler.
48-
// This is similar to NeuterableInputStream in global-scope.c++ but uses an
49-
// external kj::Canceler to trigger the disconnect.
50-
// This is currently only used in fetch() requests that use an AbortSignal.
51-
// The AbortableInputStream is created using a RefcountedCanceler,
52-
// which will be triggered when the AbortSignal is triggered.
53-
// TODO(later): It would be good to see if both this and NeuterableInputStream
54-
// could be combined into a single utility.
5555
public:
5656
AbortableInputStream(kj::Own<kj::AsyncInputStream> inner, RefcountedCanceler& canceler)
5757
: impl(kj::mv(inner), canceler) {}
@@ -76,12 +76,12 @@ class AbortableInputStream final: public kj::AsyncInputStream,
7676
AbortableImpl<kj::AsyncInputStream> impl;
7777
};
7878

79+
// A WebSocket wrapper that can be disconnected in response to a RefcountedCanceler.
80+
// This is currently only used when opening a WebSocket with a fetch() request that
81+
// is using an AbortSignal. The AbortableWebSocket is created using the AbortSignal's
82+
// RefcountedCanceler, which will be triggered when the AbortSignal is triggered.
7983
class AbortableWebSocket final: public kj::WebSocket,
8084
public kj::Refcounted {
81-
// A WebSocket wrapper that can be disconnected in response to a RefcountedCanceler.
82-
// This is currently only used when opening a WebSocket with a fetch() request that
83-
// is using an AbortSignal. The AbortableWebSocket is created using the AbortSignal's
84-
// RefcountedCanceler, which will be triggered when the AbortSignal is triggered.
8585
public:
8686
AbortableWebSocket(kj::Own<kj::WebSocket> inner, RefcountedCanceler& canceler)
8787
: impl(kj::mv(inner), canceler) {}

src/workerd/util/batch-queue.h

+31-36
Original file line numberDiff line numberDiff line change
@@ -12,38 +12,36 @@ namespace workerd {
1212

1313
using kj::uint;
1414

15+
// A double-buffered batch queue which enforces an upper bound on buffer growth.
16+
//
17+
// Objects of this type have two buffers -- the push buffer and the pop buffer -- and support
18+
// `push()` and `pop()` operations. `push()` adds elements to the push buffer. `pop()` swaps the
19+
// push and the pop buffers and returns a RAII object which provides a view onto the pop buffer.
20+
// When the RAII object is destroyed, it resets the size and capacity of the pop buffer.
21+
//
22+
// This class is useful when the cost of context switching between producers and consumers is
23+
// high and/or when you must be able to gracefully handle bursts of pushes, such as when
24+
// transferring objects between threads. Note that this class implements no cross-thread
25+
// synchronization itself, but it can become an effective multiple-producer, single-consumer queue
26+
// when wrapped as a `kj::MutexGuarded<BatchQueue<T>>`.
1527
template <typename T>
1628
class BatchQueue {
17-
// A double-buffered batch queue which enforces an upper bound on buffer growth.
18-
//
19-
// Objects of this type have two buffers -- the push buffer and the pop buffer -- and support
20-
// `push()` and `pop()` operations. `push()` adds elements to the push buffer. `pop()` swaps the
21-
// push and the pop buffers and returns a RAII object which provides a view onto the pop buffer.
22-
// When the RAII object is destroyed, it resets the size and capacity of the pop buffer.
23-
//
24-
// This class is useful when the cost of context switching between producers and consumers is
25-
// high and/or when you must be able to gracefully handle bursts of pushes, such as when
26-
// transferring objects between threads. Note that this class implements no cross-thread
27-
// synchronization itself, but it can become an effective multiple-producer, single-consumer queue
28-
// when wrapped as a `kj::MutexGuarded<BatchQueue<T>>`.
29-
3029
public:
30+
// `initialCapacity` is the number of elements of type T for which we should allocate space in the
31+
// initial buffers, and any reconstructed buffers. Buffers will be reconstructed if they are
32+
// observed to grow beyond `maxCapacity` after a completed pop operation.
3133
explicit BatchQueue(uint initialCapacity, uint maxCapacity)
3234
: pushBuffer(initialCapacity),
3335
popBuffer(initialCapacity),
3436
initialCapacity(initialCapacity),
3537
maxCapacity(maxCapacity) {}
36-
// `initialCapacity` is the number of elements of type T for which we should allocate space in the
37-
// initial buffers, and any reconstructed buffers. Buffers will be reconstructed if they are
38-
// observed to grow beyond `maxCapacity` after a completed pop operation.
3938

39+
// This is the return type of `pop()` (in fact, `pop()` is the only way to construct a non-empty
40+
// Batch). Default-constructible, moveable, and non-copyable.
41+
//
42+
// A Batch can be converted to an ArrayPtr<T>. When a Batch is destroyed, it clears the pop
43+
// buffer and resets the pop buffer capacity to `initialCapacity` if necessary.
4044
class Batch {
41-
// This is the return type of `pop()` (in fact, `pop()` is the only way to construct a non-empty
42-
// Batch). Default-constructible, moveable, and non-copyable.
43-
//
44-
// A Batch can be converted to an ArrayPtr<T>. When a Batch is destroyed, it clears the pop
45-
// buffer and resets the pop buffer capacity to `initialCapacity` if necessary.
46-
4745
public:
4846
Batch() = default;
4947
Batch(Batch&&) = default;
@@ -67,18 +65,17 @@ class BatchQueue {
6765
// It's a Maybe so we can support move operations.
6866
};
6967

68+
// If a batch is available, swap the buffers and return a Batch object backed by the pop buffer.
69+
// The caller should destroy the Batch object as soon as they are done with it. Destruction will
70+
// clear the pop buffer and, if necessary, reconstruct it to stay under `maxCapacity`.
71+
//
72+
// Throws if `pop()` is called again before the previous Batch object was destroyed. Note
73+
// that this exception is only reliable if the previous `pop()` returned a non-empty Batch.
74+
//
75+
// `pop()` accesses both buffers, so it must be synchronized with `push()` operations across
76+
// threads. Batch objects and `push()` access different buffers, so they require no explicit
77+
// cross-thread synchronization with each other.
7078
Batch pop() {
71-
// If a batch is available, swap the buffers and return a Batch object backed by the pop buffer.
72-
// The caller should destroy the Batch object as soon as they are done with it. Destruction will
73-
// clear the pop buffer and, if necessary, reconstruct it to stay under `maxCapacity`.
74-
//
75-
// Throws if `pop()` is called again before the previous Batch object was destroyed. Note
76-
// that this exception is only reliable if the previous `pop()` returned a non-empty Batch.
77-
//
78-
// `pop()` accesses both buffers, so it must be synchronized with `push()` operations across
79-
// threads. Batch objects and `push()` access different buffers, so they require no explicit
80-
// cross-thread synchronization with each other.
81-
8279
KJ_REQUIRE(popBuffer.empty(), "pop()'s previous result not yet destroyed.");
8380

8481
Batch batch;
@@ -91,16 +88,14 @@ class BatchQueue {
9188
return batch;
9289
}
9390

91+
// Add an item to the current batch.
9492
template <typename U>
9593
void push(U&& value) {
96-
// Add an item to the current batch.
97-
9894
pushBuffer.add(kj::fwd<U>(value));
9995
}
10096

10197
auto empty() const { return pushBuffer.empty(); }
10298
auto size() const { return pushBuffer.size(); }
103-
// Push buffer metadata.
10499

105100
private:
106101
kj::Vector<T> pushBuffer;

src/workerd/util/canceler.h

+7-7
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,14 @@
1111

1212
namespace workerd {
1313

14+
// A simple wrapper around kj::Canceler that can be safely
15+
// shared by multiple objects. This is used, for instance,
16+
// to support fetch() requests that use an AbortSignal.
17+
// The AbortSignal (see api/basics.h) creates an instance
18+
// of RefcountedCanceler then passes references to it out
19+
// to various other objects that will use it to wrap their
20+
// Promises.
1421
class RefcountedCanceler: public kj::Refcounted {
15-
// A simple wrapper around kj::Canceler that can be safely
16-
// shared by multiple objects. This is used, for instance,
17-
// to support fetch() requests that use an AbortSignal.
18-
// The AbortSignal (see api/basics.h) creates an instance
19-
// of RefcountedCanceler then passes references to it out
20-
// to various other objects that will use it to wrap their
21-
// Promises.
2222
public:
2323
class Listener {
2424
public:

src/workerd/util/capnp-mock.h

+20-26
Original file line numberDiff line numberDiff line change
@@ -102,11 +102,10 @@ class MockClient: public capnp::DynamicCapability::Client {
102102
}
103103
};
104104

105+
// Infrastructure to mock a capability!
106+
//
107+
// TODO(cleanup): This should obviously go in Cap'n Proto!
105108
class MockServer: public kj::Refcounted {
106-
// Infrastructure to mock a capability!
107-
//
108-
// TODO(cleanup): This should obviously go in Cap'n Proto!
109-
110109
struct ReceivedCall;
111110
public:
112111
MockServer(capnp::InterfaceSchema schema): schema(schema) {}
@@ -156,36 +155,33 @@ class MockServer: public kj::Refcounted {
156155
return kj::mv(*this);
157156
}
158157

158+
// Helper for cases where the received call is expected to invoke some callback capability.
159+
//
160+
// Expect that the params contain a field named `callbackName` whose type is an interface.
161+
// `func()` will be invoked and passed a `MockClient` representing this capability. It can
162+
// then invoke the callback as it seems fit.
163+
//
164+
// Note that it's explicitly OK if `func` captures a `WaitScope` and uses it. In this way,
165+
// the incoming call can be delayed from returning until the callback completes.
159166
template <typename Func>
160167
ExpectedCall useCallback(kj::StringPtr callbackName, Func&& func,
161168
kj::SourceLocation location = {}) && KJ_WARN_UNUSED_RESULT {
162-
// Helper for cases where the received call is expected to invoke some callback capability.
163-
//
164-
// Expect that the params contain a field named `callbackName` whose type is an interface.
165-
// `func()` will be invoked and passed a `MockClient` representing this capability. It can
166-
// then invoke the callback as it seems fit.
167-
//
168-
// Note that it's explicitly OK if `func` captures a `WaitScope` and uses it. In this way,
169-
// the incoming call can be delayed from returning until the callback completes.
170-
171169
auto& received = getReceived(location);
172170
func(received.context.getParams().get(callbackName).as<capnp::DynamicCapability>());
173171
return kj::mv(*this);
174172
}
175173

174+
// Causes the method to return the given result message, which is parsed from text.
176175
void thenReturn(kj::StringPtr message, kj::SourceLocation location = {}) && {
177-
// Causes the method to return the given result message, which is parsed from text.
178-
179176
auto& received = getReceived(location);
180177
TEXT_CODEC.decode(message, received.context.getResults());
181178
received.fulfiller.fulfill();
182179
}
183180

181+
// Causes the method to return the given result message, which is parsed from text.
182+
// All capabilities in the result message will be filled in, with MockServer instances
183+
// returned in the hashmap.
184184
kj::HashMap<kj::String, kj::Own<MockServer>> thenReturnWithMocks(kj::StringPtr message, kj::SourceLocation location = {}) && {
185-
// Causes the method to return the given result message, which is parsed from text.
186-
// All capabilities in the result message will be filled in, with MockServer instances
187-
// returned in the hashmap.
188-
189185
auto& received = getReceived(location);
190186
auto callResults = received.context.getResults();
191187
auto results = kj::HashMap<kj::String, kj::Own<MockServer>>();
@@ -203,18 +199,16 @@ class MockServer: public kj::Refcounted {
203199
return kj::mv(results);
204200
}
205201

202+
// Causes the method to throw an exception
206203
void thenThrow(kj::Exception&& e, kj::SourceLocation location = {}) && {
207-
// Causes the method to throw an exception
208-
209204
auto& received = getReceived(location);
210205
received.fulfiller.reject(kj::mv(e));
211206
}
212207

208+
// Return a new mock capability. The method result type is expected to contain a single
209+
// field with the given name whose type is an interface type. It will be filled in with a
210+
// new mock object, and the MockServer is returned in order to set further expectations.
213211
kj::Own<MockServer> returnMock(kj::StringPtr fieldName, kj::SourceLocation location = {}) && {
214-
// Return a new mock capability. The method result type is expected to contain a single
215-
// field with the given name whose type is an interface type. It will be filled in with a
216-
// new mock object, and the MockServer is returned in order to set further expectations.
217-
218212
auto& received = getReceived(location);
219213
auto field = received.method.getResultType().getFieldByName(fieldName);
220214
auto result = kj::refcounted<MockServer>(field.getType().asInterface());
@@ -348,10 +342,10 @@ class MockServer: public kj::Refcounted {
348342
};
349343
};
350344

351-
#define CAPNP(...) ("(" #__VA_ARGS__ ")"_kj)
352345
// Wraps a "capnp struct literal". This actually just stringifies the arguments, adding enclosing
353346
// parentheses. The nice thing about it, though, is that you don't have to escape quotes inside
354347
// the literal.
348+
#define CAPNP(...) ("(" #__VA_ARGS__ ")"_kj)
355349

356350
template<typename Schema, typename InitFunc>
357351
kj::String Capnp(InitFunc func) {

src/workerd/util/http-util.h

+4-6
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,10 @@
88

99
namespace workerd {
1010

11+
// Attaches the given object to a `Request` so that it lives as long as the request's properties.
12+
// The given object must support `kj::addRef()` (e.g. `kj::Refcount`).
1113
template<typename T>
1214
kj::HttpClient::Request attachToRequest(kj::HttpClient::Request req, T&& rcAttachment) {
13-
// Attaches the given object to a `Request` so that it lives as long as the request's properties.
14-
// The given object must support `kj::addRef()` (e.g. `kj::Refcount`).
15-
1615
req.body = req.body.attach(kj::addRef(*rcAttachment));
1716
req.response = req.response
1817
.then([rcAttachment = kj::mv(rcAttachment)](kj::HttpClient::Response&& response) mutable {
@@ -22,12 +21,11 @@ kj::HttpClient::Request attachToRequest(kj::HttpClient::Request req, T&& rcAttac
2221
return req;
2322
}
2423

24+
// Attaches the given object to a `WebSocketResponse` promise so that it lives as long as the
25+
// returned response's properties.
2526
template<typename T>
2627
kj::Promise<kj::HttpClient::WebSocketResponse> attachToWebSocketResponse(
2728
kj::Promise<kj::HttpClient::WebSocketResponse> promise, T&& attachment) {
28-
// Attaches the given object to a `WebSocketResponse` promise so that it lives as long as the
29-
// returned response's properties.
30-
3129
return promise.then([attachment = kj::mv(attachment)](
3230
kj::HttpClient::WebSocketResponse&& response) mutable {
3331
KJ_SWITCH_ONEOF(response.webSocketOrBody) {

src/workerd/util/mimetype.h

+9-9
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,16 @@ class MimeType final {
1919
IGNORE_PARAMS,
2020
};
2121

22-
static kj::Maybe<MimeType> tryParse(kj::StringPtr input,
23-
ParseOptions options = ParseOptions::DEFAULT);
2422
// Returning nullptr implies that the input is not a valid mime type construction.
2523
// If the ParseOptions::IGNORE_PARAMS option is set then the mime type parameters
2624
// will be ignored and will not be included in the parsed result.
25+
static kj::Maybe<MimeType> tryParse(kj::StringPtr input,
26+
ParseOptions options = ParseOptions::DEFAULT);
2727

28-
static MimeType parse(kj::StringPtr input,
29-
ParseOptions options = ParseOptions::DEFAULT);
3028
// Asserts if the input could not be parsed as a valid MimeType. tryParse should
3129
// be preferred for most cases.
30+
static MimeType parse(kj::StringPtr input,
31+
ParseOptions options = ParseOptions::DEFAULT);
3232

3333
explicit MimeType(kj::StringPtr type,
3434
kj::StringPtr subtype,
@@ -48,25 +48,25 @@ class MimeType final {
4848
bool addParam(kj::ArrayPtr<const char> name, kj::ArrayPtr<const char> value);
4949
void eraseParam(kj::StringPtr name);
5050

51-
kj::String essence() const;
5251
// Returns only the type/subtype
52+
kj::String essence() const;
5353

54-
kj::String toString() const;
5554
// Returns the type/subtype and all params.
55+
kj::String toString() const;
5656

5757
kj::String paramsToString() const;
5858

59-
MimeType clone(ParseOptions options = ParseOptions::DEFAULT) const;
6059
// Copy this MimeType. If the IGNORE_PARAMS option is set the clone
6160
// will copy only the type and subtype and will omit all of the parameters.
61+
MimeType clone(ParseOptions options = ParseOptions::DEFAULT) const;
6262

63-
bool operator==(const MimeType& other) const;
6463
// Compares only the essence of the MimeType (type and subtype). Ignores
6564
// parameters in the comparison.
65+
bool operator==(const MimeType& other) const;
6666

67-
bool operator!=(const MimeType& other) const;
6867
// Compares only the essence of the MimeType (type and subtype). Ignores
6968
// parameters in the comparison.
69+
bool operator!=(const MimeType& other) const;
7070

7171
operator kj::String() const;
7272

0 commit comments

Comments
 (0)