[PATCH] D12945: [PATCH] Add checker for objects that should not be value types

[Alexander Kornienko via cfe-commits]

alexfh added inline comments.

================
Comment at: clang-tidy/misc/NonCopyableObjects.cpp:21
@@ +20,3 @@
+  static const char *TypeNames[] = {
+    "::pthread_cond_t",
+    "::pthread_mutex_t",
----------------
aaron.ballman wrote:
> alexfh wrote:
> > How about making these lists configurable or adding a list for custom type names that should be checked in a similar way?
> Do we have a helper function for making lists like these configurable? If so, I'll gladly use it. If not, perhaps we could make some helper functionality and then implement configurability at that time in a more comprehensive way?
I tried to make clang-tidy checks configurable in a more type-safe way (http://reviews.llvm.org/D5602), but never got time to complete this. So no, currently we don't have any facilities to make this kind of configuration easier. Making these lists configurable is also not a precondition to submitting this patch. It was just an idea of an improvement.

================
Comment at: clang-tidy/misc/NonCopyableObjects.cpp:81
@@ +80,3 @@
+  if (D && BD)
+    diag(D->getLocation(), "'%0' declared as type '%1'; did you mean '%1 *'?")
+        << D->getName() << BD->getName();
----------------
aaron.ballman wrote:
> alexfh wrote:
> > I think, error messages should contain some explanation of why is this wrong. Not sure if this can be fit into a reasonable number of words, but we have to try.
> Excellent point! How about:
> 
> '%0' declared as unsafely-copyable type '%1'; did you mean '%1 *'
> 
> or
> 
> '%0' declared as type '%1', which is unsafe to copy' did you mean '%1 *'?
The latter seems easier to read. Thanks!


http://reviews.llvm.org/D12945