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: [PATCH] c++, v2: Diagnose [basic.scope.block]/2 violations even in compound-stmt of function-try-block [PR52953]
Date: Fri, 1 Sep 2023 15:24:54 +0200	[thread overview]
Message-ID: <ZPHmJl9P/CX5JViM@tucnak> (raw)
In-Reply-To: <a10f3b86-28be-6b42-f0d1-128fb8b18707@redhat.com>

On Thu, Aug 31, 2023 at 03:52:22PM -0400, Jason Merrill wrote:
> On 8/31/23 03:20, Jakub Jelinek wrote:
> > As the following testcase shows, while check_local_shadow diagnoses most of
> > the [basic.scope.block]/2 violations, it doesn't diagnose when parameter's
> > name is redeclared inside of the compound-stmt of a function-try-block.
> > 
> > There is in that case an extra scope (sk_try with parent artificial
> > sk_block with for FUNCTION_NEEDS_BODY_BLOCK another sk_block and only then
> > sk_function_param).
> > 
> > The following patch fixes that.
> > 
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> > 
> > 2023-08-31  Jakub Jelinek  <jakub@redhat.com>
> > 
> > 	PR c++/52953
> > 	* cp-tree.h (struct language_function): Add x_in_function_try_block
> > 	member.
> 
> How about adding a flag to cp_binding_level instead?  Maybe to mark the
> artificial sk_block level as such, which we could use for both this case and
> the FUNCTION_NEEDS_BODY_BLOCK cases.

So like this?

