News:

Accounts with zero posts and zero activity during the last months will be deleted periodically to fight SPAM!

Main Menu

Code completion using LSP and clangd

Started by Pecan, February 20, 2021, 09:13:50 PM

Previous topic - Next topic

ollydbg

Quote from: gd_on on September 12, 2022, 04:26:59 PM
Probably because of recent changes in C::B sdk, clangd_client does not compile when using wxWidgets 3.2.1 (probably >= 3.1.6)
Error in client.cpp, line 3672 : CodeBlocksLogEvent waits for a wxBitmapBundle*, but logbmp is a wxBitmap*

I'm building C::B rev12886, and clangd_client rev76, I don't have build issue against wx 3.2.1.

Maybe the recent change from rev12887 to rev12890 causes the build issue.

If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Miguel Gimenez

Starting with r12887, if you are compiling with wxWidgets >= 3.1.6 the last parameter of CodeBlocksLogEvent must be a pointer to wxBitmapBundle.

The change is straightforward, I will publish a patch in this thread tomorrow (I have detected an error in the original notebook HiDPI code that must be fixed beforehand).

Miguel Gimenez

Bug fixed in r12891.

To fix the plugin replace (in client.cpp:3664) this

        const int uiSize = Manager::Get()->GetImageSize(Manager::UIComponent::InfoPaneNotebooks);
        const int uiScaleFactor = Manager::Get()->GetUIScaleFactor(Manager::UIComponent::InfoPaneNotebooks);
        const wxString imgFile = ConfigManager::GetDataFolder()
                              + wxString::Format(_T("/resources.zip#zip:/images/%dx%d/filefind.png"),
                                                 uiSize, uiSize);
        wxBitmap* logbmp = new wxBitmap(cbLoadBitmapScaled(imgFile, wxBITMAP_TYPE_PNG, uiScaleFactor));

with this

        const int uiSize = Manager::Get()->GetImageSize(Manager::UIComponent::InfoPaneNotebooks);
        wxString prefix(ConfigManager::GetDataFolder()+"/resources.zip#zip:/images/");
#if wxCHECK_VERSION(3, 1, 6)
        const double uiScaleFactor = Manager::Get()->GetUIScaleFactor(Manager::UIComponent::InfoPaneNotebooks);
        wxBitmapBundle* logbmp = new wxBitmapBundle(cbLoadBitmapBundle(prefix, "filefind.png", wxRound(uiSize/uiScaleFactor), wxBITMAP_TYPE_PNG));
#else
        prefix << wxString::Format("%dx%d/", uiSize, uiSize);
        wxBitmap* logbmp = new wxBitmap(cbLoadBitmap(prefix+"filefind.png", wxBITMAP_TYPE_PNG));
#endif


ollydbg

If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Pecan

@ MaxGaspa

Until I can find the cause of the crash, you can disable Settings/Editor/GeneralSetting/Documentation popup.

This should eliminate the stall/crash. If It does not, let me know.

Thanks

Pecan

#170
Quote from: Pecan on September 13, 2022, 09:30:43 PM
@ MaxGaspa
Until I can find the cause of the crash, you can disable Settings/Editor/GeneralSetting/Documentation popup.
This should eliminate the stall/crash. If It does not, let me know.
Thanks

RFC: patch for ticket 1168.
This is a Request For Comment for the attached patch.
I have posted a patch on the second page of ticket #1168:
Direct link to Patch

The attached patch (ccManagerMouseTrap220915-1.patch) resolves the following described conditions for both CodeCompletion and Clangd_client. The patch has been tested on both MSW and Linux.

1) The HtmlDocumentationPopup gets stuck showing and cannot be closed.
2) The user did a double-click to select from the AutocompPopup, the HtmlDocumentationPopup got left showing and CB is frozen (and may crash).
3) The AutocompPopup and the HtmlPopup are showing, the user unfocused CB then re-focused CB and CB is frozen and must be killed.
4) The user has unChecked Documentation popup in options and now AutocompPopups cannot be scrolled.
5) The AutocompPopup only is stuck showing and cannot be closed.

What is actually happening? (tldr version)
CB is getting stuck in function OnPopScroll statement:
else if (m_pAutocompPopup && m_pAutocompPopup->GetScreenRect().Contains(pos))
in the GetScreenRect() function because the AutocompPopup window has been hidden and the HtmlDocumentationPopup may or may not be hidden, but a wxEVT_MOUSEWHEEL connection is owned by the html popup.

It seems that wxWidgets does not like this condition.
The debugger cannot even step out of this condition.

The problem usually arises from a slower double-mouse-click on an AutocompPopup item such that the double click gets interpreted as two single-clicks.
The AutocompPopup will get closed leaving the HtmlDocumentationPopup out to dry.
Now the conditions on which ccManager depended to remove the event connection will never be met.
We than have a happy deadly embrace.

Other sequences of action also cause the deadly embrace including (occasionally) un-focusing and re-focusing CB while both popups are showing.

I didn't trace through all the conditions causing the deadly embrace since the fix to avoid connecting wxEVT_MOUSEWHEEL to HtmlPopup when AutocompPopup was hidden fixed most of the other conditions also. (ie. the first fix eliminated the possibility of the others).
I did, however, trace through all listed conditions once the fix was applied locally.

I would like to apply the patch before the next nightly.
Comments please....

ollydbg

Quote1) The HtmlDocumentationPopup gets stuck showing and cannot be closed.
2) The user did a double-click to select from the AutocompPopup, the HtmlDocumentationPopup got left showing and CB is frozen (and may crash).
3) The AutocompPopup and the HtmlPopup are showing, the user unfocused CB then re-focused CB and CB is frozen and must be killed.
4) The user has unChecked Documentation popup in options and now AutocompPopups cannot be scrolled.
5) The AutocompPopup only is stuck showing and cannot be closed.

Hi, Pecan, good work! I see issue5 many times randomly in my daily work, but it is hard to reproduce, I even don't know how to reproduce this issue. The popup window shown on top of every application, and even CB is not focused, the popup window is still showing. What I have to do is kill the C::B process from the task manager.

Hope your fix will solve those issues, thanks!
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

ollydbg

About the ccManagerMouseTrap220915-1.patch file.

I'm not sure, but this variable


std::map<cbEditor*,bool> m_EdAutoCompMouseTraps;


The bool value is always "true"? Since I see the only code here is:

m_EdAutoCompMouseTraps[ed] = true;


Also, is the code Windows related? If yes, I think the "#ifdef __WXMSW__" should also be placed in the header file around this member variable declaration.
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Pecan

#173
Quote from: ollydbg on September 16, 2022, 07:50:24 AM
About the ccManagerMouseTrap220915-1.patch file.

I'm not sure, but this variable


std::map<cbEditor*,bool> m_EdAutoCompMouseTraps;


The bool value is always "true"? Since I see the only code here is:

m_EdAutoCompMouseTraps[ed] = true;


Also, is the code Windows related? If yes, I think the "#ifdef __WXMSW__" should also be placed in the header file around this member variable declaration.


The declaration and all references to m_EdAutoCompMouseTraps are already guarded by "#ifdef __WXMSW__"
#ifdef __WXMSW__
        /** a handle to the autocomplete list window created by (wx)scintilla, needed under Windows
         * to determine its dimensions (so the scroll event can be sent to it, if relevant)
         */
        wxListView* m_pAutocompPopup;

        /**
         * List of editors holding an event connect to popup mouse scroll event
         * for AutoCompPopup and html Documentation popup
         */
        std::map<cbEditor*,bool> m_EdAutoCompMouseTraps;
#endif // __WXMSW__

The statement:
m_EdAutoCompMouseTraps[ed] = true; is only used to enter the cbEditor* into the map assuring only one entry for this editor exists. Is there a better method for inserting a entry into a map?

It is removed by statement:
m_EdAutoCompMouseTraps.erase(m_pLastEditor);

I will make a specific comment explaining that.
Thanks for reviewing.

ollydbg

I'm tweaking wx sample workspace, see here: codeblocks cbp projects for wx samples

While, after switch from one cbp to other cbp for many times, I got many clangd.exe process running, see the image shot below, is it correct?

Thanks.
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

ollydbg

Quote from: Pecan on September 16, 2022, 07:26:36 PM
Is there a better method for inserting a entry into a map?

I'm not fully understand the code logic, but do you really need a map? When a key is added, it's  value is always "true".

I mean a std::set<cbEditor*> is better than std::map<cbEditor*,bool> ?
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Pecan

Quote from: ollydbg on September 17, 2022, 02:45:36 PM
I'm not fully understand the code logic, but do you really need a map? When a key is added, it's  value is always "true".

I mean a std::set<cbEditor*> is better than std::map<cbEditor*,bool> ?

Thanks, I learned a long time ago that other eyes reviewing code can make it better.
Direct link to newer patch

Miguel Gimenez

Patch for ticket #1168 applied in r12899, thank you.

ollydbg

The msys2 project now has clangd 15.0 for mingw64 targets.
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

ollydbg

I have a question:

LSP_ParseSemanticTokens(), what does this function used for?

I see this code:


// ----------------------------------------------------------------------------
void Parser::OnLSP_RequestedSemanticTokensResponse(wxCommandEvent& event)  //(ph 2021/03/12)
// ----------------------------------------------------------------------------
{
    if (GetIsShuttingDown()) return;

    // This is a callback after requesting textDocument/Symbol (request done at end of OnLSP_RequestedSymbolsResponse() )
    // Currently, we allow SemanticTokens for the BuiltinActiveEditor only,

    // ----------------------------------------------------------------------------
    ///  GetClientData() contains ptr to json object
    ///  DONT free it! The return to OnLSP_Event() will free it as a unique_ptr
    // ----------------------------------------------------------------------------
    json* pJson = (json*)event.GetClientData();
    wxString idStr = event.GetString();
    wxString URI = idStr.AfterFirst(STX);
    if (URI.Contains(STX))
        URI = URI.BeforeFirst(STX); //filename

    wxString uriFilename = fileUtils.FilePathFromURI(URI);      //(ph 2021/12/21)
    cbEditor*  pEditor =  nullptr;
    cbProject* pProject = nullptr;
    EditorManager* pEdMgr = Manager::Get()->GetEditorManager();
    EditorBase* pEdBase = pEdMgr->IsOpen(uriFilename);
    if (pEdBase)
    {
        pEditor = pEdMgr->GetBuiltinActiveEditor();
        if (not pEditor or (pEditor->GetFilename() != uriFilename))
            return;
        ProjectFile* pProjectFile = pEditor->GetProjectFile();
        if (pProjectFile) pProject = pProjectFile->GetParentProject();
        if ( (not pProjectFile) or (not pProject) ) return;
        ParserBase* pParser = GetParseManager()->GetParserByProject(pProject);
        if (not pParser)
            return;
    }
    else return;

    if (not pProject) pProject = Manager::Get()->GetProjectManager()->GetActiveProject();
    ProcessLanguageClient* pClient = GetLSPClient();

    // Queue the the json data to OnLSP_ParseDocumentSymbols() event, passing it the json pointer
    // The json data will be placed in a queue to be processed during OnIdle() events. //(ph 2021/09/11)
    wxCommandEvent symEvent(wxEVT_COMMAND_MENU_SELECTED, XRCID("textDocument/semanticTokens"));
    symEvent.SetString(uriFilename);
    symEvent.SetClientData(pJson);
    LSP_ParseSemanticTokens(symEvent); //Call directly


The last line:
LSP_ParseSemanticTokens(symEvent); //Call directly

This use the our old-CC's parsing code, for example


// ----------------------------------------------------------------------------
Token* LSP_SymbolsParser::DoHandleClass(EClassType ct, int lineNumber, int lastLineNumber, int endCol)            //(ph 2021/05/15)
// ----------------------------------------------------------------------------
{
    // need to force the tokenizer _not_ skip anything
    // as we 're manually parsing class decls
    // don't forget to reset that if you add any early exit condition!
    TokenizerState oldState = m_Tokenizer.GetState();
    m_Tokenizer.SetState(tsNormal);
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.