On Tue, May 19, 2020 at 10:39:59AM -0400, Jason Merrill wrote:
> 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, &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
> > >
> > > 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?
Sounds good, I agree that a new SD_ flag is better than adding a new parameter
for such a corner case.
I've reviewed the uses of the 'initialized' parameter in start_decl{,1},
grokdeclarator, and grokvardecl and adjusted where needed. It's kind of
tempting to use an enum (possibly scoped one, now that we're in C++11)
instead of a #define but I've resisted doing that thus far.
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
Thanks,
-- >8 --
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.
---
gcc/cp/cp-tree.h | 6 ++++--
gcc/cp/decl.c | 23 +++++++++++++++++------
gcc/cp/parser.c | 2 +-
gcc/testsuite/g++.dg/cpp1z/decomp52.C | 14 ++++++++++++++
4 files changed, 36 insertions(+), 9 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 31c30ff87b3..27707abaadc 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -5694,8 +5694,10 @@ enum overload_flags { NO_SPECIAL = 0, DTOR_FLAG,
TYPENAME_FLAG };
/* Used with start_decl's initialized parameter. */
#define SD_UNINITIALIZED 0
#define SD_INITIALIZED 1
-#define SD_DEFAULTED 2
-#define SD_DELETED 3
+/* Like SD_INITIALIZED, but also mark the new decl as DECL_DECOMPOSITION_P. */
+#define SD_DECOMPOSITION 2
+#define SD_DEFAULTED 3
+#define SD_DELETED 4
/* Returns nonzero iff TYPE1 and TYPE2 are the same type, or if TYPE2
is derived from TYPE1, or if TYPE2 is a pointer (reference) to a
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 6469850e2dc..b64fb3fe716 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. */
@@ -5190,13 +5193,17 @@ groktypename (cp_decl_specifier_seq *type_specifiers,
declarations appearing in the body of the class go through
grokfield.) The DECL corresponding to the DECLARATOR is returned.
If an error occurs, the error_mark_node is returned instead.
-
+
DECLSPECS are the decl-specifiers for the declaration. INITIALIZED is
SD_INITIALIZED if an explicit initializer is present, or SD_DEFAULTED
for an explicitly defaulted function, or SD_DELETED for an explicitly
deleted function, but 0 (SD_UNINITIALIZED) if this is a variable
- implicitly initialized via a default constructor. ATTRIBUTES and
- PREFIX_ATTRIBUTES are GNU attributes associated with this declaration.
+ implicitly initialized via a default constructor. It can also be
+ SD_DECOMPOSITION which behaves much like SD_INITIALIZED, but we also
+ mark the new decl as DECL_DECOMPOSITION_P.
+
+ ATTRIBUTES and PREFIX_ATTRIBUTES are GNU attributes associated with this
+ declaration.
The scope represented by the context of the returned DECL is pushed
(if it is not the global namespace) and is assigned to
@@ -5233,7 +5240,7 @@ start_decl (const cp_declarator *declarator,
*pushed_scope_p = push_scope (context);
/* Is it valid for this decl to have an initializer at all?
- If not, set INITIALIZED to zero, which will indirectly
+ If not, return error_mark_node, which will indirectly
tell `cp_finish_decl' to ignore the initializer once it is parsed. */
if (initialized
&& TREE_CODE (decl) == TYPE_DECL)
@@ -5389,6 +5396,10 @@ start_decl (const cp_declarator *declarator,
decl);
}
+ /* Create a DECL_LANG_SPECIFIC so that DECL_DECOMPOSITION_P works. */
+ if (initialized == SD_DECOMPOSITION)
+ fit_decomposition_lang_decl (decl, NULL_TREE);
+
was_public = TREE_PUBLIC (decl);
/* Enter this declaration into the symbol table. Don't push the plain
@@ -10997,7 +11008,7 @@ grokdeclarator (const cp_declarator *declarator,
else if (decl_context == TPARM)
template_parm_flag = true, decl_context = PARM;
- if (initialized > 1)
+ if (initialized == SD_DEFAULTED || initialized == SD_DELETED)
funcdef_flag = true;
location_t typespec_loc = smallest_type_location (type_quals,
@@ -13304,7 +13315,7 @@ grokdeclarator (const cp_declarator *declarator,
|| (!dependent_type_p (type)
&& !COMPLETE_TYPE_P (complete_type (type))
&& (!complete_or_array_type_p (type)
- || initialized == 0))))
+ || initialized == SD_UNINITIALIZED))))
{
if (TREE_CODE (type) != ARRAY_TYPE
|| !COMPLETE_TYPE_P (TREE_TYPE (type)))
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index e944841f5a3..a6a5d975af3 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -13974,7 +13974,7 @@ cp_parser_decomposition_declaration (cp_parser *parser,
declarator->id_loc = e.get_location ();
}
tree elt_pushed_scope;
- tree decl2 = start_decl (declarator, &decl_specs, SD_INITIALIZED,
+ tree decl2 = start_decl (declarator, &decl_specs, SD_DECOMPOSITION,
NULL_TREE, NULL_TREE, &elt_pushed_scope);
if (decl2 == error_mark_node)
decl = error_mark_node;
diff --git a/gcc/testsuite/g++.dg/cpp1z/decomp52.C
b/gcc/testsuite/g++.dg/cpp1z/decomp52.C
new file mode 100644
index 00000000000..43b76181b5e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/decomp52.C
@@ -0,0 +1,14 @@
+// DR 2289
+// PR c++/94553
+// { dg-do compile { target c++17 } }
+// A structured binding must be unique in its declarative region.
+
+void
+f ()
+{
+ int arr[1] = { 1 };
+ struct A { };
+ auto [A] = arr; // { dg-error "redeclared as different kind of entity" }
+ auto [B] = arr;
+ struct B { }; // { dg-error "redeclared as different kind of entity" }
+}
base-commit: b17a002ef579ed85edbe2c241222bfd0a86682db
--
Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA
