malaperle requested changes to this revision.
malaperle added inline comments.
This revision now requires changes to proceed.
================
Comment at: clangd/ClangdServer.cpp:528
+ Value = llvm::make_error<llvm::StringError>(
+ "invalid AST",
+ llvm::errc::invalid_argument);
----------------
Invalid
================
Comment at: clangd/ClangdServer.cpp:530
+ llvm::errc::invalid_argument);
+ Value = make_tagged(clangd::findDocumentHighlights(*AST, Pos, Logger),
TaggedFS.Tag);
+ });
----------------
It trips here with assertions enabled because we haven't checked the previous
"Value" for error before, sigh.
Expected<T> must be checked before access or destruction.
Expected<T> value was in success state. (Note: Expected<T> values in success
mode must still be checked prior to being destroyed).
How about this:
```
std::vector<DocumentHighlight> Result;
llvm::Optional<llvm::Error> Err;
Resources->getAST().get()->runUnderLock([Pos, &Result, &Err,
this](ParsedAST *AST) {
if (!AST)
Err = llvm::make_error<llvm::StringError>("Invalid AST",
llvm::errc::invalid_argument);
Result = clangd::findDocumentHighlights(*AST, Pos, Logger);
});
if (Err)
return std::move(*Err);
return make_tagged(Result, TaggedFS.Tag);
```
The tests pass for me with this.
================
Comment at: clangd/Protocol.cpp:372
+
+
+
----------------
Too many new lines. Only keep one.
================
Comment at: clangd/Protocol.h:568
+struct DocumentHighlight {
+ /*
+ *
----------------
Use /// like other places
================
Comment at: clangd/Protocol.h:575
+ /**
+ * The highlight kind, default is DocumentHighlightKind.Text.
+ */
----------------
Use /// like other places
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D38425
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
