OikawaKirie added inline comments.

================
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;
----------------
steakhal wrote:
> OikawaKirie wrote:
> > steakhal wrote:
> > > 
> > The source of lookup name of the function being imported is function 
> > `CrossTranslationUnitContext::getLookupName`. Keeping the length in the 
> > mapping can avoid parsing the lookup name during importing.
> Okay; you can copy the original StringRef to have that. But by consuming it 
> on one path makes the code much more readable.
> 
The `getAsInterger` call can also check whether the content before the first 
colon is an integer. Therefore, a sub-string operation is required here.


================
Comment at: clang/test/Analysis/Inputs/ctu-lookup-name-with-space.cpp:7-8
+    // CHECK-NOT: multiple definitions are found for the same key in index
+    f([](char) -> int { return 42; });
+    f([](char) -> bool { return true; });
+  }
----------------
steakhal wrote:
> I would rather put these into the `importee()`
The lambda exprs will not be included in the CTU index file if they are 
declared in a normal function.


================
Comment at: clang/test/Analysis/Inputs/ctu-lookup-name-with-space.cpp:13
+int importee(int X) {
+  return 1 / X;
+}
----------------
steakhal wrote:
> Why do you need to have a div by zero warning?
I am not sure whether we should test if an external function can be correctly 
imported. Hence, I wrote a div-by-zero bug here. A call to function 
`clang_analyzer_warnIfReached` is also OK here.

As the imported lambda expr will not be called, I think I can only test whether 
CTU works via another normal function.


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

https://reviews.llvm.org/D102669

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to