8000 [#446] Show symbols of external C/C++ files by ghentschke · Pull Request #449 · eclipse-cdt/cdt-lsp · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[#446] Show symbols of external C/C++ files #449

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Mar 14, 2025
Merged

[#446] Show symbols of external C/C++ files #449

merged 7 commits into from
Mar 14, 2025

Conversation

ghentschke
Copy link
Contributor
@ghentschke ghentschke commented Mar 7, 2025 •
edited
Loading

fixes #446

@ghentschke ghentschke requested review from jonahgraham and jld01 March 7, 2025 20:45
@jld01
Copy link
Contributor
jld01 commented Mar 8, 2025 •
edited
Loading

Hi @ghentschke, with this patch, the endless loop is eliminated when attempting to expand an external translation unit node in the Project Explorer view, but I observe the following exception logged:

!ENTRY org.eclipse.cdt.lsp 4 0 2025-03-08 19:21:18.267
!MESSAGE java.lang.IllegalStateException: Unexpected null value present!
!STACK 0
java.util.concurrent.ExecutionException: java.lang.IllegalStateException: Unexpected null value present!
	at java.base/java.util.concurrent.CompletableFuture.reportGet(CompletableFuture.java:396)
	at java.base/java.util.concurrent.CompletableFuture.get(CompletableFuture.java:2096)
	at org.eclipse.cdt.lsp.internal.ui.navigator.SymbolsManager.refreshTreeContentFromLS(SymbolsManager.java:249)
	at org.eclipse.cdt.lsp.internal.ui.navigator.SymbolsManager.getCompileUnitElements(SymbolsManager.java:220)
	at org.eclipse.cdt.lsp.internal.ui.navigator.SymbolsManager.fetchDeferredChildren(SymbolsManager.java:133)
	at org.eclipse.ui.progress.DeferredTreeContentManager$1.run(DeferredTreeContentManager.java:215)
	at org.eclipse.core.internal.jobs.Worker.run(Worker.java:63)
Caused by: java.lang.IllegalStateException: Unexpected null value present!
	at org.eclipse.lsp4e.internal.NullSafetyHelper.castNonNull(NullSafetyHelper.java:36)
	at org.eclipse.lsp4e.LanguageServerWrapper.lambda$19(LanguageServerWrapper.java:940)
	at java.base/java.util.concurrent.CompletableFuture.uniApplyNow(CompletableFuture.java:684)
	at java.base/java.util.concurrent.CompletableFuture.uniApplyStage(CompletableFuture.java:662)
	at java.base/java.util.concurrent.CompletableFuture.thenApply(CompletableFuture.java:2200)
	at org.eclipse.lsp4e.LanguageServerWrapper.getServerCapabilitiesAsync(LanguageServerWrapper.java:940)
	at org.eclipse.lsp4e.LanguageServers$LanguageServerDocumentExecutor.filter(LanguageServers.java:281)
	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
	at java.base/java.util.Iterator.forEachRemaining(Iterator.java:133)
	at java.base/java.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1939)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:575)
	at java.base/java.util.stream.AbstractPipeline.evaluateToArrayNode(AbstractPipeline.java:260)
	at java.base/java.util.stream.ReferencePipeline.toArray(ReferencePipeline.java:616)
	at java.base/java.util.stream.ReferencePipeline.toArray(ReferencePipeline.java:622)
	at java.base/java.util.stream.ReferencePipeline.toList(ReferencePipeline.java:627)
	at org.eclipse.lsp4e.LanguageServers$LanguageServerDocumentExecutor.getServers(LanguageServers.java:289)
	at org.eclipse.lsp4e.LanguageServers.executeOnServers(LanguageServers.java:421)
	at org.eclipse.lsp4e.LanguageServers.computeFirst(LanguageServers.java:168)
	at org.eclipse.cdt.lsp.internal.ui.navigator.SymbolsManager.refreshTreeContentFromLS(SymbolsManager.java:247)
	... 4 more

Do you also observe this exception?

@ghentschke ghentschke changed the title Fix 446 Show symbols of external C/C++ files Mar 9, 2025
@ghentschke
Copy link
Contributor Author

No, I haven't seen this exception but I it seems to be a LSP4E problem.

@ghentschke ghentschke changed the title Show symbols of external C/C++ files [#446] Show symbols of external C/C++ files Mar 9, 2025
@ghentschke
Copy link
Contributor Author
ghentschke commented Mar 9, 2025 •
edited
Loading

Hm, it's strange that serverCapabilities is null. I looks like a race condition until the LS has been started. Can you reproduce this behavior?

@ghentschke
Copy link
Contributor Author

With the latest LSP4E I cannot reproduce this behavior

@jld01
Copy link
Contributor
jld01 commented Mar 10, 2025

Hi @ghentschke, yes this is reproducible. I am using LSP4E from the current CDT target definition: org.eclipse.lsp4e_0.18.15.202412051200.jar

@ghentschke
Copy link
Contributor Author
ghentschke commented Mar 12, 2025 •
edited
Loading

Hm, when running the change with cdt 11.6.1 target platform I get lsp4e 0.18.18. Even with this version I don't see the exception. Could you please update the cdt target and check if the exception still occurs?

I had the wrong target platform

@ghentschke
Copy link
Contributor Author

I could not reproduce the exception with org.eclipse.lsp4e_0.18.15.202412051200.jar
What I've done (Windows 10):

  1. create a new HelloWorld cmake project via the cdt new project wizard.
  2. Add set(CMAKE_BUILD_TYPE Debug) to CMakeLists.txt build with debug symbols
  3. Build project with msys64/mingw64/bin/cc.exe respective /c++.exe
  4. Expand external header:
    grafik
  5. I see no exception

@jld01
Copy link
Contributor
jld01 commented Mar 13, 2025 •
edited
Loading 
5. I see no exception

@ghentschke I will try with a 2025-03 target platform.

Note that we need to update the CDT API baseline to CDT 12.0.0 / CDT-LSP 3.0.0 before committing to the main branch now.

@jld01
Copy link
Contributor
jld01 commented Mar 13, 2025 •
edited
Loading

@ghentschke, I tried updating my CDT target definition locally to use LSP4E 0.27.1 (org.eclipse.lsp4e_0.18.17.202502012025.jar) but still see the same issue. org.eclipse.lsp4e.LanguageServerWrapper.serverCapabilities is not being initialized. I will continue to investigate under debug.

@jld01
Copy link
Contributor
jld01 commented Mar 13, 2025

@ghentschke, the issue I was seeing was caused by the presence of org.eclipse.cdt.lsp.test.internal.server.MockCLanguageServerProvider in my PDE workspace.

Your change is now working for me. 👍

We do need to update the API baseline though.

jld01
jld01 reviewed Mar 13, 2025
View reviewed changes
bundles/org.eclipse.cdt.lsp/META-INF/MANIFEST.MF
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bundle-Version: 3.0.100.qualifier

Copy link
Contributor Author

Choose a reason for hiding this comme 8000 nt

The reason will be displayed to describe this comment to others. Learn more.

Must the service segment increased with every PR?

Copy link
Contributor
@jld01 jld01 Mar 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, just once per release cycle: https://github.com/eclipse-platform/eclipse.platform/blob/master/docs/VersionNumbering.md

I now note that @jonahgraham has already merged version updates to main via #451 so there is no need to modify the version. Apologies for the confusion. Please do revert back to version 3.0.100 for this PR.

Copy link
Contributor Author
@ghentschke ghentschke Mar 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jld01 if you have no objections I would like to merge this PR.

@ghentschke ghentschke requested a review from jld01 March 14, 2025 06:49
jld01
jld01 approved these changes Mar 14, 2025
View reviewed changes
Copy link
Contributor
@jld01 jld01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and looks good to me, @ghentschke.

@ghentschke ghentschke merged commit 36d98b9 into eclipse-cdt:main Mar 14, 2025
3 checks passed
@ghentschke
Copy link
Contributor Author

@jld01 Thank you very much!

@ghentschke ghentschke deleted the fix-446 branch March 14, 2025 16:51
@jonahgraham jonahgraham added this to the 3.1.0 milestone Apr 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jld01 jld01 jld01 approved these changes

@jonahgraham jonahgraham Awaiting requested review from jonahgraham

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
3.1.0
Development

Successfully merging this pull request may close these issues.

Endless loop when computing symbols for toolchain header file
3 participants
@ghentschke @jld01 @jonahgraham
0