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

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

Looks good.
Please get rid of the macro stuff, consider something along the lines I proposed for the parsing stuff.
Also clang-format the code you touch.
I haven't checked the docs and the comments of the codebase, but I'll assume you grepped and fixed all occurrences.
I look forward to this, thank you for working on this @OikawaKirie.



================
Comment at: clang/docs/analyzer/user-docs/CrossTranslationUnit.rst:84
 The next step is to create a CTU index file which holds the `USR` name and location of external definitions in the
-source files:
+source files in format `USR-Length:USR File-Path`:
 
----------------



================
Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:154-172
+  // Find the length delimiter.
+  const size_t LengthDelimiter = LineRef.find(':');
+  if (StringRef::npos == LengthDelimiter)
+    return false;
+
+  // Parse the length of LookupName as USRLength.
+  size_t USRLength = 0;
----------------



================
Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:175-180
+#define IS_CTU_INDEX_KEY_VALID(FUNCTION_NAME)                                  \
+  ([](StringRef N) {                                                           \
+    std::string FN = N.str() + ' ';                                            \
+    StringRef LN, FP;                                                          \
+    return parseCrossTUIndexItem(FN, LN, FP) && N == LN && FP.empty();         \
+  }(FUNCTION_NAME))
----------------
Please do something about this macro.
Encapsulate the logic in some other way.


================
Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:464
 
     // Check if there is and entry in the index for the function.
+    assert(IS_CTU_INDEX_KEY_VALID(FunctionName) &&
----------------



================
Comment at: clang/test/Analysis/func-mapping-test.cpp:50
   const int ui;
 };
----------------
I think you could add your lambda stuff to this file.
This is really the place for testing this.
The test you created actually demonstrating the CTU issue is also valuable IMO, so you can leave it, but have a copy here.


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

https://reviews.llvm.org/D102669