public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: Jason Merrill <jason@redhat.com>
Cc: gcc-patches@gcc.gnu.org
Subject: [RFC PATCH] c++: Diagnose [basic.scope.block]/2 violations even for block externs [PR52953]
Date: Thu, 31 Aug 2023 10:08:07 +0200	[thread overview]
Message-ID: <ZPBKZyVxYKhGMBVY@tucnak> (raw)

Hi!

C++17 had in [basic.block.scope]/2
"A parameter name shall not be redeclared in the outermost block of the function
definition nor in the outermost block of any handler associated with a
function-try-block."
and in [basic.block.scope]/4 similar rule for selection/iteration
statements.  My reading of that is that it applied even for block local
externs in all those spots, while they declare something at namespace scope,
the redeclaration happens in that outermost block etc. and introduces names
into that.
Those wordings seemed to have been moved somewhere else in C++20, but what's
worse, they were moved back and completely rewritten in
P1787R6: Declarations and where to find them
which has been applied as a DR (but admittedly, we don't claim yet to
implement that).
The current wording at https://eel.is/c++draft/basic.scope#block-2
and https://eel.is/c++draft/basic.scope#scope-2.10 seem to imply at least
to me that it doesn't apply to extern block local decls because their
target scope is the namespace scope and [basic.scope.block]/2 says
"and whose target scope is the block scope"...
Now, it is unclear if that is actually the intent or not.

There seems to be quite large implementation divergence on this as well.

Unpatched g++ e.g. on the redeclaration-5.C testcase diagnoses just
lines 55,58,67,70 (i.e. where the previous declaration is in for's
condition).

clang++ trunk diagnoses just lines 8 and 27, i.e. redeclaration in the
function body vs. parameter both in normal fn and lambda (but not e.g.
function-try-block and others, including ctors, but it diagnoses those
for non-extern decls).

ICC 19 diagnoses lines 8,32,38,41,45,52,55,58,61,64,67,70,76.

And MSCV trunk diagnoses 8,27,32,38,41,45,48,52,55,58,67,70,76,87,100,137
although the last 4 are just warnings.

g++ with the patch diagnoses
8,15,27,32,38,41,45,48,52,55,58,61,64,67,70,76,87,100,121,137
as the dg-error directives test.

So, I'm not really sure what to do.  Intuitively the patch seems right
because even block externs redeclare stuff and change meaning of the
identifiers and void foo () { int i; extern int i (int); } is rejected
by all compilers.

2023-08-31  Jakub Jelinek  <jakub@redhat.com>

	PR c++/52953
	* name-lookup.cc (check_local_shadow): Defer punting on
	DECL_EXTERNAL (decl) from the start of function to right before
	the -Wshadow* checks.

	* g++.dg/diagnostic/redeclaration-4.C: New test.
	* g++.dg/diagnostic/redeclaration-5.C: New test.

--- gcc/cp/name-lookup.cc.jj	2023-08-30 20:07:18.584830291 +0200
+++ gcc/cp/name-lookup.cc	2023-08-30 20:01:05.796967755 +0200
@@ -3096,10 +3096,6 @@ check_local_shadow (tree decl)
   if (TREE_CODE (decl) == PARM_DECL && !DECL_CONTEXT (decl))
     return;
 
-  /* External decls are something else.  */
-  if (DECL_EXTERNAL (decl))
-    return;
-
   tree old = NULL_TREE;
   cp_binding_level *old_scope = NULL;
   if (cxx_binding *binding = outer_binding (DECL_NAME (decl), NULL, true))
@@ -3219,6 +3215,9 @@ check_local_shadow (tree decl)
 	  return;
 	}
 
