c++: Diagnose [basic.scope.block]/2 violations even for block externs [PR52953]

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.

Jason said:
> Yes, I suspect that should be
>
> If a declaration that is not a name-independent declaration and <del>whose
> target scope is</del><ins>that binds a name in</ins> the block scope S of a
>
> which seems to also be needed to prohibit the already-diagnosed
>
> void f(int i) { union { int i; }; }
> void g(int i) { enum { i }; }

The following patch diagnoses DECL_EXTERNAL in check_local_shadow like
!DECL_EXTERNAL, except that
1) it uses pedwarn instead of errors for those cases
2) it doesn't diagnose shadowing of namespace scope identifiers by block
   local externs, as they could be not actually shadowing but just redeclaring
   the same objects

2023-09-05  Jakub Jelinek  <jakub@redhat.com>

	PR c++/52953
	* name-lookup.cc (check_local_shadow): Don't punt early for
	DECL_EXTERNAL decls, instead just disable the shadowing of namespace
	decls check for those and emit a pedwarn rather than error_at or
	permerror for those.  Formatting fix.

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

diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
index a2bcd4f2633ddab4a697789b73a2878e9f005aaa..e776bb868fd8a3ff60df342daca3d10aaa24e770 100644 (file)
--- a/gcc/cp/name-lookup.cc
+++ b/gcc/cp/name-lookup.cc
@@ -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))
@@ -3130,11 +3126,9 @@ check_local_shadow (tree decl)
              && DECL_CONTEXT (old) == lambda_function (current_lambda_expr ())
              && TREE_CODE (old) == PARM_DECL
              && DECL_NAME (decl) != this_identifier)
-           {
-             error_at (DECL_SOURCE_LOCATION (old),
-                       "lambda parameter %qD "
-                       "previously declared as a capture", old);
-           }
+           error_at (DECL_SOURCE_LOCATION (old),
+                     "lambda parameter %qD "
+                     "previously declared as a capture", old);
          return;
        }
       /* Don't complain if it's from an enclosing function.  */
@@ -3160,10 +3154,17 @@ check_local_shadow (tree decl)
          if (b->kind == sk_function_parms)
            {
              auto_diagnostic_group d;
-             error_at (DECL_SOURCE_LOCATION (decl),
-                       "declaration of %q#D shadows a parameter", decl);
-             inform (DECL_SOURCE_LOCATION (old),
-                     "%q#D previously declared here", old);
+             bool emit = true;
+             if (DECL_EXTERNAL (decl))
+               emit = pedwarn (DECL_SOURCE_LOCATION (decl), OPT_Wpedantic,
+                               "declaration of %q#D shadows a parameter",
+                               decl);
+             else
+               error_at (DECL_SOURCE_LOCATION (decl),
+                         "declaration of %q#D shadows a parameter", decl);
+             if (emit)
+               inform (DECL_SOURCE_LOCATION (old),
+                       "%q#D previously declared here", old);
              return;
            }
        }
@@ -3189,10 +3190,16 @@ check_local_shadow (tree decl)
               && (old_scope->kind == sk_cond || old_scope->kind == sk_for))
        {
          auto_diagnostic_group d;
-         error_at (DECL_SOURCE_LOCATION (decl),
-                   "redeclaration of %q#D", decl);
-         inform (DECL_SOURCE_LOCATION (old),
-                 "%q#D previously declared here", old);
+         bool emit = true;
+         if (DECL_EXTERNAL (decl))
+           emit = pedwarn (DECL_SOURCE_LOCATION (decl), OPT_Wpedantic,
+                           "redeclaration of %q#D", decl);
+         else
+           error_at (DECL_SOURCE_LOCATION (decl),
+                     "redeclaration of %q#D", decl);
+         if (emit)
+           inform (DECL_SOURCE_LOCATION (old),
+                   "%q#D previously declared here", old);
          return;
        }
       /* C++11:
@@ -3206,8 +3213,14 @@ check_local_shadow (tree decl)
               && old_scope->kind == sk_catch)
        {
          auto_diagnostic_group d;
-         if (permerror (DECL_SOURCE_LOCATION (decl),
-                        "redeclaration of %q#D", decl))
+         bool emit;
+         if (DECL_EXTERNAL (decl))
+           emit = pedwarn (DECL_SOURCE_LOCATION (decl), OPT_Wpedantic,
+                           "redeclaration of %q#D", decl);
+         else
+           emit = permerror (DECL_SOURCE_LOCATION (decl),
+                             "redeclaration of %q#D", decl);
+         if (emit)
            inform (DECL_SOURCE_LOCATION (old),
                    "%q#D previously declared here", old);
          return;
@@ -3311,6 +3324,7 @@ check_local_shadow (tree decl)
          || (TREE_CODE (old) == TYPE_DECL
              && (!DECL_ARTIFICIAL (old)
                  || TREE_CODE (decl) == TYPE_DECL)))
+      && !DECL_EXTERNAL (decl)
       && !instantiating_current_function_p ()
       && !warning_suppressed_p (decl, OPT_Wshadow))
     /* XXX shadow warnings in outer-more namespaces */

diff --git a/gcc/testsuite/g++.dg/diagnostic/redeclaration-4.C b/gcc/testsuite/g++.dg/diagnostic/redeclaration-4.C
new file mode 100644 (file)
index 0000000..70b65e0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/diagnostic/redeclaration-4.C
@@ -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 "declaration of 'int x' shadows a parameter" }
+}
+
+void
+fred (int x)                           // { dg-message "'int x' previously declared here" }
+try
+{
+}
+catch (int)
+{
+}
+catch (long)
+{
+  extern int x;                                // { dg-error "declaration of 'int x' shadows a parameter" }
+}
+
+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 "declaration of 'int x' shadows a parameter" }
+  }
+  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;
+};

diff --git a/gcc/testsuite/g++.dg/diagnostic/redeclaration-5.C b/gcc/testsuite/g++.dg/diagnostic/redeclaration-5.C
new file mode 100644 (file)
index 0000000..8c60d53
--- /dev/null
+++ b/gcc/testsuite/g++.dg/diagnostic/redeclaration-5.C
@@ -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 "declaration of 'int x\\\(int\\\)' shadows a parameter" }
+}
+
+void
+fred (int x)                           // { dg-message "'int x' previously declared here" }
+try
+{
+}
+catch (int)
+{
+}
+catch (long)
+{
+  extern int x (int);                  // { dg-error "declaration of 'int x\\\(int\\\)' shadows a parameter" }
+}
+
+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 "declaration of 'int x\\\(int\\\)' shadows a parameter" }
+  }
+  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;
+};

diff --git a/gcc/testsuite/g++.dg/warn/Wshadow-19.C b/gcc/testsuite/g++.dg/warn/Wshadow-19.C
new file mode 100644 (file)
index 0000000..030aefd
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wshadow-19.C
@@ -0,0 +1,27 @@
+// { dg-do compile }
+// { dg-options "-Wshadow" }
+
+void
+foo (int x)
+{
+  int y = 1;
+  {
+    extern int x;                              // { dg-warning "declaration of 'int x' shadows a parameter" }
+    extern int y;                              // { dg-warning "declaration of 'y' shadows a previous local" }
+  }
+#if __cplusplus >= 201102L
+  auto fn = [x] () { extern int x; return 0; };        // { dg-warning "declaration of 'x' shadows a lambda capture" "" { target c++11 } }
+#endif
+}
+
+int z;
+
+struct S
+{
+  int x;
+  void foo ()
+  {
+    extern int x;                              // { dg-warning "declaration of 'x' shadows a member of 'S'" }
+    extern int z;
+  }
+};