It actually changes behaviour on the
void foo (int x) try {} catch (int x) {} case, where previously
this triggered the
               || (TREE_CODE (old) == PARM_DECL
                   && (current_binding_level->kind == sk_catch
                       || current_binding_level->level_chain->kind == sk_catch)
                   && in_function_try_handler))
        {
          auto_diagnostic_group d;
          if (permerror (DECL_SOURCE_LOCATION (decl),
                         "redeclaration of %q#D", decl))
            inform (DECL_SOURCE_LOCATION (old),
                    "%q#D previously declared here", old);
diagnostics (note, just the current_binding_level->kind == sk_catch
case), while now it triggers already the earlier
          if (b->kind == sk_function_parms)
            {
              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);
error.  If you think it is important to differentiate that,
I guess I could guard the while (b->artificial) loop with say
+	  if (!in_function_try_handler
+	      || current_binding_level->kind != sk_catch)
	    while (b->artificial)
	      b = b->level_chain;
and adjust the 2 testcases.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk
or with modification?

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

	PR c++/52953
	* name-lookup.h (struct cp_binding_level): Add artificial bit-field.
	Formatting fixes.
	* name-lookup.cc (check_local_shadow): Skip artificial bindings when
	checking if parameter scope is parent scope.  Don't special case
	FUNCTION_NEEDS_BODY_BLOCK.
	* decl.cc (begin_function_body): Set
	current_binding_level->artificial.
	* semantics.cc (begin_function_try_block): Likewise.

	* g++.dg/diagnostic/redeclaration-3.C: New test.
	* g++.dg/parse/pr31952-3.C: Expect different diagnostic wording.

--- gcc/cp/name-lookup.h.jj	2023-08-21 11:57:33.105460770 +0200
+++ gcc/cp/name-lookup.h	2023-09-01 10:15:20.137943395 +0200
@@ -292,11 +292,11 @@ struct GTY(()) cp_binding_level {
       only valid if KIND == SK_TEMPLATE_PARMS.  */
   BOOL_BITFIELD explicit_spec_p : 1;
 
-  /* true means make a BLOCK for this level regardless of all else.  */
+  /* True means make a BLOCK for this level regardless of all else.  */
   unsigned keep : 1;
 
   /* Nonzero if this level can safely have additional
-      cleanup-needing variables added to it.  */
+     cleanup-needing variables added to it.  */
   unsigned more_cleanups_ok : 1;
   unsigned have_cleanups : 1;
 
@@ -308,9 +308,13 @@ struct GTY(()) cp_binding_level {
   unsigned defining_class_p : 1;
 
   /* True for SK_FUNCTION_PARMS of a requires-expression.  */
-  unsigned requires_expression: 1;
+  unsigned requires_expression : 1;
 
-  /* 22 bits left to fill a 32-bit word.  */
+  /* True for artificial blocks which should be ignored when finding
+     parent scope.  */
+  unsigned artificial : 1;
+
+  /* 21 bits left to fill a 32-bit word.  */
 };
 
 /* The binding level currently in effect.  */
--- gcc/cp/name-lookup.cc.jj	2023-08-31 14:31:06.055762306 +0200
+++ gcc/cp/name-lookup.cc	2023-09-01 10:21:03.658118594 +0200
@@ -3146,8 +3146,10 @@ check_local_shadow (tree decl)
 	     them there.  */
 	  cp_binding_level *b = current_binding_level->level_chain;
 
-	  if (FUNCTION_NEEDS_BODY_BLOCK (current_function_decl))
-	    /* Skip the ctor/dtor cleanup level.  */
+	  /* Skip artificially added scopes which aren't present
+	     in the C++ standard, e.g. for function-try-block or
+	     ctor/dtor cleanups.  */
+	  while (b->artificial)
 	    b = b->level_chain;
 
 	  /* [basic.scope.param] A parameter name shall not be redeclared
--- gcc/cp/decl.cc.jj	2023-08-31 20:48:21.127722180 +0200
+++ gcc/cp/decl.cc	2023-09-01 10:18:40.125134543 +0200
@@ -18002,6 +18002,7 @@ begin_function_body (void)
     keep_next_level (true);
 
   tree stmt = begin_compound_stmt (BCS_FN_BODY);
+  current_binding_level->artificial = 1;
 
   if (processing_template_decl)
     /* Do nothing now.  */;
--- gcc/cp/semantics.cc.jj	2023-08-31 14:31:06.102761661 +0200
+++ gcc/cp/semantics.cc	2023-09-01 10:17:49.044851974 +0200
@@ -1624,6 +1624,7 @@ begin_function_try_block (tree *compound
   /* This outer scope does not exist in the C++ standard, but we need
      a place to put __FUNCTION__ and similar variables.  */
   *compound_stmt = begin_compound_stmt (0);
+  current_binding_level->artificial = 1;
   r = begin_try_block ();
   FN_TRY_BLOCK_P (r) = 1;
   return r;
--- gcc/testsuite/g++.dg/diagnostic/redeclaration-3.C.jj	2023-09-01 09:40:12.061707730 +0200
+++ gcc/testsuite/g++.dg/diagnostic/redeclaration-3.C	2023-09-01 10:32:36.315429575 +0200
@@ -0,0 +1,204 @@
+// PR c++/52953
+// { dg-do compile }
+// { dg-options "-pedantic-errors -Wno-switch-unreachable" }
+
+void
+foo (int x)				// { dg-message "'int x' previously declared here" }
+{
+  int x;				// { dg-error "declaration of 'int x' shadows a parameter" }
+}
+
+void
+bar (int x)				// { dg-message "'int x' previously declared here" }
+try
+{
+  int x;				// { dg-error "declaration of 'int x' shadows a parameter" }
+}
+catch (...)
+{
+}
+
+volatile int v;
+
+void
+baz ()
+{
+#if __cplusplus >= 201103L
+  auto f = [] (int x) { 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" }
+    {
+      int x;				// { dg-error "redeclaration of 'int x'" }
+    }
+  if (int x = 0)			// { dg-message "'int x' previously declared here" }
+    ;
+  else
+    {
+      int x;				// { dg-error "redeclaration of 'int x'" }
+    }
+  if (int x = 1)			// { dg-message "'int x' previously declared here" }
+    int x;				// { dg-error "redeclaration of 'int x'" }
+  if (int x = 0)			// { dg-message "'int x' previously declared here" }
+    ;
+  else
+    int x;				// { dg-error "redeclaration of 'int x'" }
+  switch (int x = 1)			// { dg-message "'int x' previously declared here" }
+    {
+      int x;				// { dg-error "redeclaration of 'int x'" }
+    default:;
+    }
+  switch (int x = 1)			// { dg-message "'int x' previously declared here" }
+    int x;				// { dg-error "redeclaration of 'int x'" }
+  while (int x = v)			// { dg-message "'int x' previously declared here" }
+    {
+      int x;				// { dg-error "redeclaration of 'int x'" }
+    }
+  while (int x = v)			// { dg-message "'int x' previously declared here" }
+    int x;				// { dg-error "redeclaration of 'int x'" }
+  for (int x = v; x; ++x)		// { dg-message "'int x' previously declared here" }
+    {
+      int x;				// { dg-error "redeclaration of 'int x'" }
+    }
+  for (int x = v; x; ++x)		// { dg-message "'int x' previously declared here" }
+    int x;				// { dg-error "redeclaration of 'int x'" }
+  for (; int x = v; )			// { dg-message "'int x' previously declared here" }
+    {
+      int x;				// { dg-error "redeclaration of 'int x'" }
+    }
+  for (; int x = v; )			// { dg-message "'int x' previously declared here" }
+    int x;				// { dg-error "redeclaration of 'int x'" }
+  try
+    {
+    }
+  catch (int x)				// { dg-message "'int x' previously declared here" }
+    {
+      int x;				// { dg-error "redeclaration of 'int x'" }
+    }
+  if (int x = 1)
+    if (int x = 1)
+      ;
+  if (int x = 0)
+    ;
+  else
+    if (int x = 1)
+      ;
+  if (int x = 1)
+    switch (int x = 1)
+      ;
+  if (int x = 0)
+    while (int x = v)
+      ;
+  if (int x = 0)
+    for (int x = v; x; ++x)
+      ;
+  switch (int x = 1)
+    switch (int x = 1)
+      {
+      case 1:;
+      }
+  while (int x = 0)
+    if (int x = 1)
+      ;
+  for (int x = v; x; ++x)
+    for (int x = v; x; ++x)
+      ;
+}
+
+void
+qux (int x)				// { dg-message "'int x' previously declared here" }
+try
+{
+}
+catch (int x)				// { dg-error "declaration of 'int x' shadows a parameter" }
+{
+}
+
+void
+corge (int x)				// { dg-message "'int x' previously declared here" }
+try
+{
+}
+catch (...)
+{
+  int x;				// { dg-error "redeclaration of 'int x'" }
+}
+
+void
+fred (int x)				// { dg-message "'int x' previously declared here" }
+try
+{
+}
+catch (int)
+{
+}
+catch (long)
+{
+  int x;				// { dg-error "redeclaration of 'int x'" }
+}
+
+void
+garply (int x)
+{
+  try
+    {
+      int x;
+    }
+  catch (...)
+    {
+      int x;
+    }
+}
+
+struct S
+{
+  S (int x)				// { dg-message "'int x' previously declared here" }
+  try : s (x)
+  {
+    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 (...)
+  {
+    int x;				// { dg-error "redeclaration of 'int x'" }
+  }
+  int t;
+};
+
+struct U
+{
+  U (int x) : u (x)
+  {
+    try
+    {
+      int x;
+    }
+    catch (...)
+    {
+      int x;
+    }
+  }
+  int u;
+};
+
+struct V
+{
+  V (int x) : v (x)
+  {
+    {
+      int x;
+    }
+  }
+  int v;
+};
--- gcc/testsuite/g++.dg/parse/pr31952-3.C.jj	2020-01-14 20:02:46.918607812 +0100
+++ gcc/testsuite/g++.dg/parse/pr31952-3.C	2023-09-01 14:41:12.306964831 +0200
@@ -6,7 +6,7 @@ try
 {
   return 0;
 }
-catch (int bar)  // { dg-error "redeclaration" }
+catch (int bar)  // { dg-error "shadows a parameter" }
 {
   return 1;
 }


	Jakub


  reply	other threads:[~2023-09-01 13:24 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-31  7:20 [PATCH] c++: " Jakub Jelinek
2023-08-31 19:52 ` Jason Merrill
2023-09-01 13:24   ` Jakub Jelinek [this message]
2023-09-01 14:34     ` [PATCH] c++, v3: " Jakub Jelinek
2023-09-05 14:15     ` [PATCH] c++, v2: " 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=ZPHmJl9P/CX5JViM@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).