Unverified Commit e0118b4e authored by Kenton Varda's avatar Kenton Varda Committed by GitHub

Merge pull request #732 from capnproto/todo-soons

Resolve or defer all TODO(soon)s
parents a5e1d78f ec9e6e05
......@@ -282,6 +282,9 @@ KJ_TEST("decode all types") {
CASE(R"({"structField":{"boolField":true}})", root.getStructField().getBoolField() == true);
CASE(R"({"enumField":"bar"})", root.getEnumField() == TestEnum::BAR);
CASE_NO_ROUNDTRIP(R"({"textField":"foo\u1234bar"})",
kj::str("foo\u1234bar") == root.getTextField());
CASE_THROW_RECOVERABLE(R"({"structField":null})", "Expected object value");
CASE_THROW_RECOVERABLE(R"({"structList":null})", "Expected list value");
CASE_THROW_RECOVERABLE(R"({"boolList":null})", "Expected list value");
......
......@@ -114,5 +114,3 @@ enum TestJsonAnnotatedEnum {
baz @2 $Json.name("renamed-baz");
qux @3;
}
# TODO(now): enums
......@@ -214,8 +214,8 @@ kj::String JsonCodec::encodeRaw(JsonValue::Reader value) const {
}
void JsonCodec::encode(DynamicValue::Reader input, Type type, JsonValue::Builder output) const {
// TODO(soon): For interfaces, check for handlers on superclasses, per documentation...
// TODO(soon): For branded types, should we check for handlers on the generic?
// TODO(someday): For interfaces, check for handlers on superclasses, per documentation...
// TODO(someday): For branded types, should we check for handlers on the generic?
// TODO(someday): Allow registering handlers for "all structs", "all lists", etc?
KJ_IF_MAYBE(handler, impl->typeHandlers.find(type)) {
(*handler)->encodeBase(*this, input, output);
......@@ -828,9 +828,13 @@ private:
}
}
// TODO(soon): Support at least basic multi-lingual plane, ie ignore surrogates.
KJ_REQUIRE(codePoint < 128, "non-ASCII unicode escapes are not supported (yet!)");
target.add(0x7f & static_cast<char>(codePoint));
if (codePoint < 128) {
target.add(0x7f & static_cast<char>(codePoint));
} else {
// TODO(perf): This is sorta malloc-heavy...
char16_t u = codePoint;
target.addAll(kj::decodeUtf16(kj::arrayPtr(&u, 1)));
}
}
const size_t maxNestingDepth;
......
......@@ -1910,8 +1910,10 @@ KJ_TEST("HttpClient WebSocket handshake") {
auto headerTable = tableBuilder.build();
FakeEntropySource entropySource;
HttpClientSettings clientSettings;
clientSettings.entropySource = entropySource;
auto client = newHttpClient(*headerTable, *pipe.ends[0], entropySource);
auto client = newHttpClient(*headerTable, *pipe.ends[0], clientSettings);
testWebSocketClient(waitScope, *headerTable, hMyHeader, *client);
......@@ -1936,8 +1938,10 @@ KJ_TEST("HttpClient WebSocket error") {
auto headerTable = tableBuilder.build();
FakeEntropySource entropySource;
HttpClientSettings clientSettings;
clientSettings.entropySource = entropySource;
auto client = newHttpClient(*headerTable, *pipe.ends[0], entropySource);
auto client = newHttpClient(*headerTable, *pipe.ends[0], clientSettings);
kj::HttpHeaders headers(*headerTable);
headers.set(hMyHeader, "foo");
......@@ -2455,7 +2459,9 @@ KJ_TEST("newHttpService from HttpClient WebSockets") {
{
HttpHeaderTable table;
FakeEntropySource entropySource;
auto backClient = newHttpClient(table, *backPipe.ends[0], entropySource);
HttpClientSettings clientSettings;
clientSettings.entropySource = entropySource;
auto backClient = newHttpClient(table, *backPipe.ends[0], clientSettings);
auto frontService = newHttpService(*backClient);
HttpServer frontServer(timer, table, *frontService);
auto listenTask = frontServer.listenHttp(kj::mv(frontPipe.ends[1]));
......@@ -2494,7 +2500,9 @@ KJ_TEST("newHttpService from HttpClient WebSockets disconnect") {
{
HttpHeaderTable table;
FakeEntropySource entropySource;
auto backClient = newHttpClient(table, *backPipe.ends[0], entropySource);
HttpClientSettings clientSettings;
clientSettings.entropySource = entropySource;
auto backClient = newHttpClient(table, *backPipe.ends[0], clientSettings);
auto frontService = newHttpService(*backClient);
HttpServer frontServer(timer, table, *frontService);
auto listenTask = frontServer.listenHttp(kj::mv(frontPipe.ends[1]));
......@@ -2801,11 +2809,11 @@ KJ_TEST("HttpClient connection management") {
KJ_EXPECT(count == 0);
#if __linux__
// TODO(soon): Figure out why this doesn't work on Windows and is flakey on Mac. My guess is that
// the closing of the TCP connection propagates synchronously on Linux so that by the time we
// poll() the EventPort it reports the client end of the connection has reached EOF, whereas on
// Mac and Windows this propagation probably involves some concurrent process which may or may
// not complete before we poll(). A solution in this case would be to use a dummy in-memory
// TODO(someday): Figure out why this doesn't work on Windows and is flakey on Mac. My guess is
// that the closing of the TCP connection propagates synchronously on Linux so that by the time
// we poll() the EventPort it reports the client end of the connection has reached EOF, whereas
// on Mac and Windows this propagation probably involves some concurrent process which may or
// may not complete before we poll(). A solution in this case would be to use a dummy in-memory
// ConnectionReceiver that returns in-memory pipes (see UnbufferedPipe earlier in this file),
// so that we don't rely on any non-local behavior. Another solution would be to pause for
// a short time, maybe.
......
......@@ -1570,7 +1570,7 @@ kj::Own<kj::AsyncInputStream> HttpInputStream::getEntityBody(
KJ_IF_MAYBE(te, headers.get(HttpHeaderId::TRANSFER_ENCODING)) {
// TODO(someday): Support plugable transfer encodings? Or at least gzip?
// TODO(soon): Support stacked transfer encodings, e.g. "gzip, chunked".
// TODO(someday): Support stacked transfer encodings, e.g. "gzip, chunked".
if (fastCaseCmp<'c','h','u','n','k','e','d'>(te->cStr())) {
return kj::heap<HttpChunkedEntityReader>(*this);
} else {
......@@ -1588,7 +1588,7 @@ kj::Own<kj::AsyncInputStream> HttpInputStream::getEntityBody(
}
KJ_IF_MAYBE(c, headers.get(HttpHeaderId::CONNECTION)) {
// TODO(soon): Connection header can actually have multiple tokens... but no one ever uses
// TODO(someday): Connection header can actually have multiple tokens... but no one ever uses
// that feature?
if (fastCaseCmp<'c','l','o','s','e'>(c->cStr())) {
return kj::heap<HttpConnectionCloseEntityReader>(*this);
......
......@@ -152,7 +152,7 @@ public:
//
// HttpHeaderId::HOST
//
// TODO(soon): Fill this out with more common headers.
// TODO(someday): Fill this out with more common headers.
#define DECLARE_HEADER(id, name) \
static const HttpHeaderId id;
......@@ -637,12 +637,6 @@ kj::Own<HttpClient> newHttpClient(HttpHeaderTable& responseHeaderTable, kj::Asyn
// subsequent requests will fail. If a response takes a long time, it blocks subsequent responses.
// If a WebSocket is opened successfully, all subsequent requests fail.
kj::Own<HttpClient> newHttpClient(
HttpHeaderTable& responseHeaderTable, kj::AsyncIoStream& stream,
kj::Maybe<EntropySource&> entropySource) KJ_DEPRECATED("use HttpClientSettings");
// Temporary for backwards-compatibilty.
// TODO(soon): Remove this before next release.
kj::Own<HttpClient> newHttpClient(HttpService& service);
kj::Own<HttpService> newHttpService(HttpClient& client);
// Adapts an HttpClient to an HttpService and vice versa.
......@@ -802,12 +796,4 @@ inline void HttpHeaders::forEach(Func&& func) const {
}
}
inline kj::Own<HttpClient> newHttpClient(
HttpHeaderTable& responseHeaderTable, kj::AsyncIoStream& stream,
kj::Maybe<EntropySource&> entropySource) {
HttpClientSettings settings;
settings.entropySource = entropySource;
return newHttpClient(responseHeaderTable, stream, kj::mv(settings));
}
} // namespace kj
......@@ -183,7 +183,7 @@ public:
void shutdownWrite() override {
KJ_REQUIRE(shutdownTask == nullptr, "already called shutdownWrite()");
// TODO(soon): shutdownWrite() is problematic because it doesn't return a promise. It was
// TODO(0.8): shutdownWrite() is problematic because it doesn't return a promise. It was
// designed to assume that it would only be called after all writes are finished and that
// there was no reason to block at that point, but SSL sessions don't fit this since they
// actually have to send a shutdown message.
......
......@@ -72,8 +72,8 @@ constexpr auto NOT_SCHEME_CHARS = SCHEME_CHARS.invert();
constexpr auto HOST_CHARS = ALPHAS.orGroup(DIGITS).orAny(".-:[]_");
// [] is for ipv6 literals.
// _ is not allowed in domain names, but the WHATWG URL spec allows it in hostnames, so we do, too.
// TODO(soon): The URL spec actually allows a lot more than just '_', and requires nameprepping to
// Punycode. We'll have to decide how we want to deal with all that.
// TODO(someday): The URL spec actually allows a lot more than just '_', and requires nameprepping
// to Punycode. We'll have to decide how we want to deal with all that.
void toLower(String& text) {
for (char& c: text) {
......
......@@ -137,7 +137,7 @@ Exception::Type typeOfErrno(int error) {
Exception::Type typeOfWin32Error(DWORD error) {
switch (error) {
// TODO(soon): This needs more work.
// TODO(someday): This needs more work.
case WSAETIMEDOUT:
return Exception::Type::OVERLOADED;
......@@ -360,7 +360,7 @@ void Debug::Fault::init(
const char* file, int line, Win32Result osErrorNumber,
const char* condition, const char* macroArgs, ArrayPtr<String> argValues) {
LPVOID ptr;
// TODO(soon): Why doesn't this work for winsock errors?
// TODO(someday): Why doesn't this work for winsock errors?
DWORD result = FormatMessageW(FORMAT_MESSAGE_ALLOCATE_BUFFER |
FORMAT_MESSAGE_FROM_SYSTEM |
FORMAT_MESSAGE_IGNORE_INSERTS,
......
......@@ -729,7 +729,7 @@ KJ_TEST("DiskDirectory createTemporary") {
KJ_EXPECT(dir->listNames() == nullptr);
}
#if !__CYGWIN__ // TODO(soon): Figure out why this doesn't work on Cygwin.
#if !__CYGWIN__ // TODO(someday): Figure out why this doesn't work on Cygwin.
KJ_TEST("DiskDirectory replaceSubdir()") {
TempDir tempDir;
auto dir = tempDir.get();
......
......@@ -1171,7 +1171,7 @@ public:
candidatePath,
GENERIC_READ | GENERIC_WRITE,
0,
NULL, // TODO(soon): makeSecAttr(WriteMode::PRIVATE), when it's implemented
NULL, // TODO(someday): makeSecAttr(WriteMode::PRIVATE), when it's implemented
CREATE_NEW,
FILE_ATTRIBUTE_TEMPORARY | FILE_FLAG_DELETE_ON_CLOSE,
NULL);
......
......@@ -119,7 +119,7 @@ TEST(Mutex, MutexGuarded) {
EXPECT_EQ(321u, value.getWithoutLock());
}
#if KJ_USE_FUTEX // TODO(soon): Implement on pthread & win32
#if KJ_USE_FUTEX // TODO(someday): Implement on pthread & win32
TEST(Mutex, When) {
MutexGuarded<uint> value(123);
......
......@@ -66,7 +66,7 @@ public:
// non-trivial, assert that the mutex is locked (which should be good enough to catch problems
// in unit tests). In non-debug builds, do nothing.
#if KJ_USE_FUTEX // TODO(soon): Implement on pthread & win32
#if KJ_USE_FUTEX // TODO(someday): Implement on pthread & win32
class Predicate {
public:
virtual bool check() = 0;
......@@ -263,7 +263,7 @@ public:
inline T& getAlreadyLockedExclusive() const;
// Like `getWithoutLock()`, but asserts that the lock is already held by the calling thread.
#if KJ_USE_FUTEX // TODO(soon): Implement on pthread & win32
#if KJ_USE_FUTEX // TODO(someday): Implement on pthread & win32
template <typename Cond, typename Func>
auto when(Cond&& condition, Func&& callback) const -> decltype(callback(instance<T&>())) {
// Waits until condition(state) returns true, then calls callback(state) under lock.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment