Re: C++ PATCH for c++/54325 (wrong error initializing abstract base class)
Fixed thus. For a user-provided default constructor we don't need to
play with zeroing the object first, so we can use the normal logic that
works properly for protected access.
Tested x86_64-pc-linux-gnu, applying to trunk and 4.7.
commit 0ebf6baa1f5f27bd96db44514425075cad2cbd97
Author: Jason Merrill ja...@redhat.com
Date: Wed Jan 2 15:31:02 2013 -0500
PR c++/54325
* call.c (build_new_method_call_1): Don't use build_value_init for
user-provided default constructors.
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index bba5d9f..ad39637 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -7534,6 +7534,9 @@ build_new_method_call_1 (tree instance, tree fns, vectree, va_gc **args,
build_special_member_call. */
if (CONSTRUCTOR_NELTS (init_list) == 0
TYPE_HAS_DEFAULT_CONSTRUCTOR (basetype)
+ /* For a user-provided default constructor, use the normal
+ mechanisms so that protected access works. */
+ !type_has_user_provided_default_constructor (basetype)
!processing_template_decl)
init = build_value_init (basetype, complain);
diff --git a/gcc/testsuite/g++.dg/cpp0x/initlist-protected.C b/gcc/testsuite/g++.dg/cpp0x/initlist-protected.C
new file mode 100644
index 000..fb5cc6a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/initlist-protected.C
@@ -0,0 +1,23 @@
+// PR c++/54325
+// { dg-options -std=c++11 }
+
+class base
+{
+protected:
+base()
+{}
+};
+
+class derived : public base
+{
+public:
+derived()
+: base{} // -- Note the c++11 curly brace syntax
+{}
+};
+
+int main()
+{
+derived d1;
+return 0;
+}
Re: C++ PATCH for c++/54325 (wrong error initializing abstract base class)
On 12/24/2012 03:29 AM, Paolo Carlini wrote: Are you sure your patch handles the access control issue too?? (isn't obvious to me that it does, looking at the patch itself and your comments) Nope, you're right. I put the testcase in one file and then compiled a different one. /facepalm Jason
Re: C++ PATCH for c++/54325 (wrong error initializing abstract base class)
On 12/24/2012 05:56 AM, Jason Merrill wrote: On 12/22/2012 06:02 PM, Paolo Carlini wrote: Well, we still reject it after the patch My 4.7 and trunk compilers both accept it (with -std=c++11, of course). I just updated and rebuilt my 4.7 for you (which definitely I didn't hack over the next days) and it still rejects it. Likewise mainline. Note that the *first* testcase in the Bug Report, the one involving the abtract base, now is correctly accepted by both. Are you sure your patch handles the access control issue too?? (isn't obvious to me that it does, looking at the patch itself and your comments) Thanks, Paolo.
Re: C++ PATCH for c++/54325 (wrong error initializing abstract base class)
... to explain more concretely what I mean, if I *brutally* hack
mainline per the below, then the testcase is accepted.
Paolo.
//
Index: call.c
===
--- call.c (revision 194659)
+++ call.c (working copy)
@@ -7535,7 +7535,11 @@ build_new_method_call_1 (tree instance, tree fns,
if (CONSTRUCTOR_NELTS (init_list) == 0
TYPE_HAS_DEFAULT_CONSTRUCTOR (basetype)
!processing_template_decl)
- init = build_value_init (basetype, complain);
+ {
+ push_deferring_access_checks (dk_no_check);
+ init = build_value_init (basetype, complain);
+ pop_deferring_access_checks ();
+ }
/* If BASETYPE is an aggregate, we need to do aggregate
initialization. */
Re: C++ PATCH for c++/54325 (wrong error initializing abstract base class)
On 12/22/2012 06:02 PM, Paolo Carlini wrote: Well, we still reject it after the patch My 4.7 and trunk compilers both accept it (with -std=c++11, of course). Jason
Re: C++ PATCH for c++/54325 (wrong error initializing abstract base class)
On 12/21/2012 06:38 AM, Paolo Carlini wrote: I was looking a bit more into this Bug, and something seems still weird about the testcase in Comment #1 of the audit trail, which we also didn't reject with 4.6.x: What's weird about it? Jason
Re: C++ PATCH for c++/54325 (wrong error initializing abstract base class)
Hi, Jason Merrill ja...@redhat.com ha scritto: On 12/21/2012 06:38 AM, Paolo Carlini wrote: I was looking a bit more into this Bug, and something seems still weird about the testcase in Comment #1 of the audit trail, which we also didn't reject with 4.6.x: What's weird about it? Well, we still reject it after the patch, whereas we didn't in 4.6.x. Apparently - I didn't check - clang also accepts it. By 'weird' I meant, isn't clear that the Bug is completely resolved... Paolo
Re: C++ PATCH for c++/54325 (wrong error initializing abstract base class)
Am 07.12.2012 06:05, schrieb Jason Merrill:
It's perfectly OK to initialize a base class of abstract type; it's only an
error to create a full object of such a type. So this patch moves the check
from more generic initialization code out into a function that's definitely
creating a new object.
Tested x86_64-pc-linux-gnu, applying to trunk and 4.7.
this doesn't build on the branch:
../gcc/cp/tree.c: In function 'build_aggr_init_expr':
../gcc/cp/tree.c:399:1: error: parameter name omitted
this fixes the bootstrap, currently running the testsuite.
--- cp/tree.c~ 2012-12-07 10:01:16.665415647 +0100
+++ cp/tree.c 2012-12-07 10:11:01.373410862 +0100
@@ -396,7 +396,8 @@
callable. */
tree
-build_aggr_init_expr (tree type, tree init, tsubst_flags_t /*complain*/)
+build_aggr_init_expr (tree type, tree init,
+ tsubst_flags_t complain ATTRIBUTE_UNUSED)
{
tree fn;
tree slot;
Re: C++ PATCH for c++/54325 (wrong error initializing abstract base class)
On Fri, Dec 07, 2012 at 10:13:11AM +0100, Matthias Klose wrote:
Am 07.12.2012 06:05, schrieb Jason Merrill:
It's perfectly OK to initialize a base class of abstract type; it's only an
error to create a full object of such a type. So this patch moves the check
from more generic initialization code out into a function that's definitely
creating a new object.
Tested x86_64-pc-linux-gnu, applying to trunk and 4.7.
this doesn't build on the branch:
../gcc/cp/tree.c: In function 'build_aggr_init_expr':
../gcc/cp/tree.c:399:1: error: parameter name omitted
this fixes the bootstrap, currently running the testsuite.
Please commit as obvious with appropriate ChangeLog entry.
--- cp/tree.c~2012-12-07 10:01:16.665415647 +0100
+++ cp/tree.c 2012-12-07 10:11:01.373410862 +0100
@@ -396,7 +396,8 @@
callable. */
tree
-build_aggr_init_expr (tree type, tree init, tsubst_flags_t /*complain*/)
+build_aggr_init_expr (tree type, tree init,
+ tsubst_flags_t complain ATTRIBUTE_UNUSED)
{
tree fn;
tree slot;
Jakub
Re: C++ PATCH for c++/54325 (wrong error initializing abstract base class)
Am 07.12.2012 10:17, schrieb Jakub Jelinek:
On Fri, Dec 07, 2012 at 10:13:11AM +0100, Matthias Klose wrote:
Am 07.12.2012 06:05, schrieb Jason Merrill:
It's perfectly OK to initialize a base class of abstract type; it's only an
error to create a full object of such a type. So this patch moves the check
from more generic initialization code out into a function that's definitely
creating a new object.
Tested x86_64-pc-linux-gnu, applying to trunk and 4.7.
this doesn't build on the branch:
../gcc/cp/tree.c: In function 'build_aggr_init_expr':
../gcc/cp/tree.c:399:1: error: parameter name omitted
this fixes the bootstrap, currently running the testsuite.
Please commit as obvious with appropriate ChangeLog entry.
--- cp/tree.c~ 2012-12-07 10:01:16.665415647 +0100
+++ cp/tree.c2012-12-07 10:11:01.373410862 +0100
@@ -396,7 +396,8 @@
callable. */
tree
-build_aggr_init_expr (tree type, tree init, tsubst_flags_t /*complain*/)
+build_aggr_init_expr (tree type, tree init,
+ tsubst_flags_t complain ATTRIBUTE_UNUSED)
{
tree fn;
tree slot;
Jakub
comitted.
Matthias
2012-12-07 Matthias Klose d...@ubuntu.com
* tree.c (build_aggr_init_expr): Add parameter name, mark as unused.
Re: C++ PATCH for c++/54325 (wrong error initializing abstract base class)
Hi,
On 12/07/2012 10:24 AM, Matthias Klose wrote:
committed.
Matthias
2012-12-07 Matthias Klose d...@ubuntu.com
* tree.c (build_aggr_init_expr): Add parameter name, mark as unused.
Thanks.
I was wondering if in mainline we could just do the below. It seems
straightforward enough...
Thanks!
Paolo.
///
2012-12-07 Paolo Carlini paolo.carl...@oracle.com
* tree.c (build_aggr_init_expr): Remove tsubst_flags_t parameter.
(build_cplus_new): Adjust.
* cp-tree.h: Adjust declaration.
* init.c (build_value_init): Adjust.
Index: cp-tree.h
===
--- cp-tree.h (revision 194296)
+++ cp-tree.h (working copy)
@@ -5762,7 +5762,7 @@ extern tree build_min_nt_loc
(location_t, enum t
extern tree build_min_non_dep (enum tree_code, tree, ...);
extern tree build_min_non_dep_call_vec (tree, tree, vectree, va_gc
*);
extern tree build_cplus_new(tree, tree, tsubst_flags_t);
-extern tree build_aggr_init_expr (tree, tree, tsubst_flags_t);
+extern tree build_aggr_init_expr (tree, tree);
extern tree get_target_expr(tree);
extern tree get_target_expr_sfinae (tree, tsubst_flags_t);
extern tree build_cplus_array_type (tree, tree);
Index: init.c
===
--- init.c (revision 194296)
+++ init.c (working copy)
@@ -350,8 +350,7 @@ build_value_init (tree type, tsubst_flags_t compla
(type,
build_special_member_call (NULL_TREE, complete_ctor_identifier,
NULL, type, LOOKUP_NORMAL,
- complain),
- complain);
+ complain));
else if (TYPE_HAS_COMPLEX_DFLT (type))
{
/* This is a class that needs constructing, but doesn't have
@@ -361,7 +360,7 @@ build_value_init (tree type, tsubst_flags_t compla
tree ctor = build_special_member_call
(NULL_TREE, complete_ctor_identifier,
NULL, type, LOOKUP_NORMAL, complain);
- ctor = build_aggr_init_expr (type, ctor, complain);
+ ctor = build_aggr_init_expr (type, ctor);
if (ctor != error_mark_node)
AGGR_INIT_ZERO_FIRST (ctor) = 1;
return ctor;
Index: tree.c
===
--- tree.c (revision 194296)
+++ tree.c (working copy)
@@ -407,7 +407,7 @@ build_aggr_init_array (tree return_type, tree fn,
callable. */
tree
-build_aggr_init_expr (tree type, tree init, tsubst_flags_t /*complain*/)
+build_aggr_init_expr (tree type, tree init)
{
tree fn;
tree slot;
@@ -469,7 +469,7 @@ tree
tree
build_cplus_new (tree type, tree init, tsubst_flags_t complain)
{
- tree rval = build_aggr_init_expr (type, init, complain);
+ tree rval = build_aggr_init_expr (type, init);
tree slot;
/* Make sure that we're not trying to create an instance of an
Re: C++ PATCH for c++/54325 (wrong error initializing abstract base class)
On Fri, Dec 7, 2012 at 3:13 AM, Matthias Klose d...@ubuntu.com wrote:
Am 07.12.2012 06:05, schrieb Jason Merrill:
It's perfectly OK to initialize a base class of abstract type; it's only an
error to create a full object of such a type. So this patch moves the check
from more generic initialization code out into a function that's definitely
creating a new object.
Tested x86_64-pc-linux-gnu, applying to trunk and 4.7.
this doesn't build on the branch:
../gcc/cp/tree.c: In function 'build_aggr_init_expr':
../gcc/cp/tree.c:399:1: error: parameter name omitted
this fixes the bootstrap, currently running the testsuite.
--- cp/tree.c~ 2012-12-07 10:01:16.665415647 +0100
+++ cp/tree.c 2012-12-07 10:11:01.373410862 +0100
@@ -396,7 +396,8 @@
callable. */
tree
-build_aggr_init_expr (tree type, tree init, tsubst_flags_t /*complain*/)
+build_aggr_init_expr (tree type, tree init,
+ tsubst_flags_t complain ATTRIBUTE_UNUSED)
{
tree fn;
tree slot;
We should definitely teach the compiler to accept the former and not
be silly in requiring the latter when C++.
-- Gaby
Re: C++ PATCH for c++/54325 (wrong error initializing abstract base class)
On Fri, Dec 07, 2012 at 07:34:30AM -0600, Gabriel Dos Reis wrote:
On Fri, Dec 7, 2012 at 3:13 AM, Matthias Klose d...@ubuntu.com wrote:
Am 07.12.2012 06:05, schrieb Jason Merrill:
It's perfectly OK to initialize a base class of abstract type; it's only an
error to create a full object of such a type. So this patch moves the
check
from more generic initialization code out into a function that's definitely
creating a new object.
Tested x86_64-pc-linux-gnu, applying to trunk and 4.7.
this doesn't build on the branch:
../gcc/cp/tree.c: In function 'build_aggr_init_expr':
../gcc/cp/tree.c:399:1: error: parameter name omitted
this fixes the bootstrap, currently running the testsuite.
--- cp/tree.c~ 2012-12-07 10:01:16.665415647 +0100
+++ cp/tree.c 2012-12-07 10:11:01.373410862 +0100
@@ -396,7 +396,8 @@
callable. */
tree
-build_aggr_init_expr (tree type, tree init, tsubst_flags_t /*complain*/)
+build_aggr_init_expr (tree type, tree init,
+ tsubst_flags_t complain ATTRIBUTE_UNUSED)
{
tree fn;
tree slot;
We should definitely teach the compiler to accept the former and not
be silly in requiring the latter when C++.
Except that GCC 4.7 doesn't mandate building with C++, so the sources must
be valid C.
Jakub
Re: C++ PATCH for c++/54325 (wrong error initializing abstract base class)
On Fri, Dec 7, 2012 at 7:39 AM, Jakub Jelinek ja...@redhat.com wrote:
On Fri, Dec 07, 2012 at 07:34:30AM -0600, Gabriel Dos Reis wrote:
On Fri, Dec 7, 2012 at 3:13 AM, Matthias Klose d...@ubuntu.com wrote:
Am 07.12.2012 06:05, schrieb Jason Merrill:
It's perfectly OK to initialize a base class of abstract type; it's only
an
error to create a full object of such a type. So this patch moves the
check
from more generic initialization code out into a function that's
definitely
creating a new object.
Tested x86_64-pc-linux-gnu, applying to trunk and 4.7.
this doesn't build on the branch:
../gcc/cp/tree.c: In function 'build_aggr_init_expr':
../gcc/cp/tree.c:399:1: error: parameter name omitted
this fixes the bootstrap, currently running the testsuite.
--- cp/tree.c~ 2012-12-07 10:01:16.665415647 +0100
+++ cp/tree.c 2012-12-07 10:11:01.373410862 +0100
@@ -396,7 +396,8 @@
callable. */
tree
-build_aggr_init_expr (tree type, tree init, tsubst_flags_t /*complain*/)
+build_aggr_init_expr (tree type, tree init,
+ tsubst_flags_t complain ATTRIBUTE_UNUSED)
{
tree fn;
tree slot;
We should definitely teach the compiler to accept the former and not
be silly in requiring the latter when C++.
Except that GCC 4.7 doesn't mandate building with C++, so the sources must
be valid C.
Jakub
Right you are!
Thanks,
-- Gaby
Re: C++ PATCH for c++/54325 (wrong error initializing abstract base class)
OK. Jason
C++ PATCH for c++/54325 (wrong error initializing abstract base class)
It's perfectly OK to initialize a base class of abstract type; it's only
an error to create a full object of such a type. So this patch moves
the check from more generic initialization code out into a function
that's definitely creating a new object.
Tested x86_64-pc-linux-gnu, applying to trunk and 4.7.
commit 6fb305c7c88b07c429e8a39fbd514a417c5b6127
Author: jason jason@138bc75d-0d04-0410-961f-82ee72b054a4
Date: Fri Dec 7 04:54:27 2012 +
PR c++/54325
* tree.c (build_aggr_init_expr): Don't check for abstract class.
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index 28ff0f2..ca82f75 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -407,18 +407,13 @@ build_aggr_init_array (tree return_type, tree fn, tree slot, int nargs,
callable. */
tree
-build_aggr_init_expr (tree type, tree init, tsubst_flags_t complain)
+build_aggr_init_expr (tree type, tree init, tsubst_flags_t /*complain*/)
{
tree fn;
tree slot;
tree rval;
int is_ctor;
- /* Make sure that we're not trying to create an instance of an
- abstract class. */
- if (abstract_virtuals_error_sfinae (NULL_TREE, type, complain))
-return error_mark_node;
-
if (TREE_CODE (init) == CALL_EXPR)
fn = CALL_EXPR_FN (init);
else if (TREE_CODE (init) == AGGR_INIT_EXPR)
@@ -477,6 +472,11 @@ build_cplus_new (tree type, tree init, tsubst_flags_t complain)
tree rval = build_aggr_init_expr (type, init, complain);
tree slot;
+ /* Make sure that we're not trying to create an instance of an
+ abstract class. */
+ if (abstract_virtuals_error_sfinae (NULL_TREE, type, complain))
+return error_mark_node;
+
if (TREE_CODE (rval) == AGGR_INIT_EXPR)
slot = AGGR_INIT_EXPR_SLOT (rval);
else if (TREE_CODE (rval) == CALL_EXPR
diff --git a/gcc/testsuite/g++.dg/cpp0x/initlist-pure.C b/gcc/testsuite/g++.dg/cpp0x/initlist-pure.C
new file mode 100644
index 000..63c341c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/initlist-pure.C
@@ -0,0 +1,25 @@
+// PR c++/54325
+// { dg-options -std=c++11 }
+
+class Base {
+public:
+ Base() {};
+ virtual ~Base() {};
+
+ virtual void do_stuff() = 0;
+};
+
+class Derived: public Base {
+public:
+ Derived() : Base{} {};
+ virtual ~Derived() {};
+
+ virtual void do_stuff() {};
+};
+
+int
+main() {
+ Derived d;
+
+ return 0;
+}
diff --git a/gcc/testsuite/g++.dg/other/abstract3.C b/gcc/testsuite/g++.dg/other/abstract3.C
index 528b7d7..95e293e 100644
--- a/gcc/testsuite/g++.dg/other/abstract3.C
+++ b/gcc/testsuite/g++.dg/other/abstract3.C
@@ -8,5 +8,5 @@ struct A // { dg-message note }
struct B
{
A a; // { dg-error abstract }
- B() : a() {} // { dg-error abstract }
+ B() : a() {}
};
