diff mbox

[C++] ctor privacy

Message ID 41d815d8-c023-b804-7d4e-f363b52958ce@acm.org
State New
Headers show

Commit Message

Nathan Sidwell July 17, 2017, 4:55 p.m. UTC 
While futzing around with ctor lookup I discovered this warning about 
overly-private classes.

Originally we'd allow a public copy-ctor to be sufficiently public, but 
as the comment says, you need an already-constructed object for that to 
work.  so this implements that check -- public copy or move ctors do not 
make the class sufficiently public.

I didn't implement the further suggested check of ignoring a ctor that 
takes an already constructed object -- be it copy ctor or not.

applied to trunk.

nathan
diff mbox

Patch

2017-07-17  Nathan Sidwell  <nathan@acm.org>

	* class.c (maybe_warn_about_overly_private_class): Ignore public
	copy ctors.

	* g++.dg/warn/ctor-dtor-privacy-3.C: New.

Index: cp/class.c
===================================================================
--- cp/class.c	(revision 250280)
+++ cp/class.c	(working copy)
@@ -2240,10 +2240,10 @@  maybe_warn_about_overly_private_class (t
   /* Warn about classes that have private constructors and no friends.  */
   if (TYPE_HAS_USER_CONSTRUCTOR (t)
       /* Implicitly generated constructors are always public.  */
-      && (!CLASSTYPE_LAZY_DEFAULT_CTOR (t)
-	  || !CLASSTYPE_LAZY_COPY_CTOR (t)))
+      && !CLASSTYPE_LAZY_DEFAULT_CTOR (t))
     {
       bool nonprivate_ctor = false;
+      tree copy_or_move = NULL_TREE;
 
       /* If a non-template class does not define a copy
 	 constructor, one is defined for it, enabling it to avoid
@@ -2260,13 +2260,15 @@  maybe_warn_about_overly_private_class (t
       else
 	for (ovl_iterator iter (CLASSTYPE_CONSTRUCTORS (t));
 	     !nonprivate_ctor && iter; ++iter)
-	  /* Ideally, we wouldn't count copy constructors (or, in
-	     fact, any constructor that takes an argument of the class
-	     type as a parameter) because such things cannot be used
-	     to construct an instance of the class unless you already
-	     have one.  But, for now at least, we're more
-	     generous.  */
-	  if (! TREE_PRIVATE (*iter))
+	  if (TREE_PRIVATE (*iter))
+	    continue;
+	  else if (copy_fn_p (*iter) || move_fn_p (*iter))
+	    /* Ideally, we wouldn't count any constructor that takes
+	       an argument of the class type as a parameter, because
+	       such things cannot be used to construct an instance of
+	       the class unless you already have one.  */
+	    copy_or_move = *iter;
+	  else
 	    nonprivate_ctor = true;
 
       if (!nonprivate_ctor)
@@ -2274,6 +2276,10 @@  maybe_warn_about_overly_private_class (t
 	  warning (OPT_Wctor_dtor_privacy,
 		   "%q#T only defines private constructors and has no friends",
 		   t);
+	  if (copy_or_move)
+	    inform (DECL_SOURCE_LOCATION (copy_or_move),
+		    "%q#D is public, but requires an existing %q#T object",
+		    copy_or_move, t);
 	  return;
 	}
     }
Index: testsuite/g++.dg/warn/ctor-dtor-privacy-3.C
===================================================================
--- testsuite/g++.dg/warn/ctor-dtor-privacy-3.C	(revision 0)
+++ testsuite/g++.dg/warn/ctor-dtor-privacy-3.C	(working copy)
@@ -0,0 +1,20 @@ 
+// { dg-do compile { target c++11 } }
+// { dg-additional-options "-Wctor-dtor-privacy" } 
+
+class X // { dg-message "only defines private" }
+{
+public:
+  X (X const &); // { dg-message "requires an existing" }
+};
+
+class Y // { dg-message "only defines private" }
+{
+public:
+  Y (Y &&);  // { dg-message "requires an existing" }
+};
+
+class Z
+{
+public:
+  Z (int);
+};