On 10/09/2017 06:15 PM, Nathan Sidwell wrote:
On 10/09/2017 11:57 AM, Jason Merrill wrote:

Hmm, why do we only check extern "C" conflicts for functions?

I suspect a bug.  I noticed it as existing behaviour and was puzzled the first time around rearranging this code, but didn't want to get distracted from the big picture.

This patch checks both vars and fns (as typedefs don't have linkage, I don't think they can be extern C, but ICBW).

I also tweaked the diagnostics, so that the first one is a pedwarn and the later ones are notes. (so we won't arbitrarily truncate the series of diagnostics.

Look good to you?

nathan
--
Nathan Sidwell
2017-10-10  Nathan Sidwell  <nat...@acm.org>

	* name-lookup.c (extern_c_fns): Rename to ...
	(extern_c_decls): ... here.
	(check_extern_c_conflict, extern_c_linkage_bindings): Update.
	(do_pushdecl): Check extern-c fns and vars.

	* g++.dg/lookup/extern-c-redecl6.C: New.
	* g++.dg/lookup/extern-c-hidden.C: Adjust diagnostics.
	* g++.dg/lookup/extern-c-redecl.C: Likewise.
	* g++.old-deja/g++.other/using9.C: Likewise.

Index: cp/name-lookup.c
===================================================================
--- cp/name-lookup.c	(revision 253605)
+++ cp/name-lookup.c	(working copy)
@@ -2511,13 +2511,13 @@ update_binding (cp_binding_level *level,
   return decl;
 }
 
-/* Table of identifiers to extern C functions (or LISTS thereof).  */
+/* Table of identifiers to extern C declarations (or LISTS thereof).  */
 
-static GTY(()) hash_table<named_decl_hash> *extern_c_fns;
+static GTY(()) hash_table<named_decl_hash> *extern_c_decls;
 
-/* DECL has C linkage. If we have an existing instance, make sure it
-   has the same exception specification [7.5, 7.6].  If there's no
-   instance, add DECL to the map.  */
+/* DECL has C linkage. If we have an existing instance, make sure the
+   new one is compatible.  Make sure it has the same exception
+   specification [7.5, 7.6].  Add DECL to the map.  */
 
 static void
 check_extern_c_conflict (tree decl)
@@ -2526,10 +2526,10 @@ check_extern_c_conflict (tree decl)
   if (DECL_ARTIFICIAL (decl) || DECL_IN_SYSTEM_HEADER (decl))
     return;
 
-  if (!extern_c_fns)
-    extern_c_fns = hash_table<named_decl_hash>::create_ggc (127);
+  if (!extern_c_decls)
+    extern_c_decls = hash_table<named_decl_hash>::create_ggc (127);
 
-  tree *slot = extern_c_fns
+  tree *slot = extern_c_decls
     ->find_slot_with_hash (DECL_NAME (decl),
 			   IDENTIFIER_HASH_VALUE (DECL_NAME (decl)), INSERT);
   if (tree old = *slot)
@@ -2543,9 +2543,10 @@ check_extern_c_conflict (tree decl)
 	     about a (possible) mismatch, when inserting the decl.  */
       else if (!decls_match (decl, old))
 	mismatch = 1;
-      else if (!comp_except_specs (TYPE_RAISES_EXCEPTIONS (TREE_TYPE (old)),
-				   TYPE_RAISES_EXCEPTIONS (TREE_TYPE (decl)),
-				   ce_normal))
+      else if (TREE_CODE (decl) == FUNCTION_DECL
+	       && !comp_except_specs (TYPE_RAISES_EXCEPTIONS (TREE_TYPE (old)),
+				      TYPE_RAISES_EXCEPTIONS (TREE_TYPE (decl)),
+				      ce_normal))
 	mismatch = -1;
       else if (DECL_ASSEMBLER_NAME_SET_P (old))
 	SET_DECL_ASSEMBLER_NAME (decl, DECL_ASSEMBLER_NAME (old));
@@ -2553,12 +2554,12 @@ check_extern_c_conflict (tree decl)
       if (mismatch)
 	{
 	  pedwarn (input_location, 0,
-		   "declaration of %q#D with C language linkage", decl);
-	  pedwarn (DECL_SOURCE_LOCATION (old), 0,
-		   "conflicts with previous declaration %q#D", old);
+		   "conflicting C language linkage declaration %q#D", decl);
+	  inform (DECL_SOURCE_LOCATION (old),
+		  "previous declaration %q#D", old);
 	  if (mismatch < 0)
-	    pedwarn (input_location, 0,
-		     "due to different exception specifications");
+	    inform (input_location,
+		    "due to different exception specifications");
 	}
       else
 	{
@@ -2587,8 +2588,8 @@ check_extern_c_conflict (tree decl)
 tree
 c_linkage_bindings (tree name)
 {
-  if (extern_c_fns)
-    if (tree *slot = extern_c_fns
+  if (extern_c_decls)
+    if (tree *slot = extern_c_decls
 	->find_slot_with_hash (name, IDENTIFIER_HASH_VALUE (name), NO_INSERT))
       {
 	tree result = *slot;
@@ -3030,9 +3031,8 @@ do_pushdecl (tree decl, bool is_friend)
 		    else
 		      *slot = head;
 		  }
-		if (TREE_CODE (match) == FUNCTION_DECL
-		    && DECL_EXTERN_C_P (match))
-		  /* We need to check and register the fn now.  */
+		if (DECL_EXTERN_C_P (match))
+		  /* We need to check and register the decl now.  */
 		  check_extern_c_conflict (match);
 	      }
 	    return match;
@@ -3113,7 +3113,9 @@ do_pushdecl (tree decl, bool is_friend)
 	}
       else if (VAR_P (decl))
 	maybe_register_incomplete_var (decl);
-      else if (TREE_CODE (decl) == FUNCTION_DECL && DECL_EXTERN_C_P (decl))
+
+      if ((VAR_P (decl) || TREE_CODE (decl) == FUNCTION_DECL)
+	  && DECL_EXTERN_C_P (decl))
 	check_extern_c_conflict (decl);
     }
   else