+      if (DECL_EXTERNAL (decl))
+	return;
+
       /* If '-Wshadow=compatible-local' is specified without other
 	 -Wshadow= flags, we will warn only when the type of the
 	 shadowing variable (DECL) can be converted to that of the
@@ -3274,6 +3273,9 @@ check_local_shadow (tree decl)
       return;
     }
 
+  if (DECL_EXTERNAL (decl))
+    return;
+
   if (!warn_shadow)
     return;
 
--- gcc/testsuite/g++.dg/diagnostic/redeclaration-4.C.jj	2023-08-30 20:01:37.013537549 +0200
+++ gcc/testsuite/g++.dg/diagnostic/redeclaration-4.C	2023-08-30 20:12:03.763900190 +0200
@@ -0,0 +1,167 @@
+// PR c++/52953
+// { dg-do compile }
+// { dg-options "-pedantic-errors -Wno-switch-unreachable" }
+
+void
+foo (int x)				// { dg-message "'int x' previously declared here" }
+{
+  extern int x;				// { dg-error "declaration of 'int x' shadows a parameter" }
+}
+
+void
+bar (int x)				// { dg-message "'int x' previously declared here" }
+try
+{
+  extern int x;				// { dg-error "declaration of 'int x' shadows a parameter" }
+}
+catch (...)
+{
+}
+
+volatile int v;
+
+void
+baz ()
+{
+#if __cplusplus >= 201103L
+  auto f = [] (int x) { extern int x; };// { dg-error "declaration of 'int x' shadows a parameter" "" { target c++11 } }
+					// { dg-message "'int x' previously declared here" "" { target c++11 } .-1 }
+#endif
+  if (int x = 1)			// { dg-message "'int x' previously declared here" }
+    {
+      extern int x;			// { dg-error "redeclaration of 'int x'" }
+    }
+  if (int x = 0)			// { dg-message "'int x' previously declared here" }
+    ;
+  else
+    {
+      extern int x;			// { dg-error "redeclaration of 'int x'" }
+    }
+  if (int x = 1)			// { dg-message "'int x' previously declared here" }
+    extern int x;			// { dg-error "redeclaration of 'int x'" }
+  if (int x = 0)			// { dg-message "'int x' previously declared here" }
+    ;
+  else
+    extern int x;			// { dg-error "redeclaration of 'int x'" }
+  switch (int x = 1)			// { dg-message "'int x' previously declared here" }
+    {
+      extern int x;			// { dg-error "redeclaration of 'int x'" }
+    default:;
+    }
+  switch (int x = 1)			// { dg-message "'int x' previously declared here" }
+    extern int x;			// { dg-error "redeclaration of 'int x'" }
+  while (int x = v)
+    {
+      extern int x;			// { dg-error "'int x' conflicts with a previous declaration" }
+    }
+  while (int x = v)
+    extern int x;			// { dg-error "'int x' conflicts with a previous declaration" }
+  for (int x = v; x; ++x)		// { dg-message "'int x' previously declared here" }
+    {
+      extern int x;			// { dg-error "redeclaration of 'int x'" }
+    }
+  for (int x = v; x; ++x)		// { dg-message "'int x' previously declared here" }
+    extern int x;			// { dg-error "redeclaration of 'int x'" }
+  for (; int x = v; )
+    {
+      extern int x;			// { dg-error "'int x' conflicts with a previous declaration" }
+    }
+  for (; int x = v; )
+    extern int x;			// { dg-error "'int x' conflicts with a previous declaration" }
+  try
+    {
+    }
+  catch (int x)				// { dg-message "'int x' previously declared here" }
+    {
+      extern int x;			// { dg-error "redeclaration of 'int x'" }
+    }
+}
+
+void
+corge (int x)				// { dg-message "'int x' previously declared here" }
+try
+{
+}
+catch (...)
+{
+  extern int x;				// { dg-error "redeclaration of 'int x'" }
+}
+
+void
+fred (int x)				// { dg-message "'int x' previously declared here" }
+try
+{
+}
+catch (int)
+{
+}
+catch (long)
+{
+  extern int x;				// { dg-error "redeclaration of 'int x'" }
+}
+
+void
+garply (int x)
+{
+  try
+    {
+      extern int x;
+    }
+  catch (...)
+    {
+      extern int x;
+    }
+}
+
+struct S
+{
+  S (int x)				// { dg-message "'int x' previously declared here" }
+  try : s (x)
+  {
+    extern int x;				// { dg-error "declaration of 'int x' shadows a parameter" }
+  }
+  catch (...)
+  {
+  }
+  int s;
+};
+
+struct T
+{
+  T (int x)				// { dg-message "'int x' previously declared here" }
+  try : t (x)
+  {
+  }
+  catch (...)
+  {
+    extern int x;				// { dg-error "redeclaration of 'int x'" }
+  }
+  int t;
+};
+
+struct U
+{
+  U (int x) : u (x)
+  {
+    try
+    {
+      extern int x;
+    }
+    catch (...)
+    {
+      extern int x;
+    }
+  }
+  int u;
+};
+
+struct V
+{
+  V (int x) : v (x)
+  {
+    {
+      extern int x;
+    }
+  }
+  int v;
+};
--- gcc/testsuite/g++.dg/diagnostic/redeclaration-5.C.jj	2023-08-30 20:03:56.662613023 +0200
+++ gcc/testsuite/g++.dg/diagnostic/redeclaration-5.C	2023-08-30 20:16:00.749641990 +0200
@@ -0,0 +1,167 @@
+// PR c++/52953
+// { dg-do compile }
+// { dg-options "-pedantic-errors -Wno-switch-unreachable" }
+
+void
+foo (int x)				// { dg-message "'int x' previously declared here" }
+{
+  extern int x (int);			// { dg-error "declaration of 'int x\\\(int\\\)' shadows a parameter" }
+}
+
+void
+bar (int x)				// { dg-message "'int x' previously declared here" }
+try
+{
+  extern int x (int);			// { dg-error "declaration of 'int x\\\(int\\\)' shadows a parameter" }
+}
+catch (...)
+{
+}
+
+volatile int v;
+
+void
+baz ()
+{
+#if __cplusplus >= 201103L
+  auto f = [] (int x) { extern int x (int); };// { dg-error "declaration of 'int x\\\(int\\\)' shadows a parameter" "" { target c++11 } }
+					// { dg-message "'int x' previously declared here" "" { target c++11 } .-1 }
+#endif
+  if (int x = 1)			// { dg-message "'int x' previously declared here" }
+    {
+      extern int x (int);		// { dg-error "redeclaration of 'int x\\\(int\\\)'" }
+    }
+  if (int x = 0)			// { dg-message "'int x' previously declared here" }
+    ;
+  else
+    {
+      extern int x (int);		// { dg-error "redeclaration of 'int x\\\(int\\\)'" }
+    }
+  if (int x = 1)			// { dg-message "'int x' previously declared here" }
+    extern int x (int);			// { dg-error "redeclaration of 'int x\\\(int\\\)'" }
+  if (int x = 0)			// { dg-message "'int x' previously declared here" }
+    ;
+  else
+    extern int x (int);			// { dg-error "redeclaration of 'int x\\\(int\\\)'" }
+  switch (int x = 1)			// { dg-message "'int x' previously declared here" }
+    {
+      extern int x (int);		// { dg-error "redeclaration of 'int x\\\(int\\\)'" }
+    default:;
+    }
+  switch (int x = 1)			// { dg-message "'int x' previously declared here" }
+    extern int x (int);			// { dg-error "redeclaration of 'int x\\\(int\\\)'" }
+  while (int x = v)
+    {
+      extern int x (int);		// { dg-error "'int x\\\(int\\\)' redeclared as different kind of entity" }
+    }
+  while (int x = v)
+    extern int x (int);			// { dg-error "'int x\\\(int\\\)' redeclared as different kind of entity" }
+  for (int x = v; x; ++x)		// { dg-message "'int x' previously declared here" }
+    {
+      extern int x (int);		// { dg-error "redeclaration of 'int x\\\(int\\\)'" }
+    }
+  for (int x = v; x; ++x)		// { dg-message "'int x' previously declared here" }
+    extern int x (int);			// { dg-error "redeclaration of 'int x\\\(int\\\)'" }
+  for (; int x = v; )
+    {
+      extern int x (int);		// { dg-error "'int x\\\(int\\\)' redeclared as different kind of entity" }
+    }
+  for (; int x = v; )
+    extern int x (int);			// { dg-error "'int x\\\(int\\\)' redeclared as different kind of entity" }
+  try
+    {
+    }
+  catch (int x)				// { dg-message "'int x' previously declared here" }
+    {
+      extern int x (int);		// { dg-error "redeclaration of 'int x\\\(int\\\)'" }
+    }
+}
+
+void
+corge (int x)				// { dg-message "'int x' previously declared here" }
+try
+{
+}
+catch (...)
+{
+  extern int x (int);			// { dg-error "redeclaration of 'int x\\\(int\\\)'" }
+}
+
+void
+fred (int x)				// { dg-message "'int x' previously declared here" }
+try
+{
+}
+catch (int)
+{
+}
+catch (long)
+{
+  extern int x (int);			// { dg-error "redeclaration of 'int x\\\(int\\\)'" }
+}
+
+void
+garply (int x)
+{
+  try
+    {
+      extern int x (int);
+    }
+  catch (...)
+    {
+      extern int x (int);
+    }
+}
+
+struct S
+{
+  S (int x)				// { dg-message "'int x' previously declared here" }
+  try : s (x)
+  {
+    extern int x (int);			// { dg-error "declaration of 'int x\\\(int\\\)' shadows a parameter" }
+  }
+  catch (...)
+  {
+  }
+  int s;
+};
+
+struct T
+{
+  T (int x)				// { dg-message "'int x' previously declared here" }
+  try : t (x)
+  {
+  }
+  catch (...)
+  {
+    extern int x (int);			// { dg-error "redeclaration of 'int x\\\(int\\\)'" }
+  }
+  int t;
+};
+
+struct U
+{
+  U (int x) : u (x)
+  {
+    try
+    {
+      extern int x (int);
+    }
+    catch (...)
+    {
+      extern int x (int);
+    }
+  }
+  int u;
+};
+
+struct V
+{
+  V (int x) : v (x)
+  {
+    {
+      extern int x (int);
+    }
+  }
+  int v;
+};

	Jakub


             reply	other threads:[~2023-08-31  8:08 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-31  8:08 Jakub Jelinek [this message]
2023-08-31 21:46 ` Jason Merrill
2023-09-01 13:34   ` [PATCH] c++, v2: " Jakub Jelinek
2023-09-05 14:00     ` Jason Merrill

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZPBKZyVxYKhGMBVY@tucnak \
    --to=jakub@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jason@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).