Bug 94553 - Revise [basic.scope.declarative]/4.2
Summary: Revise [basic.scope.declarative]/4.2
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 10.0
: P3 normal
Target Milestone: ---
Assignee: Marek Polacek
URL:
Keywords: accepts-invalid
Depends on:
Blocks: c++-core-issues
  Show dependency treegraph
 
Reported: 2020-04-10 18:11 UTC by Marek Polacek
Modified: 2020-06-29 15:11 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2020-05-13 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Marek Polacek 2020-04-10 18:11:41 UTC
<http://eel.is/c++draft/basic.scope#declarative-4.2> has a note:
A structured binding ([dcl.struct.bind]), namespace name ([basic.namespace]), or class template name ([temp.pre]) must be unique in its declarative region.
but we don't fully implement that.  Consequently, the following is accepts-valid.  (The structured binding part was added in CWG 2289.)

void
f ()
{
  int arr[1] = { 1 };
  struct A { };
  auto [A] = arr; // error
  auto [B] = arr;
  struct B { }; // error
}

struct C { };
template<typename> int C; // error
template<typename> int D;
struct D { }; // error
Comment 1 Marek Polacek 2020-04-10 18:12:22 UTC
(In reply to Marek Polacek from comment #0)
> Consequently, the following is accepts-valid.

s/valid/invalid/, of course.
Comment 2 Marek Polacek 2020-04-10 23:18:55 UTC
To fix CWG 2289, we need this:

--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -1705,6 +1705,9 @@ duplicate_decls (tree newdecl, tree olddecl, bool newdecl_is_friend)
      inform (olddecl_loc, "previous declaration %q#D", olddecl);
      return error_mark_node;
    }
+      else if ((VAR_P (olddecl) && DECL_DECOMPOSITION_P (olddecl))
+          || (VAR_P (newdecl) && DECL_DECOMPOSITION_P (newdecl)))
+   /* A structured binding must be unique in its declarative region.  */;
       else if (DECL_IMPLICIT_TYPEDEF_P (olddecl)
           || DECL_IMPLICIT_TYPEDEF_P (newdecl))
    /* One is an implicit typedef, that's ok.  */


but that's not enough to detect the 'A' case: cp_parser_decomposition_declaration
uses

13968       tree decl2 = start_decl (declarator, &decl_specs, SD_INITIALIZED,
13969                                NULL_TREE, NULL_TREE, &elt_pushed_scope);

to create the 'A' VAR_DECL but in this start_decl's grokdeclarator we don't do fit_decomposition_lang_decl because the declarator kind is not cdk_decomp, so then when start_decl calls maybe_push_decl, the decl 'A' isn't DECL_DECOMPOSITION_P and we don't detect this case.  So we need a way to signal to start_decl that it should fit_decomposition_lang_decl.
Comment 3 Marek Polacek 2020-05-13 16:23:45 UTC
A patch for DR 2289 posted: https://gcc.gnu.org/pipermail/gcc-patches/2020-May/545692.html
Comment 4 CVS Commits 2020-05-20 17:39:07 UTC
The master branch has been updated by Marek Polacek <mpolacek@gcc.gnu.org>:

https://gcc.gnu.org/g:2a8565fa1182ed326721a50c700f9f5275355d40

commit r11-529-g2a8565fa1182ed326721a50c700f9f5275355d40
Author: Marek Polacek <polacek@redhat.com>
Date:   Tue May 19 23:53:28 2020 -0400

    c++: Implement DR 2289, Uniqueness of structured binding names [PR94553]
    
    DR 2289 clarified that since structured bindings have no C compatibility
    implications, they should be unique in their declarative region, see
    [basic.scope.declarative]/4.2.
    
    The duplicate_decls hunk is the gist of the patch, but that alone would
    not be enough to detect the 'A' case: cp_parser_decomposition_declaration
    uses
    
    13968       tree decl2 = start_decl (declarator, &decl_specs, SD_INITIALIZED,
    13969                                NULL_TREE, NULL_TREE, &elt_pushed_scope);
    
    to create the 'A' VAR_DECL but in this start_decl's grokdeclarator we
    don't do fit_decomposition_lang_decl because the declarator kind is not
    cdk_decomp, so then when start_decl calls maybe_push_decl, the decl 'A'
    isn't DECL_DECOMPOSITION_P and we don't detect this case.  So I needed a
    way to signal to start_decl that it should fit_decomposition_lang_decl.
    In this patch, I'm adding SD_DECOMPOSITION flag to say that the variable
    is initialized and it should also be marked as DECL_DECOMPOSITION_P.
    
            DR 2289
            PR c++/94553
            * cp-tree.h (SD_DECOMPOSITION): New flag.
            * decl.c (duplicate_decls): Make sure a structured binding is unique
            in its declarative region.
            (start_decl): If INITIALIZED is SD_DECOMPOSITION, call
            fit_decomposition_lang_decl.
            (grokdeclarator): Compare INITIALIZED directly to SD_* flags.
            * parser.c (cp_parser_decomposition_declaration): Pass SD_DECOMPOSITION
            to start_decl.
    
            * g++.dg/cpp1z/decomp52.C: New test.
Comment 5 Marek Polacek 2020-05-20 17:41:15 UTC
The structured binding part is now fixed, but the variable template part isn't yet, so not closing.
Comment 6 Marek Polacek 2020-06-26 16:07:37 UTC
I think it would make sense to detect this too:

struct E { };
template<typename> concept E = false;
template<typename> concept F = false;
struct F { };
Comment 7 CVS Commits 2020-06-29 15:02:31 UTC
The master branch has been updated by Marek Polacek <mpolacek@gcc.gnu.org>:

https://gcc.gnu.org/g:54980635c537f3130481da2d8b1109c775db8bb0

commit r11-1714-g54980635c537f3130481da2d8b1109c775db8bb0
Author: Marek Polacek <polacek@redhat.com>
Date:   Fri Jun 26 12:40:59 2020 -0400

    c++: Check uniqueness of concepts/variable templates [PR94553]
    
    This patch wraps up PR94553.  Variable template names have no C
    compatibility implications so they should be unique in their
    declarative region.  It occurred to me that this applies to concepts
    as well.  This is not specified in [basic.scope.declarative]/4.2
    but that seems like a bug in the standard.
    
    I couldn't use variable_template_p because that uses PRIMARY_TEMPLATE_P
    which uses DECL_PRIMARY_TEMPLATE and that might not have been set up yet
    (push_template_decl hasn't yet been called).  PRIMARY_TEMPLATE_P is
    important to distinguish between a variable template and a variable in a
    function template.  But I think we don't have to worry about that in
    duplicate_decls: a template declaration cannot appear at block scope,
    and additional checks in duplicate_decls suggest that it won't ever
    see a TEMPLATE_DECL for a variable in a function template.  So
    checking that the DECL_TEMPLATE_RESULT is a VAR_DECL seems to be fine.
    I could have added a default argument to variable_template_p too to
    avoid checking PRIMARY_TEMPLATE_P but it didn't seem worth the effort.
    
    gcc/cp/ChangeLog:
    
            PR c++/94553
            * decl.c (duplicate_decls): Make sure a concept or a variable
            template is unique in its declarative region.
    
    gcc/testsuite/ChangeLog:
    
            PR c++/94553
            * g++.dg/cpp1y/pr68578.C: Adjust dg-error.
            * g++.dg/cpp1y/var-templ66.C: New test.
            * g++.dg/cpp2a/concepts-redecl1.C: New test.
Comment 8 Marek Polacek 2020-06-29 15:11:29 UTC
Fixed.