Replication of original PR #84: CPP Languager Server support (clang) #1

yashwanthatla · 2025-06-21T12:48:46Z

PR Summary

Add C++ Language Support via Clangd Language Server

Overview

This PR extends multilspy to support the C++ programming language by implementing a ClangdLanguageServer class and adding the necessary configurations and tests.

Change Types

Type	Description
Feature	Add C++ language support using clangd language server
Test	Add tests for C/C++ language server functionality

Affected Modules

Module / File	Change Description
`multilspy/multilspy_config.py`	Added CPP enum value to the Language enum class
`multilspy/language_server.py`	Extended LanguageServer.create method to handle C++ language
`multilspy/language_servers/clangd_language_server/clangd_language_server.py`	New file implementing ClangdLanguageServer class
`multilspy/test_multilspy_clangd.py`	New test file for C/CPP Language Server capabilities

Notes for Reviewers

The implementation adds a new language server for C/C++ using clangd
Tests have been added to verify language server capabilities

CPP Languager Server support (clang)

codeowlai · 2025-06-21T12:54:51Z

src/multilspy/language_servers/clangd_language_server/clangd_language_server.py

+        assert platform_id.value in [
+            "linux-x64"
+        ], "Only linux-x64 is supported for in multilspy at the moment"


🐛 Correctness Issue

Platform check too restrictive.

The platform check only allows Linux x64, which will cause crashes on other platforms where the code might actually work.

Current Code (Diff):

- assert platform_id.value in [ - "linux-x64" - ], "Only linux-x64 is supported for in multilspy at the moment"

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

assert platform_id.value in [

"linux-x64"

], "Only linux-x64 is supported for in multilspy at the moment"

assert platform_id.value in [

"linux-x64",

"darwin-x64",

"win32-x64"

], "Platform not supported by multilspy at the moment"

codeowlai · 2025-06-21T12:54:54Z

src/multilspy/language_servers/clangd_language_server/clangd_language_server.py

+                    logger, dependency["url"], clangd_ls_dir, dependency["archiveType"]
+                )
+        assert os.path.exists(clangd_executable_path)
+        os.chmod(clangd_executable_path, stat.S_IEXEC)


🐛 Correctness Issue

Missing read permissions for executable.

The executable is set with execute-only permissions (S_IEXEC), but read permissions are also required to run the file.

Current Code (Diff):

- os.chmod(clangd_executable_path, stat.S_IEXEC) + os.chmod(clangd_executable_path, stat.S_IEXEC | stat.S_IREAD)

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

os.chmod(clangd_executable_path, stat.S_IEXEC)

os.chmod(clangd_executable_path, stat.S_IEXEC | stat.S_IREAD)

codeowlai · 2025-06-21T12:54:57Z

src/multilspy/language_servers/clangd_language_server/clangd_language_server.py

+                FileUtils.download_and_extract_archive(
+                    logger, dependency["url"], clangd_ls_dir, dependency["archiveType"]
+                )
+        assert os.path.exists(clangd_executable_path)


🐛 Correctness Issue

Missing error handling for executable.

The code asserts the executable exists but doesn't handle the case where it doesn't exist after download, causing a crash.

Current Code (Diff):

- assert os.path.exists(clangd_executable_path) + if not os.path.exists(clangd_executable_path): + raise FileNotFoundError(f"Failed to find or download clangd executable at {clangd_executable_path}")

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

assert os.path.exists(clangd_executable_path)

if not os.path.exists(clangd_executable_path):

raise FileNotFoundError(f"Failed to find or download clangd executable at {clangd_executable_path}")

codeowlai · 2025-06-21T12:55:00Z

tests/multilspy/test_multilspy_clangd.py

+    # generate build
+    os.system('cmake --build build')
+    # create sym link to compile commands at the top level
+    os.system('ln -s build/compile_commands.json .')


🐛 Correctness Issue

Platform Compatibility: Unix-specific command.

The 'ln -s' command is Unix-specific and will cause the test to fail on Windows systems, breaking cross-platform compatibility.

Current Code (Diff):

- os.system('ln -s build/compile_commands.json .') + import platform + if platform.system() == 'Windows': + import shutil + shutil.copy('build/compile_commands.json', '.') + else: + os.system('ln -s build/compile_commands.json .')

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

os.system('ln -s build/compile_commands.json .')

import platform

if platform.system() == 'Windows':

import shutil

shutil.copy('build/compile_commands.json', '.')

else:

os.system('ln -s build/compile_commands.json .')

codeowlai · 2025-06-21T12:55:03Z

tests/multilspy/test_multilspy_clangd.py

+    # switch to repo directory
+    os.chdir(source_directory_path)
+    # set cmake to export compile commands
+    os.system('cmake -DCMAKE_EXPORT_COMPILE_COMMANDS=1 -S . -B build')


🐛 Correctness Issue

Error Handling: Missing command execution checks.

Using os.system() without checking return codes can lead to silent failures if commands fail, causing test failures without clear error messages.

Current Code (Diff):

- os.system('cmake -DCMAKE_EXPORT_COMPILE_COMMANDS=1 -S . -B build') + import subprocess + try: + subprocess.run(['cmake', '-DCMAKE_EXPORT_COMPILE_COMMANDS=1', '-S', '.', '-B', 'build'], check=True) + except subprocess.CalledProcessError as e: + raise RuntimeError(f"Failed to configure CMake: {e}")

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

os.system('cmake -DCMAKE_EXPORT_COMPILE_COMMANDS=1 -S . -B build')

import subprocess

try:

subprocess.run(['cmake', '-DCMAKE_EXPORT_COMPILE_COMMANDS=1', '-S', '.', '-B', 'build'], check=True)

except subprocess.CalledProcessError as e:

raise RuntimeError(f"Failed to configure CMake: {e}")

codeowlai · 2025-06-21T12:55:07Z

tests/multilspy/test_multilspy_clangd.py

+    # set cmake to export compile commands
+    os.system('cmake -DCMAKE_EXPORT_COMPILE_COMMANDS=1 -S . -B build')
+    # generate build
+    os.system('cmake --build build')


🐛 Correctness Issue

Error Handling: Missing command execution checks.

Using os.system() without checking return codes can lead to silent failures if commands fail, causing test failures without clear error messages.

Current Code (Diff):

- os.system('cmake --build build') + try: + subprocess.run(['cmake', '--build', 'build'], check=True) + except subprocess.CalledProcessError as e: + raise RuntimeError(f"Failed to build project: {e}")

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

os.system('cmake --build build')

try:

subprocess.run(['cmake', '--build', 'build'], check=True)

except subprocess.CalledProcessError as e:

raise RuntimeError(f"Failed to build project: {e}")

Developer and others added 5 commits March 18, 2025 18:54

clangd language server commit

9f69f75

Merge branch 'microsoft:main' into main

6cb0fb2

Fix request definition UT failure

a8186b3

address review comments for clangd support

6c2565b

Merge pull request microsoft#84 from tomorrowCoder/main

ab58821

CPP Languager Server support (clang)

codeowlai bot reviewed Jun 21, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Replication of original PR #84: CPP Languager Server support (clang) #1

Replication of original PR #84: CPP Languager Server support (clang) #1

Uh oh!

yashwanthatla commented Jun 21, 2025 •

edited by codeowlai bot

Loading

Uh oh!

codeowlai bot Jun 21, 2025

Uh oh!

codeowlai bot Jun 21, 2025

Uh oh!

codeowlai bot Jun 21, 2025

Uh oh!

codeowlai bot Jun 21, 2025

Uh oh!

codeowlai bot Jun 21, 2025

Uh oh!

codeowlai bot Jun 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	os.chmod(clangd_executable_path, stat.S_IEXEC)
	os.chmod(clangd_executable_path, stat.S_IEXEC \| stat.S_IREAD)

	assert os.path.exists(clangd_executable_path)
	if not os.path.exists(clangd_executable_path):
	raise FileNotFoundError(f"Failed to find or download clangd executable at {clangd_executable_path}")

-    os.system('ln -s build/compile_commands.json .')
+    import platform
+    if platform.system() == 'Windows':
+        import shutil
+        shutil.copy('build/compile_commands.json', '.')
+    else:
+        os.system('ln -s build/compile_commands.json .')

-    os.system('cmake -DCMAKE_EXPORT_COMPILE_COMMANDS=1 -S . -B build')
+    import subprocess
+    try:
+        subprocess.run(['cmake', '-DCMAKE_EXPORT_COMPILE_COMMANDS=1', '-S', '.', '-B', 'build'], check=True)
+    except subprocess.CalledProcessError as e:
+        raise RuntimeError(f"Failed to configure CMake: {e}")

Replication of original PR #84: CPP Languager Server support (clang) #1

Are you sure you want to change the base?

Replication of original PR #84: CPP Languager Server support (clang) #1

Uh oh!

Conversation

yashwanthatla commented Jun 21, 2025 • edited by codeowlai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

Overview

Change Types

Affected Modules

Notes for Reviewers

Uh oh!

codeowlai bot Jun 21, 2025

Choose a reason for hiding this comment

Uh oh!

codeowlai bot Jun 21, 2025

Choose a reason for hiding this comment

Uh oh!

codeowlai bot Jun 21, 2025

Choose a reason for hiding this comment

Uh oh!

codeowlai bot Jun 21, 2025

Choose a reason for hiding this comment

Uh oh!

codeowlai bot Jun 21, 2025

Choose a reason for hiding this comment

Uh oh!

codeowlai bot Jun 21, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

yashwanthatla commented Jun 21, 2025 •

edited by codeowlai bot

Loading