Re: [PATCH] c++: Implement DR 2289, Uniqueness of structured binding names [PR94553]

2020-05-19 Thread Jason Merrill via Gcc-patches

On 5/18/20 5:07 PM, Marek Polacek wrote:

On Mon, May 18, 2020 at 04:50:44PM -0400, Jason Merrill via Gcc-patches wrote:

On 5/13/20 12:22 PM, Marek Polacek wrote:

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, _specs, SD_INITIALIZED,
13969NULL_TREE, NULL_TREE, _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


Why isn't it?  I see

   cp_declarator *declarator = make_declarator (cdk_decomp);

a few lines up.


Right, and we use that for the underlying decomp base VAR_DECL.  But each of
the named variables used in a structured binding are created in the
FOR_EACH_VEC_ELT loop, and that creates a cdk_id declarator for them:

13963   if (i == 0)
13964 declarator = make_id_declarator (NULL_TREE, e.get_value (),
13965  sfk_none, e.get_location ());


Ah, yes.

You name the new parameter FORCE_LANG_DECL_P and describe it as meaning 
"create a lang decl node for the new decl", but the actual meaning seems 
to be "mark the new decl as DECL_DECOMPOSITION_P", which seems too 
specific for adding a new parameter to a frequently used function.


How about adding a new SD_DECOMPOSITION value for the initialized 
parameter, or an attribute?


Jason



Re: [PATCH] c++: Implement DR 2289, Uniqueness of structured binding names [PR94553]

2020-05-18 Thread Marek Polacek via Gcc-patches
On Mon, May 18, 2020 at 04:50:44PM -0400, Jason Merrill via Gcc-patches wrote:
> On 5/13/20 12:22 PM, Marek Polacek wrote:
> > 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, _specs, 
> > SD_INITIALIZED,
> > 13969NULL_TREE, NULL_TREE, 
> > _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
> 
> Why isn't it?  I see
> 
>   cp_declarator *declarator = make_declarator (cdk_decomp);
> 
> a few lines up.

Right, and we use that for the underlying decomp base VAR_DECL.  But each of
the named variables used in a structured binding are created in the
FOR_EACH_VEC_ELT loop, and that creates a cdk_id declarator for them:

13963   if (i == 0)
13964 declarator = make_id_declarator (NULL_TREE, e.get_value (),
13965  sfk_none, e.get_location ());


Marek



Re: [PATCH] c++: Implement DR 2289, Uniqueness of structured binding names [PR94553]

2020-05-18 Thread Jason Merrill via Gcc-patches

On 5/13/20 12:22 PM, Marek Polacek wrote:

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, _specs, SD_INITIALIZED,
13969NULL_TREE, NULL_TREE, _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


Why isn't it?  I see

  cp_declarator *declarator = make_declarator (cdk_decomp);

a few lines up.

Jason



[PATCH] c++: Implement DR 2289, Uniqueness of structured binding names [PR94553]

2020-05-13 Thread Marek Polacek via Gcc-patches
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, _specs, SD_INITIALIZED,
13969NULL_TREE, NULL_TREE, _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.
A simple flag seems the easiest solution.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

DR 2289
PR c++/94553
* cp-tree.h (groktypename): Fix formatting.
(start_decl): Add new bool parameter with a default argument.
* decl.c (duplicate_decls): Make sure a structured binding is unique
in its declarative region.
(start_decl): Add new bool parameter.  If it's true, call
fit_decomposition_lang_decl.
* parser.c (cp_parser_decomposition_declaration): Pass true to
start_decl.

* g++.dg/cpp1z/decomp52.C: New test.
---
 gcc/cp/cp-tree.h  |  7 +--
 gcc/cp/decl.c | 13 +++--
 gcc/cp/parser.c   |  3 ++-
 gcc/testsuite/g++.dg/cpp1z/decomp52.C | 14 ++
 4 files changed, 32 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp1z/decomp52.C

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index f7c11bcf838..1f1f5b69313 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6525,8 +6525,11 @@ extern void warn_misplaced_attr_for_class_type  
(location_t location,
 tree class_type);
 extern tree check_tag_decl (cp_decl_specifier_seq *, bool);
 extern tree shadow_tag (cp_decl_specifier_seq *);
-extern tree groktypename   (cp_decl_specifier_seq *, const 
cp_declarator *, bool);
-extern tree start_decl (const cp_declarator *, 
cp_decl_specifier_seq *, int, tree, tree, tree *);
+extern tree groktypename   (cp_decl_specifier_seq *,
+const cp_declarator *, bool);
+extern tree start_decl (const cp_declarator *,
+cp_decl_specifier_seq *, int,
+tree, tree, tree *, bool = 
false);
 extern void start_decl_1   (tree, bool);
 extern bool check_array_initializer(tree, tree, tree);
 extern void omp_declare_variant_finalize   (tree, tree);
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 1b6a5672334..6464dbbfc99 100644
--- 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.  */
@@ -5199,7 +5202,8 @@ groktypename (cp_decl_specifier_seq *type_specifiers,
The scope represented by the context of the returned DECL is pushed
(if it is not the global namespace) and is assigned to
*PUSHED_SCOPE_P.  The caller is then responsible for calling
-   pop_scope on *PUSHED_SCOPE_P if it is set.  */
+   pop_scope on *PUSHED_SCOPE_P if it is set.  If FORCE_LANG_DECL_P, create
+   a lang decl node for the new decl.  */
 
 tree
 start_decl (const cp_declarator *declarator,
@@ -5207,7 +5211,8 @@ start_decl (const cp_declarator *declarator,
int initialized,
tree attributes,
tree prefix_attributes,
-   tree *pushed_scope_p)
+   tree *pushed_scope_p,
+   bool force_lang_decl_p/*=false*/)
 {
   tree decl;
   tree context;
@@ -5387,6 +5392,10 @@ start_decl (const cp_declarator *declarator,
   decl);
 }
 
+  /* Create a DECL_LANG_SPECIFIC so that DECL_DECOMPOSITION_P works.  */
+  if (force_lang_decl_p)
+fit_decomposition_lang_decl (decl, NULL_TREE);
+
   was_public = TREE_PUBLIC (decl);
 
   /* Enter this declaration into the symbol table.  Don't push the plain
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index f1ddef220fe..06d6a262699