[NFC] [C++20] [Modules] [clangd] Add test for code completion for C++20 Named Modules

[ChuanqiXu]

Summary

For issue https://github.com/llvm/llvm-project/issues/58723. Although we can't say the current status is good (enough), it should be good to add some tests to prevent the current status become worse.

[ChuanqiXu]

clang-tools-extra/clangd/test/completion-modules2.test

14

It looks redundant at the first sight. I know we can reduce the size by the following trick:

#ifdef USE_EXPORT
export
#endif
void printA()

...

But this style will become very complex soon as long as the the size increases. An example could be found at: https://github.com/llvm/llvm-project/blob/main/clang/test/CXX/module/module.interface/p2.cpp. It was pretty painful for me to understand and edit the test cases. So I don't like such styles. I prefer the psychology that the somebody's redundancy is another's clearness. And the redundant test cases looks not a problem to me.

[ChuanqiXu]

Add dependency to clangd tests to make sure they can find the corresponding rules.

[ChuanqiXu]

Set the path to clang correctly.

[ChuanqiXu]

The failed tests in windows says:

Failed to find compilation database for \\C:\ws\w9\llvm-project\premerge-checks\build\tools\clang\tools\extra\clangd\test\Output\completion-modules.test.tmp\Use.cpp

[sammccall]

I think at this point we specifically don't want to commit to preserving particular modules behavior, this is more or less what the unsupported state means (https://github.com/clangd/clangd/issues/1293).

In particular the pattern where modules are built externally by clang, clang must be version-locked to clangd, and module content is never indexed, doesn't seem to be a good foundation for most users, so having the freedom to tear it down later seems better than encouraging people to rely on it.

clang-tools-extra/clangd/test/CMakeLists.txt

2

A dependency on clang here probably isn't acceptable:

• it's a *large* increase in what needs to be built for check-clangd, which we try quite hard to keep small
• the conceptual reason is that clangd is consuming modules built by (version-locked) clang, for various reasons this shouldn't be required (not least: users likely don't have a version-locked clang installed)

clang-tools-extra/clangd/test/completion-modules.test

2

We generally find lit tests too hard to maintain for this sort of thing.

In particular:

• lots of noise added to each test
• failures are too hard to parse
• too much redundant setup work duplicated in each testcase
• substituting filenames tends to run into escaping issues with windows and backslashes, etc
• test scope ends up being too large
• use of real filesystem doesn't verify the implementation is VFS-clean

It's usually OK to have a single smoke test if needed, but generally these are written as unittests.

[ChuanqiXu]

Thanks for the explanation. I got your point.

clang-tools-extra/clangd/test/CMakeLists.txt

 set(CLANGD_TEST_DEPS
+  clang
   clangd
   ClangdTests
   clangd-indexer
   # No tests for it, but we should still make sure they build.
   dexp
+  split-file
   )
 
 if(CLANGD_BUILD_XPC)
   list(APPEND CLANGD_TEST_DEPS clangd-xpc-test-client)
   list(APPEND CLANGD_TEST_DEPS ClangdXpcUnitTests)
 endif()
 
 if(CLANGD_ENABLE_REMOTE)

clang-tools-extra/clangd/test/completion-modules.test

+# Tests that the importee can find the exported entities.
+# RUN: rm -fr %t
+# RUN: mkdir -p %t
+# RUN: split-file %s %t
+#
+# RUN: sed -e "s|DIR|%/t|g" %t/compile_commands.json.tmpl > %t/compile_commands.json
+# RUN: sed -e "s|DIR|%/t|g" %t/definition.jsonrpc.tmpl > %t/definition.jsonrpc
+# RUN: %clang++ -std=c++20 %t/A.cppm --precompile -o %t/A.pcm
+# RUN: clangd -lit-test < %t/definition.jsonrpc  | FileCheck -strict-whitespace %t/definition.jsonrpc
+
+#--- A.cppm
+export module A;
+export void printA() {}
+
+#--- Use.cpp
+import A;
+void foo() {
+    print
+}
+
+#--- compile_commands.json.tmpl
+[
+    {
+      "directory": "DIR",
+      "command": "clang++  -fprebuilt-module-path=DIR -std=c++20 -o DIR/main.cpp.o -c DIR/Use.cpp",
+      "file": "DIR/Use.cpp"
+    }
+]
+
+#--- definition.jsonrpc.tmpl
+{
+  "jsonrpc": "2.0",
+  "id": 0,
+  "method": "initialize",
+  "params": {
+    "processId": 123,
+    "rootPath": "clangd",
+    "capabilities": {
+      "textDocument": {
+        "completion": {
+          "completionItem": {
+            "snippetSupport": true
+          }
+        }
+      }
+    },
+    "trace": "off"
+  }
+}
+---
+{
+  "jsonrpc": "2.0",
+  "method": "textDocument/didOpen",
+  "params": {
+    "textDocument": {
+      "uri": "file://DIR/Use.cpp",
+      "languageId": "cpp",
+      "version": 1,
+      "text": "import A;\nvoid foo() {\n    print\n}\n"
+    }
+  }
+}
+
+# CHECK:  "id": 1,
+# CHECK-NEXT:  "jsonrpc": "2.0",
+# CHECK-NEXT:  "result": {
+# CHECK-NEXT:    "isIncomplete": false,
+# CHECK-NEXT:    "items": [
+# CHECK-NEXT:      {
+# CHECK-NEXT:        "detail": "void",
+# CHECK-NEXT:        "filterText": "printA",
+# CHECK-NEXT:        "insertText": "printA()",
+# CHECK-NEXT:        "insertTextFormat": 2,
+# CHECK-NEXT:        "kind": 3,
+# CHECK-NEXT:        "label": " printA()",
+# CHECK-NEXT:        "score":
+# CHECK-NEXT:        "sortText": "40b10a1eprintA",
+# CHECK-NEXT:        "textEdit": {
+# CHECK-NEXT:          "newText": "printA()",
+# CHECK-NEXT:          "range": {
+# CHECK-NEXT:            "end": {
+# CHECK-NEXT:              "character": 6,
+# CHECK-NEXT:              "line": 2
+# CHECK-NEXT:            },
+# CHECK-NEXT:            "start": {
+# CHECK-NEXT:              "character": 4,
+# CHECK-NEXT:              "line": 2
+# CHECK-NEXT:            }
+# CHECK-NEXT:          }
+
+---
+{"jsonrpc":"2.0","id":1,"method":"textDocument/completion","params":{"textDocument":{"uri":"file://DIR/Use.cpp"},"context":{"triggerKind":1},"position":{"line":2,"character":6}}}
+---
+{"jsonrpc":"2.0","id":2,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}

clang-tools-extra/clangd/test/completion-modules2.test

+# Tests that the importee can't find the exported entities.
+# RUN: rm -fr %t
+# RUN: mkdir -p %t
+# RUN: split-file %s %t
+#
+# RUN: sed -e "s|DIR|%/t|g" %t/compile_commands.json.tmpl > %t/compile_commands.json
+# RUN: sed -e "s|DIR|%/t|g" %t/definition.jsonrpc.tmpl > %t/definition.jsonrpc
+# RUN: %clang++ -std=c++20 %t/A.cppm --precompile -o %t/A.pcm
+# RUN: clangd -lit-test < %t/definition.jsonrpc  | FileCheck %t/definition.jsonrpc
+
+#--- A.cppm
+export module A;
+void printA() {}
+
+#--- Use.cpp
+import A;
+void foo() {
+    print
+}
+
+#--- compile_commands.json.tmpl
+[
+    {
+      "directory": "DIR",
+      "command": "clang++  -fprebuilt-module-path=DIR -std=c++20 -o DIR/main.cpp.o -c DIR/Use.cpp",
+      "file": "DIR/Use.cpp"
+    }
+]
+
+#--- definition.jsonrpc.tmpl
+{
+  "jsonrpc": "2.0",
+  "id": 0,
+  "method": "initialize",
+  "params": {
+    "processId": 123,
+    "rootPath": "clangd",
+    "capabilities": {
+      "textDocument": {
+        "completion": {
+          "completionItem": {
+            "snippetSupport": true
+          }
+        }
+      }
+    },
+    "trace": "off"
+  }
+}
+---
+{
+  "jsonrpc": "2.0",
+  "method": "textDocument/didOpen",
+  "params": {
+    "textDocument": {
+      "uri": "file://DIR/Use.cpp",
+      "languageId": "cpp",
+      "version": 1,
+      "text": "import A;\nvoid foo() {\n    print\n}\n"
+    }
+  }
+}
+
+# CHECK-NOT: "filterText": "printA"
+
+---
+{"jsonrpc":"2.0","id":1,"method":"textDocument/completion","params":{"textDocument":{"uri":"file://DIR/Use.cpp"},"context":{"triggerKind":1},"position":{"line":2,"character":6}}}
+---
+{"jsonrpc":"2.0","id":2,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}

clang-tools-extra/clangd/test/completion-modules3.test

+# Tests that the importee can find the exported entities.
+# RUN: rm -fr %t
+# RUN: mkdir -p %t
+# RUN: split-file %s %t
+#
+# RUN: sed -e "s|DIR|%/t|g" %t/compile_commands.json.tmpl > %t/compile_commands.json
+# RUN: sed -e "s|DIR|%/t|g" %t/definition.jsonrpc.tmpl > %t/definition.jsonrpc
+# RUN: %clang++ -std=c++20 %t/A.cppm --precompile -o %t/A.pcm
+# RUN: clangd -lit-test < %t/definition.jsonrpc  | FileCheck -strict-whitespace %t/definition.jsonrpc
+
+#--- A.cppm
+export module A;
+void printA() {}
+
+#--- Use.cpp
+export module A:part;
+import A;
+void foo() {
+    print
+}
+
+#--- compile_commands.json.tmpl
+[
+    {
+      "directory": "DIR",
+      "command": "clang++  -fprebuilt-module-path=DIR -std=c++20 -o DIR/main.cpp.o -c DIR/Use.cpp",
+      "file": "DIR/Use.cpp"
+    }
+]
+
+#--- definition.jsonrpc.tmpl
+{
+  "jsonrpc": "2.0",
+  "id": 0,
+  "method": "initialize",
+  "params": {
+    "processId": 123,
+    "rootPath": "clangd",
+    "capabilities": {
+      "textDocument": {
+        "completion": {
+          "completionItem": {
+            "snippetSupport": true
+          }
+        }
+      }
+    },
+    "trace": "off"
+  }
+}
+---
+{
+  "jsonrpc": "2.0",
+  "method": "textDocument/didOpen",
+  "params": {
+    "textDocument": {
+      "uri": "file://DIR/Use.cpp",
+      "languageId": "cpp",
+      "version": 1,
+      "text": "export module A:part;\nimport A;\nvoid foo() {\n    print\n}\n"
+    }
+  }
+}
+
+# CHECK:  "id": 1,
+# CHECK-NEXT:  "jsonrpc": "2.0",
+# CHECK-NEXT:  "result": {
+# CHECK-NEXT:    "isIncomplete": false,
+# CHECK-NEXT:    "items": [
+# CHECK-NEXT:      {
+# CHECK-NEXT:        "detail": "void",
+# CHECK-NEXT:        "filterText": "printA",
+# CHECK-NEXT:        "insertText": "printA()",
+# CHECK-NEXT:        "insertTextFormat": 2,
+# CHECK-NEXT:        "kind": 3,
+# CHECK-NEXT:        "label": " printA()",
+# CHECK-NEXT:        "score":
+# CHECK-NEXT:        "sortText": "409fc4f5printA",
+# CHECK-NEXT:        "textEdit": {
+# CHECK-NEXT:          "newText": "printA()",
+# CHECK-NEXT:          "range": {
+# CHECK-NEXT:            "end": {
+# CHECK-NEXT:              "character": 6,
+# CHECK-NEXT:              "line": 3
+# CHECK-NEXT:            },
+# CHECK-NEXT:            "start": {
+# CHECK-NEXT:              "character": 4,
+# CHECK-NEXT:              "line": 3
+# CHECK-NEXT:            }
+# CHECK-NEXT:          }
+
+---
+{"jsonrpc":"2.0","id":1,"method":"textDocument/completion","params":{"textDocument":{"uri":"file://DIR/Use.cpp"},"context":{"triggerKind":1},"position":{"line":3,"character":6}}}
+---
+{"jsonrpc":"2.0","id":2,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}

clang-tools-extra/clangd/test/completion-modules4.test

+# Tests that the importee can't find the entities in the global module fragment.
+# RUN: rm -fr %t
+# RUN: mkdir -p %t
+# RUN: split-file %s %t
+#
+# RUN: sed -e "s|DIR|%/t|g" %t/compile_commands.json.tmpl > %t/compile_commands.json
+# RUN: sed -e "s|DIR|%/t|g" %t/definition.jsonrpc.tmpl > %t/definition.jsonrpc
+# RUN: %clang++ -std=c++20 %t/A.cppm --precompile -o %t/A.pcm
+# RUN: clangd -lit-test < %t/definition.jsonrpc  | FileCheck %t/definition.jsonrpc
+
+#--- foo.h
+void printA() {}
+
+#--- A.cppm
+module;
+#include "foo.h"
+export module A;
+
+
+#--- Use.cpp
+import A;
+void foo() {
+    print
+}
+
+#--- compile_commands.json.tmpl
+[
+    {
+      "directory": "DIR",
+      "command": "clang++  -fprebuilt-module-path=DIR -std=c++20 -o DIR/main.cpp.o -c DIR/Use.cpp",
+      "file": "DIR/Use.cpp"
+    }
+]
+
+#--- definition.jsonrpc.tmpl
+{
+  "jsonrpc": "2.0",
+  "id": 0,
+  "method": "initialize",
+  "params": {
+    "processId": 123,
+    "rootPath": "clangd",
+    "capabilities": {
+      "textDocument": {
+        "completion": {
+          "completionItem": {
+            "snippetSupport": true
+          }
+        }
+      }
+    },
+    "trace": "off"
+  }
+}
+---
+{
+  "jsonrpc": "2.0",
+  "method": "textDocument/didOpen",
+  "params": {
+    "textDocument": {
+      "uri": "file://DIR/Use.cpp",
+      "languageId": "cpp",
+      "version": 1,
+      "text": "import A;\nvoid foo() {\n    print\n}\n"
+    }
+  }
+}
+
+# CHECK-NOT: "filterText": "printA"
+
+---
+{"jsonrpc":"2.0","id":1,"method":"textDocument/completion","params":{"textDocument":{"uri":"file://DIR/Use.cpp"},"context":{"triggerKind":1},"position":{"line":2,"character":6}}}
+---
+{"jsonrpc":"2.0","id":2,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}