https://github.com/HighCommander4 commented:

Thanks for the patch!

The code change looks reasonable to me.

It would be good to add an associated test case as well. I haven't written any 
code completion test cases that involve modules yet, but I had a look around 
and 
[macros-in-modules.c](https://searchfox.org/llvm/source/clang/test/CodeCompletion/macros-in-modules.c)
 seems to be an example.

The general idea is:

 * Use commands in `// RUN` lines to set up the test case, including any 
dependent files
    * Note: this test creates some files using `echo`, but you can also just 
check in files to the directory `clang/test/CodeCompletion/Inputs` and 
reference those
 * Then run a `c-index-test -code-completion-at` command to invoke code 
completion. `%s` means "the current file".
 * The output of that command will list what the code completion proposals are, 
and the `FileCheck` at the end of the command together with `// CHECK` lines 
like 
[this](https://searchfox.org/llvm/rev/b4084bd213224799d350359a89235a41b3297f4a/clang/test/CodeCompletion/macros-in-modules.c#11)
 allow making assertions about the contents of the proposals.

The test can be run with `<build>/bin/llvm-lit -v 
clang/test/CodeCompletion/<testname>.cpp`.

Note that this particular test case pre-dates support for standard modules (and 
it's also a C language test case), so it uses a module map which translates an 
`#include` to a module. We can write a C++ language test case and use standard 
syntax like `import`.

Let me know if that makes sense / if you have any issues trying to sort the 
test out.

https://github.com/llvm/llvm-project/pull/187657
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to