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 for this JSON message.
----------------
ioeric wrote:
> Why this empty block?
Oops, was tracing here but it didn't turn out to be useful. Reverted.
================
Comment at: clangd/Trace.cpp:90
+
+void start(raw_ostream &OS) {
+ assert(!T && "start() called twice without stop()");
----------------
ioeric wrote:
> Would it make sense to have a helper class which handles starting/stopping
> in, say, constructor/destructor? ... so that we don't need to worry about
> forgetting to stop?
I didn't do this for a couple of reasons:
- it obscures the static-stateful nature of this code, making it look like
e.g. you could dynamically turn on tracing while clangd is running. (Which
would be cool, but wouldn't currently work)
- it's some more boilerplate, and isn't actually very convenient to ClangdMain
(since `start()` is conditional)
================
Comment at: clangd/tool/ClangdMain.cpp:98
+ llvm::errs() << "Error while opening trace file: " << EC.message();
+ }
+ trace::start(*TraceStream);
----------------
ioeric wrote:
> Is this intended? Start tracing even if there is an error?
Oops, thanks!
https://reviews.llvm.org/D39086
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
