-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[Support] Fix some warnings in LSP Transport #160010
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This fixes: ``` [321/5941] Building CXX object lib\Support\LSP\CMakeFiles\LLVMSupportLSP.dir\Transport.cpp.obj C:\git\llvm-project\llvm\lib\Support\LSP\Transport.cpp(123): warning C4930: 'std::lock_guard<std::mutex> responseHandlersLock(llvm::lsp::MessageHandler::ResponseHandlerTy)': prototyped function not called (was a variable definition intended?) [384/5941] Building CXX object unittests\Support\LSP\CMakeFiles\LLVMSupportLSPTests.dir\Transport.cpp.obj C:\git\llvm-project\llvm\unittests\Support\LSP\Transport.cpp(190): warning C4804: '+=': unsafe use of type 'bool' in operation ```
@llvm/pr-subscribers-llvm-support Author: Alexandre Ganea (aganea) ChangesWhen building with latest MSVC on Windows, this fixes some compile-time warnings from last week's integration in #157885:
Full diff: https://github.com/llvm/llvm-project/pull/160010.diff 2 Files Affected:
diff --git a/llvm/lib/Support/LSP/Transport.cpp b/llvm/lib/Support/LSP/Transport.cpp
index e71f17701636b..31b5a89f9b5e0 100644
--- a/llvm/lib/Support/LSP/Transport.cpp
+++ b/llvm/lib/Support/LSP/Transport.cpp
@@ -120,7 +120,7 @@ bool MessageHandler::onReply(llvm::json::Value Id,
// mapping and erase it.
ResponseHandlerTy ResponseHandler;
{
- std::lock_guard<std::mutex> responseHandlersLock(ResponseHandlerTy);
+ std::lock_guard<std::mutex> responseHandlersLock(ResponseHandlersMutex);
auto It = ResponseHandlers.find(debugString(Id));
if (It != ResponseHandlers.end()) {
ResponseHandler = std::move(It->second);
diff --git a/llvm/unittests/Support/LSP/Transport.cpp b/llvm/unittests/Support/LSP/Transport.cpp
index 514e93e983523..0172dee1d603d 100644
--- a/llvm/unittests/Support/LSP/Transport.cpp
+++ b/llvm/unittests/Support/LSP/Transport.cpp
@@ -174,7 +174,7 @@ TEST_F(TransportInputTest, OutgoingRequest) {
TEST_F(TransportInputTest, OutgoingRequestJSONParseFailure) {
// Make an outgoing request that expects a failure response.
- bool responseCallbackInvoked = false;
+ unsigned responseCallbackInvoked = 0;
auto callFn = getMessageHandler().outgoingRequest<CompletionList, Position>(
"outgoing-request-json-parse-failure",
[&responseCallbackInvoked](const llvm::json::Value &id,
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - cheers
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/137/builds/25974 Here is the relevant piece of the build log for the reference
|
When building with latest MSVC on Windows, this fixes some compile-time warnings from last week's integration in #157885: