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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman 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) {
----------------
aaronpuchert wrote:
> 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.
Good points -- we're likely fine here, but if we lack some test coverage for this, it'd be good to add some. If you find you need to add tests that wind up Just Working, feel free to commit them without review.


Repository:
  rC Clang

https://reviews.llvm.org/D52443