Code completion using LSP and clangd

[Pecan]

I see there are many pch files in the folder such as:

C:\Users\[myusername]\AppData\Local\Temp\preamble-c7460b.pch

I think those files is created by clangd, and are there any way to automatically delete them when exit C::B?

EDIT：

clangd writes too much disk : CPP-19402

This discussion looks like the pch can keep in "memory".

Here is the patch to fix this pch file issue:

Code

 clangd_client/src/LSPclient/src/client.cpp | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/clangd_client/src/LSPclient/src/client.cpp b/clangd_client/src/LSPclient/src/client.cpp
index 88f5f8f..f6b5eb9 100644
--- a/clangd_client/src/LSPclient/src/client.cpp
+++ b/clangd_client/src/LSPclient/src/client.cpp
@@ -266,6 +266,10 @@ ProcessLanguageClient::ProcessLanguageClient(const cbProject* pProject, const ch
 
     command += " --limit-results=20";              // Limit the number of results returned by clangd. 0 means no limit (default=100)
 
+    // clangd writes too much disk : CPP-19402 https://youtrack.jetbrains.com/issue/CPP-19402
+    // "-pch-storage=memory"
+    command += " -pch-storage=memory";
+
     if (wxDirExists(clangResourceDir))
         command += " --resource-dir=" + clangResourceDir;  // Directory for system includes
 

Thanks for that.

I've added code to remove preamble-*.tmp and preamble-*.pch files when the project closes.

I'll later add an option to keep the .pch in memory. (For me, I'd rather keep them on disk and remove them at project close time).

As an aside: Windows does not allow me to remove files that are open and being used. But Linux does. Do you know of a way to tell Linux NOT to allow me to delete open files?

[Pecan]

I see there are some code snippet like:

Code

m_MutexInputBufGuard.Lock;

m_MutexInputBufGuard.Unlock();

But in the code, we have to carefully handle the unlocking the wxMutex when return the function body, especially when there are multiply returns.

Is it possible to use the wxMutexLocker, and check the IsOK() function for checking whether it get locked or not.

There are only 2 locks in the code that can cause any trouble. The lock on the input buffer. One to write to the buffer, and one to get the next clangd response out of the buffer. And neither affect the UI thread.

I tried wxMutexLocker first before giving up on it.
I want to be able to unlock the input buffer and then do more work in the function.
When I unlocked the mutex before the function ended,  wxWidgets gave me errors about the mutex. I lost confidence that I could mix a wxMutexLocker and manual unlocks.

In fact, I removed all locks on the main UI thread and used idle time callbacks instead.

[ollydbg]

I see there are some code snippet like:

Code

m_MutexInputBufGuard.Lock;

m_MutexInputBufGuard.Unlock();

But in the code, we have to carefully handle the unlocking the wxMutex when return the function body, especially when there are multiply returns.

Is it possible to use the wxMutexLocker, and check the IsOK() function for checking whether it get locked or not.

There are only 2 locks in the code that can cause any trouble. The lock on the input buffer. One to write to the buffer, and one to get the next clangd response out of the buffer. And neither affect the UI thread.

I tried wxMutexLocker first before giving up on it.
I want to be able to unlock the input buffer and then do more work in the function.
When I unlocked the mutex before the function ended,  wxWidgets gave me errors about the mutex. I lost confidence that I could mix a wxMutexLocker and manual unlocks.

In fact, I removed all locks on the main UI thread and used idle time callbacks instead.

Yes, there are only There are only 2 locks in the code. Sometimes, I got error message about lock failed, I'm not sure why.

BTW: since the console pipe is connected with clangd.exe, I'm not sure why the locker is needed. Since the content is from a thread to the main GUI thread by the Event, when you got the Event, you were already in the main GUI thread, and the content in the Event(wxThreadEvent) is already deep copied. So, I think the locker is not necessary, am I correct?

[Pecan]

BTW: since the console pipe is connected with clangd.exe, I'm not sure why the locker is needed. Since the content is from a thread to the main GUI thread by the Event, when you got the Event, you were already in the main GUI thread, and the content in the Event(wxThreadEvent) is already deep copied. So, I think the locker is not necessary, am I correct?

There are two non main UI threads accessing the clangd input buffer. The pipe thread and the ReadJson thread. Without the lock I assume the ReadJason thread could remove data at the same time the pipe "ProcessEvent" was writing to the (UI client.h) std::string buffer.

the "locks failed" messages could be coming from attempts to lock the symbols tree (comsuming). But it's ok. If the symbols tree update thread can't get the lock. Its ok to block.

If the lock fails in :LSP_ParseDocumentSymbols (stows symbols in tree), it just requeues itself for an idle time callback. It does not block. 

[ollydbg]

BTW: since the console pipe is connected with clangd.exe, I'm not sure why the locker is needed. Since the content is from a thread to the main GUI thread by the Event, when you got the Event, you were already in the main GUI thread, and the content in the Event(wxThreadEvent) is already deep copied. So, I think the locker is not necessary, am I correct?

There are two non main UI threads accessing the clangd input buffer. The pipe thread and the ReadJson thread. Without the lock I assume the ReadJason thread could remove data at the same time the pipe "ProcessEvent" was writing to the (UI client.h) std::string buffer.

the "locks failed" messages could be coming from attempts to lock the symbols tree (comsuming). But it's ok. If the symbols tree update thread can't get the lock. Its ok to block.

If the lock fails in :LSP_ParseDocumentSymbols (stows symbols in tree), it just requeues itself for an idle time callback. It does not block.

Hi, Pecan. Thanks for the explanation.

I'm surprised to find that you implemented the symbol tree. This is really a good job!

[ollydbg]

FYI:

I have make a simple wiki page:

CB Clangd Client - Code::Blocks

 

[Pecan]

FYI:

I have make a simple wiki page:

CB Clangd Client - Code::Blocks

 

Thanks, Someday I'll get a chance to add more info.

[ollydbg]

CB Clangd Client / Code / Commit [r32]

Please note that in this revision.
I'm using the Windows clangd_client_wx31_64.cbp
The generated dll file name is:  Clangd_Client.dll
And the zip file name is: clangd_client.zip

Do you think the name should be the same? I mean to keep the case consistent.

[ollydbg]

I see an alert from this code snippet, and here is the fix:

Code

 clangd_client/src/LSPclient/src/client.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clangd_client/src/LSPclient/src/client.cpp b/clangd_client/src/LSPclient/src/client.cpp
index 459b809..e03730d 100644
--- a/clangd_client/src/LSPclient/src/client.cpp
+++ b/clangd_client/src/LSPclient/src/client.cpp
@@ -1073,7 +1073,7 @@ bool ProcessLanguageClient::DoValidateUTF8data(std::string& data)
             std::string invStr(&data[invloc], 1);
             //unsigned int invInt = (unsigned int)data[invloc];
             unsigned char invChar(invStr[0]);
-            wxUniChar uniChar(invChar);
+            wxUniChar uniChar((unsigned int)invChar);
 
             // clangd response:
             // {"id":"textDocument/completion","jsonrpc":"2.0","result":{

When debugging, I see that the "invChar" is 161(dec), and it cause the alert from wx.
So, we have to first convert to "unsigned int".

See the document from: wxWidgets: wxUniChar Class Reference

wxUniChar::wxUniChar    (    unsigned char     c   )    

Create a character from the 8-bit character value c using the current locale encoding.

[Pecan]

CB Clangd Client / Code / Commit [r32]

Please note that in this revision.
I'm using the Windows clangd_client_wx31_64.cbp
The generated dll file name is:  Clangd_Client.dll
And the zip file name is: clangd_client.zip

Do you think the name should be the same? I mean to keep the case consistent.

I do, but others did not. Wasn't worth quibbling about to me.

Thanks for the wxUniChar fix.

[AndrewCot]

The plugin spec says they need to be the same. I cannot remember which OS or if it was the plugin, but case differences have caused me problems. I missed this one when comparing CodeBlocks_wx31_64.cbp and clangd_client_wx31_64.cbp changes for the plugin as my CodeBlocks_wx31_64.cbp has the following line for the output:

                <Option output="devel31_64/share/CodeBlocks/plugins/clangd_client" prefix_auto="1" extension_auto="1" />

[Pecan]

@ ollydbg

This change didn't work for me. (Message #69)

I want the codepoint. I don't get any asserts.

With "wxUniChar uniChar(invChar);" I get:

Code

Error: Removed clangd response invalid utf8 char:position(3665), hex(85), U(2026), <cant post> ResponseID:textDocument/completion

Note that I get the codepoint U(2026) back.

With "wxUniChar uniChar(unsigned int(invChar));" I get:

Code

Error: Removed clangd response invalid utf8 char:position(6899), hex(85), U(85), ,<cant post on sf>. ResponseID:textDocument/completion

Here I get only the hex value.

So I changed the wxString::Format to:

Code

msg += wxString::Format("position(%d), hex(%02hhX), U(%x), \'%s\'", invloc, (unsigned int)invChar, (int)uniChar.GetValue(), invStr );

Note the "(int)uniChar.GetValue()"

I'm using wx3.1.5 on windows and wx3.0 on linux.
Works with no asserts.
Does it work for you

Note: it took me 45 minutes to post this. Don't try and post a msg with an invalid utf8 char.  It's a PITA

[ollydbg]

The plugin spec says they need to be the same. I cannot remember which OS or if it was the plugin, but case differences have caused me problems. I missed this one when comparing CodeBlocks_wx31_64.cbp and clangd_client_wx31_64.cbp changes for the plugin as my CodeBlocks_wx31_64.cbp has the following line for the output:

                <Option output="devel31_64/share/CodeBlocks/plugins/clangd_client" prefix_auto="1" extension_auto="1" />

For generated files, I prefer the lower case file name format. Since you have the commit right to the svn repo, can you fix them?

BTW:

The custom variables in build options can be improved from my point of view:

1,  I see "TARGET_DEVEL_DIR_AC" and "TARGET_DEVEL_DIR_PECAN" and "TARGET_DEVEL_DIR" in custom variables. Can we just keep only one variable? I mean we can use a "global variable" in the Menu->Settings->Global variables. This way, we can set those variables by our own setting.

2, the name TARGET_DEVEL_DIR is not correct here. I think "DEVEL_DIR" mainly refer to a folder named "devel31_64" which store the built exe or dlls. So, a better name could be "CB_SOURCE_ROOT" which refer the the root of the svn/git source code root folder.

[ollydbg]

@ ollydbg

This change didn't work for me. (Message #69)

I want the codepoint. I don't get any asserts.

With "wxUniChar uniChar(invChar);" I get:

Code

Error: Removed clangd response invalid utf8 char:position(3665), hex(85), U(2026), <cant post> ResponseID:textDocument/completion

Note that I get the codepoint U(2026) back.

With "wxUniChar uniChar(unsigned int(invChar));" I get:

Code

Error: Removed clangd response invalid utf8 char:position(6899), hex(85), U(85), ,<cant post on sf>. ResponseID:textDocument/completion

Here I get only the hex value.

So I changed the wxString::Format to:

Code

msg += wxString::Format("position(%d), hex(%02hhX), U(%x), \'%s\'", invloc, (unsigned int)invChar, (int)uniChar.GetValue(), invStr );

Note the "(int)uniChar.GetValue()"

I'm using wx3.1.5 on windows and wx3.0 on linux.
Works with no asserts.
Does it work for you

In my computer, it works differently than yours.

I did a simple test:

Code

    unsigned char invChar = 0x83;
    wxUniChar uniChar(invChar);

    wxString msg = wxString::Format("hex(%02hhX), U(%x)", (unsigned int)invChar, uniChar.GetValue());

    wxLogMessage(msg);

With the above code, the program just pop up an alert (see screen shot in attachment)

While, with below code, it works OK without the alert.

Code

    unsigned char invChar = 0x83;
    wxUniChar uniChar((unsigned int)invChar);

    wxString msg = wxString::Format("hex(%02hhX), U(%x)", (unsigned int)invChar, uniChar.GetValue());

    wxLogMessage(msg);

Please note that "Create a character from the 8-bit character value c using the current locale encoding.", which means in my locale encoding, a 0x83 is not a valid Unicode code point, I mean maybe we need two bytes or three bytes to convert it to a Unicode code point.

I'm not sure why in your test case, a 0x83 will becomes a larger value code point U(2026). Maybe, you have different locale encoding as mine. I'm on Windows 7 64bit Chinese language edition, so my local encoding maybe some Chinese language.

Note: it took me 45 minutes to post this. Don't try and post a msg with an invalid utf8 char.  It's a PITA

I also meet this kind of forum error from time to time. So bad.

[ollydbg]

This is a code format fix:

Code

 clangd_client/src/LSPclient/src/client.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clangd_client/src/LSPclient/src/client.cpp b/clangd_client/src/LSPclient/src/client.cpp
index fadcb51..459b809 100644
--- a/clangd_client/src/LSPclient/src/client.cpp
+++ b/clangd_client/src/LSPclient/src/client.cpp
@@ -702,9 +702,9 @@ void ProcessLanguageClient::OnLSP_Terminated(wxThreadEvent& event_pipedprocess_t
     wxCommandEvent terminatedEvt(wxEVT_COMMAND_MENU_SELECTED, XRCID("idLSP_Process_Terminated"));
     terminatedEvt.SetEventObject((wxObject*)m_pCBProject);
     terminatedEvt.SetInt(processExitCode);
-    Manager::Get()->GetAppFrame()->GetEventHandler()->ProcessEvent(terminatedEvt)
+    Manager::Get()->GetAppFrame()->GetEventHandler()->ProcessEvent(terminatedEvt);
 
-;    if (processExitCode != 0)
+    if (processExitCode != 0)
     {
         wxString msg = "Unusual termination of LanguageProcessClient(LSP) occured.";
         if (lspClientLogFile.IsOpened() )