[PATCH] D102669: [analyzer][ctu] Fix wrong 'multiple definitions' errors caused by space characters in lookup names when parsing the ctu index file

[Ella Ma via Phabricator via cfe-commits]

OikawaKirie marked an inline comment as done.
OikawaKirie added a comment.

In D102669#2765233 <https://reviews.llvm.org/D102669#2765233>, @steakhal wrote:

> I'm not really familiar with the extdefmap part, but I'm surprised that we are using spaces as separators.
> Shouldn't we consider using a different character?

I prefer the idea of changing the delimiter character, but it may lead to modifying a lot of test cases.
I think we'd better make this change in another revision in the future if we do want to change it.



================
Comment at: clang/test/Analysis/ctu-lookup-name-with-space.cpp:7
+// RUN: echo '"%t/importee.cpp" : ["g++", "-c", "%t/importee.cpp"]' > %t/invocations.yaml
+// RUN: echo '[{"directory": "%t", "command": "g++ -c %t/importee.cpp", "file": "%t/importee.cpp"}, {"directory": "%t", "command": "g++ -c %t/trigger.cpp", "file": "%t/trigger.cpp"}]' > %t/compile_commands.json
+
----------------
steakhal wrote:
> Probably splitting this up into multiple lines would result in a more readable solution.
> 
> Something along these lines should work:
> ```
> cat >%t/compile_commands.json <<EOL
> line 1
> line 2
> ...
> EOL
> ```
The suggestion is great, however I cannot find a way to write the `RUN` commands.
Could you please tell me how to write the commands in this way? It is also useful to help me merging the test case into one file.


================
Comment at: clang/test/Analysis/ctu-lookup-name-with-space.cpp:11-23
+// RUN: cd %t && %clang_cc1 -fsyntax-only -analyze \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=. \
+// RUN:   -analyzer-config ctu-invocation-list=invocations.yaml \
+// RUN:   trigger.cpp 2>&1 | FileCheck importee.cpp
+
----------------
steakhal wrote:
> Why do you need two separate invocations? Couldn't you just merge these?
> I've seen cases where `-verify` was used in conjunction with `FileCheck`.
I forgot the `--allow-empty` argument during writing this test case. I will merge them in an update.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102669/new/

https://reviews.llvm.org/D102669