[PATCH] D52443: Thread safety analysis: Examine constructor arguments

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added inline comments.


================
Comment at: lib/Analysis/ThreadSafety.cpp:1970
+  // There can be default arguments, so we stop when one iterator is at end().
+  for (auto Arg = ArgBegin; Param != Params.end() && Arg != ArgEnd;
+       ++Param, ++Arg) {
----------------
aaron.ballman wrote:
> How should this interact with variadic functions? Either ones that use `...` with a C varargs function, or ones that use parameter packs in C++. (I realize this is basically existing code, but the question remains.)
For instantiated variadic templates we match against the instantiation of the template, so we can match the parameters just as for an ordinary function. My understanding is that the thread safety analysis doesn't run over templates, only instantiations, so that should be fine.

With C variadic arguments we should have a shorter parameter list, so we don't check the matching variadic arguments. However, if I'm not mistakten, the variadic arguments are passed by value, so the reference checks aren't needed.

Maybe I can add some tests for these cases.


================
Comment at: lib/Analysis/ThreadSafety.cpp:2050
+  } else {
+    ExamineCallArguments(D, Exp->arg_begin(), Exp->arg_end(), false);
   }
----------------
aaron.ballman wrote:
> Can you add a comment for the bool argument? e.g., `/*OperatorFun*/false`
Depending on `OperatorFun`, we shift some of the iterators. We could also do that on the caller site. I'll see if that looks better.


Repository:
  rC Clang

https://reviews.llvm.org/D52443