sammccall updated this revision to Diff 124203.
sammccall marked 2 inline comments as done.
sammccall added a comment.
Renames to address review comments
https://reviews.llvm.org/D40399
Files:
clangd/ClangdUnit.cpp
clangd/JSONExpr.cpp
clangd/JSONExpr.h
clangd/JSONRPCDispatcher.cpp
clangd/JSONRPCDispatcher.h
clangd/Trace.cpp
clangd/Trace.h
clangd/tool/ClangdMain.cpp
test/clangd/trace.test
unittests/clangd/JSONExprTests.cpp
unittests/clangd/TraceTests.cpp
Index: unittests/clangd/TraceTests.cpp
===================================================================
--- unittests/clangd/TraceTests.cpp
+++ unittests/clangd/TraceTests.cpp
@@ -116,7 +116,7 @@
ASSERT_NE(++Event, Events->end()) << "Expected span start";
EXPECT_TRUE(VerifyObject(*Event, {{"ph", "B"}, {"name", "A"}}));
ASSERT_NE(++Event, Events->end()) << "Expected log message";
- EXPECT_TRUE(VerifyObject(*Event, {{"ph", "i"}, {"name", "B"}}));
+ EXPECT_TRUE(VerifyObject(*Event, {{"ph", "i"}, {"name", "Log"}}));
ASSERT_NE(++Event, Events->end()) << "Expected span end";
EXPECT_TRUE(VerifyObject(*Event, {{"ph", "E"}}));
ASSERT_EQ(++Event, Events->end());
Index: unittests/clangd/JSONExprTests.cpp
===================================================================
--- unittests/clangd/JSONExprTests.cpp
+++ unittests/clangd/JSONExprTests.cpp
@@ -167,7 +167,6 @@
ExpectErr("Unexpected EOF", "");
ExpectErr("Unexpected EOF", "[");
ExpectErr("Text after end of document", "[][]");
- ExpectErr("Text after end of document", "[][]");
ExpectErr("Invalid bareword", "fuzzy");
ExpectErr("Expected , or ]", "[2?]");
ExpectErr("Expected object key", "{a:2}");
@@ -185,6 +184,49 @@
})");
}
+TEST(JSONTest, Inspection) {
+ llvm::Expected<Expr> Doc = parse(R"(
+ {
+ "null": null,
+ "boolean": false,
+ "number": 2.78,
+ "string": "json",
+ "array": [null, true, 3.14, "hello", [1,2,3], {"time": "arrow"}],
+ "object": {"fruit": "banana"}
+ }
+ )");
+ EXPECT_TRUE(!!Doc);
+
+ obj *O = Doc->asObject();
+ ASSERT_TRUE(O);
+
+ EXPECT_FALSE(O->getNull("missing"));
+ EXPECT_FALSE(O->getNull("boolean"));
+ EXPECT_TRUE(O->getNull("null"));
+
+ EXPECT_EQ(O->getNumber("number"), llvm::Optional<double>(2.78));
+ EXPECT_EQ(O->getString("string"), llvm::Optional<llvm::StringRef>("json"));
+ ASSERT_FALSE(O->getObject("missing"));
+ ASSERT_FALSE(O->getObject("array"));
+ ASSERT_TRUE(O->getObject("object"));
+ EXPECT_EQ(*O->getObject("object"), (obj{{"fruit", "banana"}}));
+
+ ary *A = O->getArray("array");
+ ASSERT_TRUE(A);
+ EXPECT_EQ(A->getBoolean(1), llvm::Optional<bool>(true));
+ ASSERT_TRUE(A->getArray(4));
+ EXPECT_EQ(*A->getArray(4), (ary{1, 2, 3}));
+ int I = 0;
+ for (Expr &E : *A) {
+ if (I++ == 5) {
+ ASSERT_TRUE(E.asObject());
+ EXPECT_EQ(E.asObject()->getString("time"),
+ llvm::Optional<llvm::StringRef>("arrow"));
+ } else
+ EXPECT_FALSE(E.asObject());
+ }
+}
+
} // namespace
} // namespace json
} // namespace clangd
Index: test/clangd/trace.test
===================================================================
--- test/clangd/trace.test
+++ test/clangd/trace.test
@@ -1,4 +1,4 @@
-# RUN: clangd -run-synchronously -trace %t < %s && FileCheck %s < %t
+# RUN: clangd -pretty -run-synchronously -trace %t < %s && FileCheck %s < %t
# It is absolutely vital that this file has CRLF line endings.
#
Content-Length: 125
@@ -8,9 +8,19 @@
Content-Length: 152
{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///foo.c","languageId":"c","version":1,"text":"void main() {}"}}}
-# CHECK: "textDocument/didOpen"
-# CHECK: "Preamble: /foo.c"
-# CHECK: "Build: /foo.c"
+# CHECK: {"displayTimeUnit":"ns","traceEvents":[
+# Start opening the doc.
+# CHECK: "name": "textDocument/didOpen"
+# CHECK: "ph": "E"
+# Start building the preamble.
+# CHECK: "name": "Preamble"
+# CHECK: },
+# Finish building the preamble, with filename.
+# CHECK: "File": "/foo.c"
+# CHECK-NEXT: },
+# CHECK-NEXT: "ph": "E"
+# Start building the file.
+# CHECK: "name": "Build"
Content-Length: 44
{"jsonrpc":"2.0","id":5,"method":"shutdown"}
Index: clangd/tool/ClangdMain.cpp
===================================================================
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -124,7 +124,7 @@
TraceFile.reset();
llvm::errs() << "Error while opening trace file: " << EC.message();
} else {
- TraceSession = trace::Session::create(*TraceStream);
+ TraceSession = trace::Session::create(*TraceStream, PrettyPrint);
}
}
Index: clangd/Trace.h
===================================================================
--- clangd/Trace.h
+++ clangd/Trace.h
@@ -21,6 +21,7 @@
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_TRACE_H_
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_TRACE_H_
+#include "JSONExpr.h"
#include "llvm/ADT/Twine.h"
#include "llvm/Support/raw_ostream.h"
@@ -35,7 +36,8 @@
class Session {
public:
// Starts a sessions capturing trace events and writing Trace Event JSON.
- static std::unique_ptr<Session> create(llvm::raw_ostream &OS);
+ static std::unique_ptr<Session> create(llvm::raw_ostream &OS,
+ bool Pretty = false);
~Session();
private:
@@ -46,14 +48,30 @@
void log(const llvm::Twine &Name);
// Records an event whose duration is the lifetime of the Span object.
+//
+// Arbitrary JSON metadata can be attached while this span is active:
+// SPAN_ATTACH(MySpan, "Payload", SomeJSONExpr);
+// SomeJSONExpr is evaluated and copied only if actually needed.
class Span {
public:
- Span(const llvm::Twine &Name);
+ Span(std::string Name);
~Span();
+ // Returns mutable span metadata if this span is interested.
+ // Prefer to use SPAN_ATTACH rather than accessing this directly.
+ json::obj *args() { return Args.get(); }
+
private:
+ std::unique_ptr<json::obj> Args;
};
+#define SPAN_ATTACH(S, Name, Expr) \
+ do { \
+ if ((S).args() != nullptr) { \
+ (*((S).args()))[(Name)] = (Expr); \
+ } \
+ } while (0)
+
} // namespace trace
} // namespace clangd
} // namespace clang
Index: clangd/Trace.cpp
===================================================================
--- clangd/Trace.cpp
+++ clangd/Trace.cpp
@@ -27,13 +27,17 @@
// Perhaps we should replace this by something that disturbs performance less.
class Tracer {
public:
- Tracer(raw_ostream &Out)
- : Out(Out), Sep(""), Start(std::chrono::system_clock::now()) {
+ Tracer(raw_ostream &Out, bool Pretty)
+ : Out(Out), Sep(""), Start(std::chrono::system_clock::now()),
+ JSONFormat(Pretty ? "{0:2}" : "{0}") {
// The displayTimeUnit must be ns to avoid low-precision overlap
// calculations!
Out << R"({"displayTimeUnit":"ns","traceEvents":[)"
<< "\n";
- rawEvent("M", R"("name": "process_name", "args":{"name":"clangd"})");
+ rawEvent("M", json::obj{
+ {"name", "process_name"},
+ {"args", json::obj{{"name", "clangd"}}},
+ });
}
~Tracer() {
@@ -43,32 +47,34 @@
// Record an event on the current thread. ph, pid, tid, ts are set.
// Contents must be a list of the other JSON key/values.
- template <typename T> void event(StringRef Phase, const T &Contents) {
+ void event(StringRef Phase, json::obj &&Contents) {
uint64_t TID = get_threadid();
std::lock_guard<std::mutex> Lock(Mu);
// If we haven't already, emit metadata describing this thread.
if (ThreadsWithMD.insert(TID).second) {
SmallString<32> Name;
get_thread_name(Name);
if (!Name.empty()) {
- rawEvent(
- "M",
- formatv(
- R"("tid": {0}, "name": "thread_name", "args":{"name":"{1}"})",
- TID, StringRef(&Name[0], Name.size())));
+ rawEvent("M", json::obj{
+ {"tid", TID},
+ {"name", "thread_name"},
+ {"args", json::obj{{"name", Name}}},
+ });
}
}
- rawEvent(Phase, formatv(R"("ts":{0}, "tid":{1}, {2})", timestamp(), TID,
- Contents));
+ Contents["ts"] = timestamp();
+ Contents["tid"] = TID;
+ rawEvent(Phase, std::move(Contents));
}
private:
// Record an event. ph and pid are set.
// Contents must be a list of the other JSON key/values.
- template <typename T>
- void rawEvent(StringRef Phase, const T &Contents) /*REQUIRES(Mu)*/ {
+ void rawEvent(StringRef Phase, json::obj &&Event) /*REQUIRES(Mu)*/ {
// PID 0 represents the clangd process.
- Out << Sep << R"({"pid":0, "ph":")" << Phase << "\", " << Contents << "}";
+ Event["pid"] = 0;
+ Event["ph"] = Phase;
+ Out << Sep << formatv(JSONFormat, json::Expr(std::move(Event)));
Sep = ",\n";
}
@@ -82,14 +88,15 @@
const char *Sep /*GUARDED_BY(Mu)*/;
DenseSet<uint64_t> ThreadsWithMD /*GUARDED_BY(Mu)*/;
const sys::TimePoint<> Start;
+ const char *JSONFormat;
};
static Tracer *T = nullptr;
} // namespace
-std::unique_ptr<Session> Session::create(raw_ostream &OS) {
+std::unique_ptr<Session> Session::create(raw_ostream &OS, bool Pretty) {
assert(!T && "A session is already active");
- T = new Tracer(OS);
+ T = new Tracer(OS, Pretty);
return std::unique_ptr<Session>(new Session());
}
@@ -101,19 +108,25 @@
void log(const Twine &Message) {
if (!T)
return;
- T->event("i", formatv(R"("name":"{0}")", yaml::escape(Message.str())));
+ T->event("i", json::obj{
+ {"name", "Log"},
+ {"args", json::obj{{"Message", Message.str()}}},
+ });
}
-Span::Span(const Twine &Text) {
+Span::Span(std::string Name) {
if (!T)
return;
- T->event("B", formatv(R"("name":"{0}")", yaml::escape(Text.str())));
+ T->event("B", json::obj{{"name", std::move(Name)}});
+ Args = llvm::make_unique<json::obj>();
}
Span::~Span() {
if (!T)
return;
- T->event("E", R"("_":0)" /* Dummy property to ensure valid JSON */);
+ if (!Args)
+ Args = llvm::make_unique<json::obj>();
+ T->event("E", Args ? json::obj{{"args", std::move(*Args)}} : json::obj{});
}
} // namespace trace
Index: clangd/JSONRPCDispatcher.h
===================================================================
--- clangd/JSONRPCDispatcher.h
+++ clangd/JSONRPCDispatcher.h
@@ -13,6 +13,7 @@
#include "JSONExpr.h"
#include "Logger.h"
#include "Protocol.h"
+#include "Trace.h"
#include "clang/Basic/LLVM.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringMap.h"
@@ -55,19 +56,27 @@
/// Context object passed to handlers to allow replies.
class RequestContext {
public:
- RequestContext(JSONOutput &Out, llvm::Optional<json::Expr> ID)
- : Out(Out), ID(std::move(ID)) {}
+ RequestContext(JSONOutput &Out, StringRef Method,
+ llvm::Optional<json::Expr> ID)
+ : Out(Out), ID(std::move(ID)),
+ Tracer(llvm::make_unique<trace::Span>(Method)) {
+ if (this->ID)
+ SPAN_ATTACH(tracer(), "ID", *this->ID);
+ }
/// Sends a successful reply.
void reply(json::Expr &&Result);
/// Sends an error response to the client, and logs it.
void replyError(ErrorCode code, const llvm::StringRef &Message);
/// Sends a request to the client.
void call(llvm::StringRef Method, json::Expr &&Params);
+ trace::Span &tracer() { return *Tracer; }
+
private:
JSONOutput &Out;
llvm::Optional<json::Expr> ID;
+ std::unique_ptr<trace::Span> Tracer;
};
/// Main JSONRPC entry point. This parses the JSONRPC "header" and calls the
Index: clangd/JSONRPCDispatcher.cpp
===================================================================
--- clangd/JSONRPCDispatcher.cpp
+++ clangd/JSONRPCDispatcher.cpp
@@ -59,6 +59,7 @@
Out.log("Attempted to reply to a notification!\n");
return;
}
+ SPAN_ATTACH(tracer(), "Reply", Result);
Out.writeMessage(json::obj{
{"jsonrpc", "2.0"},
{"id", *ID},
@@ -69,6 +70,9 @@
void RequestContext::replyError(ErrorCode code,
const llvm::StringRef &Message) {
Out.log("Error " + Twine(static_cast<int>(code)) + ": " + Message + "\n");
+ SPAN_ATTACH(tracer(), "Error",
+ (json::obj{{"code", static_cast<int>(code)},
+ {"message", Message.str()}}));
if (ID) {
Out.writeMessage(json::obj{
{"jsonrpc", "2.0"},
@@ -82,6 +86,8 @@
void RequestContext::call(StringRef Method, json::Expr &&Params) {
// FIXME: Generate/Increment IDs for every request so that we can get proper
// replies once we need to.
+ SPAN_ATTACH(tracer(), "Call",
+ (json::obj{{"method", Method.str()}, {"params", Params}}));
Out.writeMessage(json::obj{
{"jsonrpc", "2.0"},
{"id", 1},
@@ -104,8 +110,7 @@
llvm::StringRef MethodStr = Method->getValue(MethodStorage);
auto I = Handlers.find(MethodStr);
auto &Handler = I != Handlers.end() ? I->second : UnknownHandler;
- trace::Span Tracer(MethodStr);
- Handler(RequestContext(Out, std::move(ID)), Params);
+ Handler(RequestContext(Out, MethodStr, std::move(ID)), Params);
}
bool JSONRPCDispatcher::call(StringRef Content, JSONOutput &Out) const {
@@ -249,7 +254,6 @@
std::vector<char> JSON(ContentLength + 1, '\0');
llvm::StringRef JSONRef;
{
- trace::Span Tracer("Reading request");
// Now read the JSON. Insert a trailing null byte as required by the
// YAML parser.
In.read(JSON.data(), ContentLength);
Index: clangd/JSONExpr.h
===================================================================
--- clangd/JSONExpr.h
+++ clangd/JSONExpr.h
@@ -55,14 +55,14 @@
// object (json::obj)
//
// The kind can be queried directly, or implicitly via the typed accessors:
-// if (Optional<StringRef> S = E.string())
+// if (Optional<StringRef> S = E.asString()
// assert(E.kind() == Expr::String);
//
// Array and Object also have typed indexing accessors for easy traversal:
// Expected<Expr> E = parse(R"( {"options": {"font": "sans-serif"}} )");
-// if (json::obj* O = E->object())
-// if (json::obj* Opts = O->object("options"))
-// if (Optional<StringRef> Font = Opts->string("font"))
+// if (json::obj* O = E->asObject())
+// if (json::obj* Opts = O->getObject("options"))
+// if (Optional<StringRef> Font = Opts->getString("font"))
// assert(Opts->at("font").kind() == Expr::String);
//
// === Serialization ===
@@ -166,38 +166,38 @@
}
// Typed accessors return None/nullptr if the Expr is not of this type.
- llvm::Optional<std::nullptr_t> null() const {
+ llvm::Optional<std::nullptr_t> asNull() const {
if (LLVM_LIKELY(Type == T_Null))
return nullptr;
return llvm::None;
}
- llvm::Optional<bool> boolean() const {
- if (LLVM_LIKELY(Type == T_Null))
+ llvm::Optional<bool> asBoolean() const {
+ if (LLVM_LIKELY(Type == T_Boolean))
return as<bool>();
return llvm::None;
}
- llvm::Optional<double> number() const {
+ llvm::Optional<double> asNumber() const {
if (LLVM_LIKELY(Type == T_Number))
return as<double>();
return llvm::None;
}
- llvm::Optional<llvm::StringRef> string() const {
+ llvm::Optional<llvm::StringRef> asString() const {
if (Type == T_String)
return llvm::StringRef(as<std::string>());
if (LLVM_LIKELY(Type == T_StringRef))
return as<llvm::StringRef>();
return llvm::None;
}
- const ObjectExpr *object() const {
+ const ObjectExpr *asObject() const {
return LLVM_LIKELY(Type == T_Object) ? &as<ObjectExpr>() : nullptr;
}
- ObjectExpr *object() {
+ ObjectExpr *asObject() {
return LLVM_LIKELY(Type == T_Object) ? &as<ObjectExpr>() : nullptr;
}
- const ArrayExpr *array() const {
+ const ArrayExpr *asArray() const {
return LLVM_LIKELY(Type == T_Array) ? &as<ArrayExpr>() : nullptr;
}
- ArrayExpr *array() {
+ ArrayExpr *asArray() {
return LLVM_LIKELY(Type == T_Array) ? &as<ArrayExpr>() : nullptr;
}
@@ -292,6 +292,63 @@
Expr &operator[](ObjectKey &&K) {
return emplace(std::move(K), Expr(nullptr)).first->second;
}
+
+ // Look up a property, returning nullptr if it doesn't exist.
+ json::Expr *get(const ObjectKey &K) {
+ auto I = find(K);
+ if (I == end())
+ return nullptr;
+ return &I->second;
+ }
+ const json::Expr *get(const ObjectKey &K) const {
+ auto I = find(K);
+ if (I == end())
+ return nullptr;
+ return &I->second;
+ }
+ // Typed accessors return None/nullptr if
+ // - the property doesn't exist
+ // - or it has the wrong type
+ llvm::Optional<std::nullptr_t> getNull(const ObjectKey &K) const {
+ if (auto *V = get(K))
+ return V->asNull();
+ return llvm::None;
+ }
+ llvm::Optional<bool> getBoolean(const ObjectKey &K) const {
+ if (auto *V = get(K))
+ return V->asBoolean();
+ return llvm::None;
+ }
+ llvm::Optional<double> getNumber(const ObjectKey &K) const {
+ if (auto *V = get(K))
+ return V->asNumber();
+ return llvm::None;
+ }
+ llvm::Optional<llvm::StringRef> getString(const ObjectKey &K) const {
+ if (auto *V = get(K))
+ return V->asString();
+ return llvm::None;
+ }
+ const ObjectExpr *getObject(const ObjectKey &K) const {
+ if (auto *V = get(K))
+ return V->asObject();
+ return nullptr;
+ }
+ ObjectExpr *getObject(const ObjectKey &K) {
+ if (auto *V = get(K))
+ return V->asObject();
+ return nullptr;
+ }
+ const ArrayExpr *getArray(const ObjectKey &K) const {
+ if (auto *V = get(K))
+ return V->asArray();
+ return nullptr;
+ }
+ ArrayExpr *getArray(const ObjectKey &K) {
+ if (auto *V = get(K))
+ return V->asArray();
+ return nullptr;
+ }
};
class ArrayExpr : public std::vector<Expr> {
@@ -306,6 +363,26 @@
for (const auto &V : C)
emplace_back(V);
}
+
+ // Typed accessors return None/nullptr if the element has the wrong type.
+ llvm::Optional<std::nullptr_t> getNull(size_t I) const {
+ return (*this)[I].asNull();
+ }
+ llvm::Optional<bool> getBoolean(size_t I) const {
+ return (*this)[I].asBoolean();
+ }
+ llvm::Optional<double> getNumber(size_t I) const {
+ return (*this)[I].asNumber();
+ }
+ llvm::Optional<llvm::StringRef> getString(size_t I) const {
+ return (*this)[I].asString();
+ }
+ const ObjectExpr *getObject(size_t I) const {
+ return (*this)[I].asObject();
+ }
+ ObjectExpr *getObject(size_t I) { return (*this)[I].asObject(); }
+ const ArrayExpr *getArray(size_t I) const { return (*this)[I].asArray(); }
+ ArrayExpr *getArray(size_t I) { return (*this)[I].asArray(); }
};
private:
Index: clangd/JSONExpr.cpp
===================================================================
--- clangd/JSONExpr.cpp
+++ clangd/JSONExpr.cpp
@@ -161,7 +161,7 @@
}
case '[': {
Out = json::ary{};
- json::ary &A = *Out.array();
+ json::ary &A = *Out.asArray();
eatWhitespace();
if (peek() == ']') {
++P;
@@ -185,7 +185,7 @@
}
case '{': {
Out = json::obj{};
- json::obj &O = *Out.object();
+ json::obj &O = *Out.asObject();
eatWhitespace();
if (peek() == '}') {
++P;
@@ -507,17 +507,17 @@
return false;
switch (L.kind()) {
case Expr::Null:
- return L.null() == R.null();
+ return *L.asNull() == *R.asNull();
case Expr::Boolean:
- return L.boolean() == R.boolean();
+ return *L.asBoolean() == *R.asBoolean();
case Expr::Number:
- return L.boolean() == R.boolean();
+ return *L.asNumber() == *R.asNumber();
case Expr::String:
- return L.string() == R.string();
+ return *L.asString() == *R.asString();
case Expr::Array:
- return *L.array() == *R.array();
+ return *L.asArray() == *R.asArray();
case Expr::Object:
- return *L.object() == *R.object();
+ return *L.asObject() == *R.asObject();
}
llvm_unreachable("Unknown expression kind");
}
Index: clangd/ClangdUnit.cpp
===================================================================
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -1293,7 +1293,8 @@
// (if there are no other references to it).
OldPreamble.reset();
- trace::Span Tracer(llvm::Twine("Preamble: ") + That->FileName);
+ trace::Span Tracer("Preamble");
+ SPAN_ATTACH(Tracer, "File", That->FileName);
std::vector<DiagWithFixIts> PreambleDiags;
StoreDiagsConsumer PreambleDiagnosticsConsumer(/*ref*/ PreambleDiags);
IntrusiveRefCntPtr<DiagnosticsEngine> PreambleDiagsEngine =
@@ -1342,7 +1343,8 @@
// Compute updated AST.
llvm::Optional<ParsedAST> NewAST;
{
- trace::Span Tracer(llvm::Twine("Build: ") + That->FileName);
+ trace::Span Tracer("Build");
+ SPAN_ATTACH(Tracer, "File", That->FileName);
NewAST = ParsedAST::Build(
std::move(CI), PreambleForAST, SerializedPreambleDecls,
std::move(ContentsBuffer), PCHs, VFS, That->Logger);
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
