kadircet added inline comments.
Comment at: clang-tools-extra/trunk/unittests/clangd/TraceTests.cpp:57
+}
+std::string VS = V->getValue(Tmp).str();
+if (VS != I->second) {
RKSimon wrote:
> @sammccall PVS Studio is reporting a potential null dereferenc
RKSimon added inline comments.
Herald added subscribers: usaxena95, kadircet, arphaman, MaskRay.
Comment at: clang-tools-extra/trunk/unittests/clangd/TraceTests.cpp:57
+}
+std::string VS = V->getValue(Tmp).str();
+if (VS != I->second) {
@sammccall PVS
This revision was automatically updated to reflect the committed changes.
Closed by commit rL317193: Performance tracing facility for clangd. (authored
by sammccall).
Changed prior to commit:
https://reviews.llvm.org/D39086?vs=121253&id=121257#toc
Repository:
rL LLVM
https://reviews.llvm.or
sammccall updated this revision to Diff 121253.
sammccall added a comment.
clang-format
https://reviews.llvm.org/D39086
Files:
clangd/CMakeLists.txt
clangd/ClangdServer.cpp
clangd/ClangdUnit.cpp
clangd/JSONRPCDispatcher.cpp
clangd/ProtocolHandlers.cpp
clangd/Trace.cpp
clangd/Trace
sammccall added inline comments.
Comment at: clangd/Trace.h:38
+ // Starts a sessions capturing trace events and writing Trace Event JSON.
+ static std::unique_ptr createJSON(llvm::raw_ostream &OS);
+ ~Session();
ioeric wrote:
> `createJSON` is a bit confusing
sammccall updated this revision to Diff 121252.
sammccall marked an inline comment as done.
sammccall added a comment.
createJSON -> create
https://reviews.llvm.org/D39086
Files:
clangd/CMakeLists.txt
clangd/ClangdServer.cpp
clangd/ClangdUnit.cpp
clangd/JSONRPCDispatcher.cpp
clangd/Pr
ioeric accepted this revision.
ioeric added a comment.
still lgtm
Comment at: clangd/Trace.h:38
+ // Starts a sessions capturing trace events and writing Trace Event JSON.
+ static std::unique_ptr createJSON(llvm::raw_ostream &OS);
+ ~Session();
`createJSON`
sammccall updated this revision to Diff 121247.
sammccall added a comment.
Provide RAII-like interface to trace functionality.
Note that we may want to provide a backend API here.
https://reviews.llvm.org/D39086
Files:
clangd/CMakeLists.txt
clangd/ClangdServer.cpp
clangd/ClangdUnit.cpp
ilya-biryukov added inline comments.
Comment at: clangd/Trace.cpp:87
+
+static Tracer* T = nullptr;
+} // namespace
sammccall wrote:
> ilya-biryukov wrote:
> > Maybe use `unique_ptr` (or even `llvm::Optional`)?
> > Otherwise we leak memory and don't flush the st
sammccall marked an inline comment as done.
sammccall added inline comments.
Comment at: clangd/Trace.cpp:87
+
+static Tracer* T = nullptr;
+} // namespace
ilya-biryukov wrote:
> Maybe use `unique_ptr` (or even `llvm::Optional`)? Otherwise
> we leak memory and d
sammccall updated this revision to Diff 119864.
sammccall added a comment.
Address FIXME now that r316330 is in.
https://reviews.llvm.org/D39086
Files:
clangd/CMakeLists.txt
clangd/ClangdServer.cpp
clangd/ClangdUnit.cpp
clangd/JSONRPCDispatcher.cpp
clangd/ProtocolHandlers.cpp
clangd
ilya-biryukov added inline comments.
Comment at: clangd/Trace.cpp:87
+
+static Tracer* T = nullptr;
+} // namespace
Maybe use `unique_ptr` (or even `llvm::Optional`)? Otherwise we
leak memory and don't flush the stream if users of tracing API forget to call
`st
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
lgtm
https://reviews.llvm.org/D39086
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c
sammccall updated this revision to Diff 119562.
sammccall marked 3 inline comments as done.
sammccall added a comment.
Address review comments.
https://reviews.llvm.org/D39086
Files:
clangd/CMakeLists.txt
clangd/ClangdServer.cpp
clangd/ClangdUnit.cpp
clangd/JSONRPCDispatcher.cpp
clang
sammccall added inline comments.
Comment at: clangd/JSONRPCDispatcher.cpp:230
- // Finally, execute the action for this JSON message.
- if (!Dispatcher.call(JSONRef, Out))
-Out.log("JSON dispatch failed!\n");
+ {
+// Finally, execute the action fo
ioeric added a comment.
Looks good in general. A few nits.
Comment at: clangd/JSONRPCDispatcher.cpp:230
- // Finally, execute the action for this JSON message.
- if (!Dispatcher.call(JSONRef, Out))
-Out.log("JSON dispatch failed!\n");
+ {
+// Fi
sammccall created this revision.
Herald added a subscriber: mgorny.
This lets you visualize clangd's activity on different threads over time,
and understand critical paths of requests and object lifetimes.
The data produced can be visualized in Chrome (at chrome://tracing), or
in a standalone copy
17 matches
Mail list logo