Index: testsuite/g++.dg/lookup/extern-c-hidden.C
===================================================================
--- testsuite/g++.dg/lookup/extern-c-hidden.C	(revision 253605)
+++ testsuite/g++.dg/lookup/extern-c-hidden.C	(working copy)
@@ -1,11 +1,11 @@
 // Make sure unhidding an extern-c still checks it is compatible
 
-extern "C" float fabsf (float);  // { dg-error "conflicts with previous declaration" }
+extern "C" float fabsf (float);  // { dg-message "previous declaration" }
 
 namespace Bob 
 {
   extern "C" float fabsf (float, float); // { dg-error "C language" }
-  extern "C" double fabs (double, double); // { dg-error "conflicts with previous declaration" }
+  extern "C" double fabs (double, double); // { dg-message "previous declaration" }
 }
 
 extern "C" double fabs (double); // { dg-error "C language" }
Index: testsuite/g++.dg/lookup/extern-c-redecl.C
===================================================================
--- testsuite/g++.dg/lookup/extern-c-redecl.C	(revision 253605)
+++ testsuite/g++.dg/lookup/extern-c-redecl.C	(working copy)
@@ -3,7 +3,7 @@
 // { dg-do compile }
 
 namespace A {
-    extern "C" void foo_func () throw(); // { dg-error "conflicts" }
+    extern "C" void foo_func () throw(); // { dg-message "previous" }
 }
 // next line should trigger an error because
 // it conflicts with previous declaration of foo_func (), due to
Index: testsuite/g++.dg/lookup/extern-c-redecl6.C
===================================================================
--- testsuite/g++.dg/lookup/extern-c-redecl6.C	(revision 0)
+++ testsuite/g++.dg/lookup/extern-c-redecl6.C	(working copy)
@@ -0,0 +1,25 @@
+extern "C" {
+  int i; // { dg-message "previous" }
+  float f; // { dg-message "previous" }
+  void fn (); // { dg-message "previous" }
+  int ai1[1]; // { dg-message "previous" }
+  extern int ai[];
+
+  namespace OK
+  {
+    int i;
+    float f;
+    void fn ();
+    extern int ai1[];
+    int ai[2];
+  }
+
+  namespace BAD
+  {
+    long i; // { dg-error "C language linkage" }
+    double f; // { dg-error "C language linkage" }
+    int fn (); // { dg-error "C language linkage" }
+    int ai1[2]; // { dg-error "C language linkage" }
+  }
+}
+
Index: testsuite/g++.old-deja/g++.other/using9.C
===================================================================
--- testsuite/g++.old-deja/g++.other/using9.C	(revision 253605)
+++ testsuite/g++.old-deja/g++.other/using9.C	(working copy)
@@ -13,7 +13,7 @@ struct x {};
 using ::x;
 using ::a;
 
-extern "C" void foo ();		// { dg-error "previous declaration" }
+extern "C" void foo ();		// { dg-message "previous declaration" }
 
 namespace {
   extern "C" int foo ();	// { dg-error "C.*linkage" }

Reply via email to