public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jason Merrill <jason@redhat.com>
To: Jakub Jelinek <jakub@redhat.com>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [RFC PATCH] c++: Diagnose [basic.scope.block]/2 violations even for block externs [PR52953]
Date: Thu, 31 Aug 2023 17:46:28 -0400	[thread overview]
Message-ID: <495f5797-fbb9-e9f6-faa4-af994782edd5@redhat.com> (raw)
In-Reply-To: <ZPBKZyVxYKhGMBVY@tucnak>

On 8/31/23 04:08, Jakub Jelinek wrote:
> 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.

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 }; }

I've suggested this to Core.

> 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.

I think this direction makes sense, though we might pedwarn on these 
rather than error to reduce possible breakage.

> 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.

Don't we want to consider externs for the -Wshadow* checks as well?

Jason


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

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-31  8:08 Jakub Jelinek
2023-08-31 21:46 ` Jason Merrill [this message]
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=495f5797-fbb9-e9f6-faa4-af994782edd5@redhat.com \
    --to=jason@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jakub@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).