[PATCH] D52395: Thread safety analysis: Require exclusive lock for passing by non-const reference

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

> Unlike checking const qualifiers on member functions, there are probably not many false positives here: if a function takes a non-const reference, it will in almost all cases modify the object that we passed it.

I'm not certain I agree with the predicate here. We can make that inference when there *is* a const qualifier on the parameter, but I don't I think we can make any assumptions about whether it will or won't modify the object passed in in the absence of a const qualifier. This has come up in the past for things like `C(C&)` being a valid copy constructor despite the parameter being non-const. We might need some data to back this assertion up.



================
Comment at: lib/Analysis/ThreadSafety.cpp:2023
           QualType Qt = Pvd->getType();
-          if (Qt->isReferenceType())
-            checkAccess(Arg, AK_Read, POK_PassByRef);
+          if (const auto* RT = dyn_cast<const ReferenceType>(&*Qt)) {
+            if (RT->getPointeeType().isConstQualified())
----------------
This isn't specific to your changes, but this gives me the impression we don't distinguish between rvalue references and lvalue references, but that may be something of interest in here.


Repository:
  rC Clang

https://reviews.llvm.org/D52395