- Converted Protocol.h parse() functions to take JSON::Expr. These no longer detect and log unknown fields, as this is not that useful and no longer free. I haven't changed the error handling too much: fields that were treated as optional before are still optional, even when it's wrong. Exception: object properties with the wrong type are now ignored.
- Made JSONRPCDispatcher parse using json::parse
- The bug where 'method' must come before 'params' in the stream is fixed as a side-effect. (And the same bug in executeCommand).
- Some parser crashers fixed as a side effect. e.g. https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=3890
- The debug stream now prettyprints the input messages with --pretty.
- Request params are attached to traces when tracing is enabled.
- Fixed some bugs in tests (errors tolerated by YAMLParser, and off-by-ones in Content-Length that our null-termination was masking)
- Fixed a random double-escape bug in ClangdLSPServer (it was our last use of YAMLParser!)
Details
Diff Detail
- Repository
- rCTE Clang Tools Extra
Event Timeline
Oops, Eric is most likely to have this stuff in cache :-)
Sorry this is huge and ugly, -500 lines though!
I think the Protocol code could be a lot tighter (there's still a lot of repeated "is this an object", and "if this property exists parse it as type X and store it in field Y, else fail". (The kind of stuff YamlIO is good at).
I wanted to stick to a pretty literal translation first, and maybe later find a way to clean it up further. Hope that's OK!
clangd/JSONRPCDispatcher.cpp | ||
---|---|---|
221 | Use llvm::toString? | |
clangd/JSONRPCDispatcher.h | ||
47 | It seems that we are piggybacking a state of clangd with JSONOutput. It might make sense to store the option in clangd (or the dispatcher?) and pass it via writeMessage, but what you have is also fine if it saves trouble :) |
clangd/JSONRPCDispatcher.h | ||
---|---|---|
47 | Yeah, this isn't great. In the short term I don't think it does much harm. Longer-term we'll revisit logging organization as part of RequestContext that Ilya's working on. |
It seems that we are piggybacking a state of clangd with JSONOutput. It might make sense to store the option in clangd (or the dispatcher?) and pass it via writeMessage, but what you have is also fine if it saves trouble :)