* [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) @ 2004-02-10 0:37 Giovanni Bajo 2004-02-10 2:40 ` Gabriel Dos Reis 2004-02-21 13:45 ` Giovanni Bajo 0 siblings, 2 replies; 46+ messages in thread From: Giovanni Bajo @ 2004-02-10 0:37 UTC (permalink / raw) To: gcc-patches Hello, this is a partial fix for PR2204, which is about checking if PARM_DECLs in function declarations have abstract types. Currently, the check is being performed in grokparms, which is fine for many cases, but it's way too early if the type is the class being defined, or an incomplete type. In that case, we need to defer the check later, when/if the type is completed. To do this, I allow PARM_DECLS to be registered in the incomplete_vars list (by grokparms) and check for abstractness upon type completition (finish_struct_1, which calls complete_vars). Notice that we cannot check it in start_function, because the program is ill-formed even if there is no definition for the function. Alas, this solution is partial, mainly because it doesn't get into account array types. In fact, parameters with array type are decayed into pointer types within grokdeclarator (even before a PARM_DECL is built), so they never get registered into the incomplete_vars list. A solution for this might be extracting the standard decays out of grokdeclarator, and having grokparms call it. There is also another user of grokdeclarator(PARM) (in pt.c, to build template parameters), which should be updated as well. Comments on this plan are highly appreciated. Another case we don't handle yet is when the type of the parameter is dependent. In that case, we probably need to check for abstractness upon instantiation, while tsubsting. I'll investigate. Meanwhile, this tested succesfully on i686-pc-linux-gnu. The testcase have a handful of xfails for the cases we don't handle yet. OK for mainline? Giovanni Bajo 2004-02-10 Giovanni Bajo <giovannibajo@gcc.gnu.org> PR c++/2204 * decl.c (cp_finish_decl): Extract logic to strip array and pointer to array types into... (strip_array_and_pointer_to_array_types): New function. (grokparms): Register PARM_DECLs of incomplete types for later check, using... (maybe_register_incomplete_parms): New function. (complete_vars): When completing the type for a PARM_DECL, check for abstractness. Fix a typo in the comment. * typeck2.c (abstract_virtual_errors): Reword diagnostics, make them appear at the correct location. 2004-02-10 Giovanni Bajo <giovannibajo@gcc.gnu.org> PR c++/2204 * g++.dg/other/abstract2.C: New test. Index: decl.c =================================================================== RCS file: /cvs/gcc/gcc/gcc/cp/decl.c,v retrieving revision 1.1179 diff -c -3 -p -r1.1179 decl.c *** decl.c 2 Feb 2004 16:53:02 -0000 1.1179 --- decl.c 9 Feb 2004 23:27:43 -0000 *************** static void expand_static_init (tree, tr *** 120,125 **** --- 120,127 ---- static tree next_initializable_field (tree); static tree reshape_init (tree, tree *); static tree build_typename_type (tree, tree, tree); + static void maybe_register_incomplete_parm (tree); + static tree strip_array_and_pointer_to_array_types (tree); /* Erroneous argument lists can use this *IFF* they do not modify it. */ tree error_mark_list; *************** cp_finish_decl (tree decl, tree init, tr *** 4883,4901 **** || TREE_CODE (type) == METHOD_TYPE) abstract_virtuals_error (decl, strip_array_types (TREE_TYPE (type))); ! else if (POINTER_TYPE_P (type) || TREE_CODE (type) == ARRAY_TYPE) ! { ! /* If it's either a pointer or an array type, strip through all ! of them but the last one. If the last is an array type, issue ! an error if the element type is abstract. */ ! while (POINTER_TYPE_P (TREE_TYPE (type)) ! || TREE_CODE (TREE_TYPE (type)) == ARRAY_TYPE) ! type = TREE_TYPE (type); ! if (TREE_CODE (type) == ARRAY_TYPE) ! abstract_virtuals_error (decl, TREE_TYPE (type)); ! } ! else ! abstract_virtuals_error (decl, type); if (TREE_CODE (decl) == FUNCTION_DECL || TREE_TYPE (decl) == error_mark_node) --- 4885,4894 ---- || TREE_CODE (type) == METHOD_TYPE) abstract_virtuals_error (decl, strip_array_types (TREE_TYPE (type))); ! else ! abstract_virtuals_error (decl, ! strip_array_and_pointer_to_array_types ! (type)); if (TREE_CODE (decl) == FUNCTION_DECL || TREE_TYPE (decl) == error_mark_node) *************** grokparms (tree first_parm) *** 8669,8674 **** --- 8660,8667 ---- type = build_pointer_type (type); TREE_TYPE (decl) = type; } + else if (!COMPLETE_TYPE_P (type) || currently_open_class (type)) + maybe_register_incomplete_parm (decl); else if (abstract_virtuals_error (decl, type)) any_error = 1; /* Seems like a good idea. */ else if (POINTER_TYPE_P (type)) *************** finish_method (tree decl) *** 11049,11055 **** return decl; } ! \f /* VAR is a VAR_DECL. If its type is incomplete, remember VAR so that we can lay it out later, when and if its type becomes complete. */ --- 11042,11098 ---- return decl; } ! ! /* Strip all the array/pointer-to-array types from TYPE. This is used to ! extract the underlying type from declarations such as: ! ! A a[2]; ! A (*a)[2]; ! A (**a[2])[2]; ! ! which are invalid if 'A' is an abstract type. */ ! ! static tree ! strip_array_and_pointer_to_array_types (tree type) ! { ! if (POINTER_TYPE_P (type) || TREE_CODE (type) == ARRAY_TYPE) ! { ! /* If it's either a pointer or an array type, strip through all ! of them but the last one. If the last is an array type, then ! we can return its element type. */ ! while (POINTER_TYPE_P (TREE_TYPE (type)) ! || TREE_CODE (TREE_TYPE (type)) == ARRAY_TYPE) ! type = TREE_TYPE (type); ! if (TREE_CODE (type) == ARRAY_TYPE) ! return TREE_TYPE (type); ! } ! return type; ! } ! ! /* PARM is a PARM_DECL, whose type is incomplete at the point of parsing. ! We keep track of these because we must issue an error if, upon ! completition, the type ends up being abstract. */ ! ! static void ! maybe_register_incomplete_parm (tree var) ! { ! my_friendly_assert (TREE_CODE (var) == PARM_DECL, 20040208); ! ! /* If the type is dependent, there is nothing we can do here. The ! diagnostic will have to be issued during template substitution. */ ! if (!dependent_type_p (TREE_TYPE (var))) ! { ! tree inner_type = TREE_TYPE (var); ! ! /* FIXME: We currently don't get any ARRAY_TYPE because grokdeclarator ! decays them to POINTER_TYPE too early. */ ! inner_type = strip_array_and_pointer_to_array_types (inner_type); ! inner_type = TYPE_MAIN_VARIANT (inner_type); ! ! if (!COMPLETE_TYPE_P (inner_type) && CLASS_TYPE_P (inner_type)) ! incomplete_vars = tree_cons (inner_type, var, incomplete_vars); ! } ! } /* VAR is a VAR_DECL. If its type is incomplete, remember VAR so that we can lay it out later, when and if its type becomes complete. */ *************** maybe_register_incomplete_var (tree var) *** 11078,11084 **** } /* Called when a class type (given by TYPE) is defined. If there are ! any existing VAR_DECLs whose type hsa been completed by this declaration, update them now. */ void --- 11121,11127 ---- } /* Called when a class type (given by TYPE) is defined. If there are ! any existing VAR_DECLs whose type has been completed by this declaration, update them now. */ void *************** complete_vars (tree type) *** 11095,11100 **** --- 11138,11148 ---- /* Complete the type of the variable. The VAR_DECL itself will be laid out in expand_expr. */ complete_type (TREE_TYPE (var)); + /* If it is a PARM_DECL, we need to emit an error if the + type is abstract, because this is why we registered it + here. */ + if (TREE_CODE (var) == PARM_DECL) + abstract_virtuals_error (var, TREE_TYPE (var)); /* Remove this entry from the list. */ *list = TREE_CHAIN (*list); } Index: typeck2.c =================================================================== RCS file: /cvs/gcc/gcc/gcc/cp/typeck2.c,v retrieving revision 1.154 diff -c -3 -p -r1.154 typeck2.c *** typeck2.c 22 Jan 2004 00:03:52 -0000 1.154 --- typeck2.c 9 Feb 2004 23:27:46 -0000 *************** abstract_virtuals_error (tree decl, tree *** 149,182 **** return 0; if (TREE_CODE (decl) == VAR_DECL) ! error ("cannot declare variable `%D' to be of type `%T'", ! decl, type); else if (TREE_CODE (decl) == PARM_DECL) ! error ("cannot declare parameter `%D' to be of type `%T'", ! decl, type); else if (TREE_CODE (decl) == FIELD_DECL) ! error ("cannot declare field `%D' to be of type `%T'", ! decl, type); else if (TREE_CODE (decl) == FUNCTION_DECL && TREE_CODE (TREE_TYPE (decl)) == METHOD_TYPE) ! error ("invalid return type for member function `%#D'", decl); else if (TREE_CODE (decl) == FUNCTION_DECL) ! error ("invalid return type for function `%#D'", decl); } else ! error ("cannot allocate an object of type `%T'", type); /* Only go through this once. */ if (TREE_PURPOSE (u) == NULL_TREE) { TREE_PURPOSE (u) = error_mark_node; ! error (" because the following virtual functions are abstract:"); for (tu = u; tu; tu = TREE_CHAIN (tu)) ! cp_error_at ("\t%#D", TREE_VALUE (tu)); } else ! error (" since type `%T' has abstract virtual functions", type); return 1; } --- 149,196 ---- return 0; if (TREE_CODE (decl) == VAR_DECL) ! error ("%Jcannot declare variable `%D' to be of abstract type `%T'", ! decl, decl, type); else if (TREE_CODE (decl) == PARM_DECL) ! error ("%Jcannot declare parameter `%D' to be of abstract type `%T'", ! decl, decl, type); else if (TREE_CODE (decl) == FIELD_DECL) ! error ("%Jcannot declare field `%D' to be of abstract type `%T'", ! decl, decl, type); else if (TREE_CODE (decl) == FUNCTION_DECL && TREE_CODE (TREE_TYPE (decl)) == METHOD_TYPE) ! error ("%Jinvalid abstract return type for member function `%#D'", ! decl, decl); else if (TREE_CODE (decl) == FUNCTION_DECL) ! error ("%Jinvalid abstract return type for function `%#D'", ! decl, decl); } else ! error ("cannot allocate an object of abstract type `%T'", type); /* Only go through this once. */ if (TREE_PURPOSE (u) == NULL_TREE) { TREE_PURPOSE (u) = error_mark_node; ! if (decl) ! inform ("%J because the following virtual functions are pure:", ! decl); ! else ! inform (" because the following virtual functions are pure:"); ! for (tu = u; tu; tu = TREE_CHAIN (tu)) ! inform ("%J\t%#D", TREE_VALUE (tu), TREE_VALUE (tu)); } else ! { ! if (decl) ! inform ("%J since type `%T' has pure virtual functions", ! decl, type); ! else ! inform (" since type `%T' has pure virtual functions", ! type); ! } return 1; } // { dg-do compile } // Contributed by Gabriel Dos Reis <gdr at integrable-solutions dot net> // PR c++/2204: Check for parameters of abstract type in function declarations. namespace N1 { struct X; struct Y1 { void g(X parm1); // { dg-error "abstract" } void g(X parm2[2]); // { dg-error "abstract" "" { xfail *-*-* } } }; template <int N> struct Y2 { void g(X parm3); // { dg-error "abstract" } void g(X parm4[2]); // { dg-error "abstract" "" { xfail *-*-* } } }; struct X { virtual void xfunc(void) = 0; // { dg-error "note" } }; } namespace N2 { struct X1 { virtual void xfunc(void) = 0; // { dg-error "note" } void g(X1 parm5); // { dg-error "abstract" } void g(X1 parm6[2]); // { dg-error "abstract" "" { xfail *-*-* } } }; template <int N> struct X2 { virtual void xfunc(void) = 0; // { dg-error "note" "" { xfail *-*-* } } void g(X2 parm7); // { dg-error "abstract" "" { xfail *-*-* } } void g(X2 parm8[2]); // { dg-error "abstract" "" { xfail *-*-* } } }; } namespace N3 { struct X { virtual void xfunc(void) = 0; // { dg-error "note" } }; void g(X parm9); // { dg-error "abstract" } void g(X parm10[2]); // { dg-error "abstract" "" { xfail *-*-* } } template <int N> void g(X parm11); // { dg-error "abstract" } template <int N> void g(X parm12[2]); // { dg-error "abstract" "" { xfail *-*-* } } } // { dg-excess-errors "following virtual functions|has pure virtual functions" } ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) 2004-02-10 0:37 [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) Giovanni Bajo @ 2004-02-10 2:40 ` Gabriel Dos Reis 2004-02-10 2:47 ` Zack Weinberg ` (2 more replies) 2004-02-21 13:45 ` Giovanni Bajo 1 sibling, 3 replies; 46+ messages in thread From: Gabriel Dos Reis @ 2004-02-10 2:40 UTC (permalink / raw) To: Giovanni Bajo; +Cc: gcc-patches "Giovanni Bajo" <giovannibajo@libero.it> writes: | if (TREE_CODE (decl) == VAR_DECL) | ! error ("%Jcannot declare variable `%D' to be of abstract type `%T'", | ! decl, decl, type); That is an abuse of %J. Use error_at instead. E.g. error_at ("cannot declare variable `%+D' to be of abstract type `%T'", decl, type); | ! error ("%Jcannot declare parameter `%D' to be of abstract type `%T'", | ! decl, decl, type); Ditto. | else if (TREE_CODE (decl) == FIELD_DECL) | ! error ("%Jcannot declare field `%D' to be of abstract type `%T'", | ! decl, decl, type); Ditto. | else if (TREE_CODE (decl) == FUNCTION_DECL | && TREE_CODE (TREE_TYPE (decl)) == METHOD_TYPE) | ! error ("%Jinvalid abstract return type for member function `%#D'", | ! decl, decl); Ditto. | else if (TREE_CODE (decl) == FUNCTION_DECL) | ! error ("%Jinvalid abstract return type for function `%#D'", | ! decl, decl); Ditto. As a side comment, I don't understand why you went through that change. -- Gaby ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) 2004-02-10 2:40 ` Gabriel Dos Reis @ 2004-02-10 2:47 ` Zack Weinberg 2004-02-10 2:50 ` Gabriel Dos Reis 2004-02-21 13:45 ` Zack Weinberg 2004-02-10 3:12 ` Giovanni Bajo 2004-02-21 13:45 ` Gabriel Dos Reis 2 siblings, 2 replies; 46+ messages in thread From: Zack Weinberg @ 2004-02-10 2:47 UTC (permalink / raw) To: Gabriel Dos Reis; +Cc: Giovanni Bajo, gcc-patches Gabriel Dos Reis <gdr@integrable-solutions.net> writes: > "Giovanni Bajo" <giovannibajo@libero.it> writes: > > | if (TREE_CODE (decl) == VAR_DECL) > | ! error ("%Jcannot declare variable `%D' to be of abstract type `%T'", > | ! decl, decl, type); > > That is an abuse of %J. Use error_at instead. [etc] I told Giovanni that error/warning/pedwarn_at were to be eliminated in favor of %J. Did I misunderstand what you've said about these in the past? zw ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) 2004-02-10 2:47 ` Zack Weinberg @ 2004-02-10 2:50 ` Gabriel Dos Reis 2004-02-10 3:43 ` Zack Weinberg 2004-02-21 13:45 ` Gabriel Dos Reis 2004-02-21 13:45 ` Zack Weinberg 1 sibling, 2 replies; 46+ messages in thread From: Gabriel Dos Reis @ 2004-02-10 2:50 UTC (permalink / raw) To: Zack Weinberg; +Cc: Giovanni Bajo, gcc-patches "Zack Weinberg" <zack@codesourcery.com> writes: | Gabriel Dos Reis <gdr@integrable-solutions.net> writes: | | > "Giovanni Bajo" <giovannibajo@libero.it> writes: | > | > | if (TREE_CODE (decl) == VAR_DECL) | > | ! error ("%Jcannot declare variable `%D' to be of abstract type `%T'", | > | ! decl, decl, type); | > | > That is an abuse of %J. Use error_at instead. | [etc] | | I told Giovanni that error/warning/pedwarn_at were to be eliminated in | favor of %J. Did I misunderstand what you've said about these in the | past? I don't know exactly what you're referring to. But, the repetition of the "decl" is not right and if any removal should lead to that, then that removal should not happen. There are some uses of error_at which are wrong. E.g. error_at ("foo is nonsensical", decl); // WRONG error ("%Jfoo is nonsensical", decl); // Good error ("%Jdeclaration '%D' is nonsensical", decl, decl); // WRONG error_at ("declaration '%+D' is nonsensical", decl); // Good -- Gaby ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) 2004-02-10 2:50 ` Gabriel Dos Reis @ 2004-02-10 3:43 ` Zack Weinberg 2004-02-10 4:27 ` Gabriel Dos Reis 2004-02-21 13:45 ` Zack Weinberg 2004-02-21 13:45 ` Gabriel Dos Reis 1 sibling, 2 replies; 46+ messages in thread From: Zack Weinberg @ 2004-02-10 3:43 UTC (permalink / raw) To: Gabriel Dos Reis; +Cc: Giovanni Bajo, gcc-patches Gabriel Dos Reis <gdr@integrable-solutions.net> writes: > "Zack Weinberg" <zack@codesourcery.com> writes: > > | Gabriel Dos Reis <gdr@integrable-solutions.net> writes: > | > | > "Giovanni Bajo" <giovannibajo@libero.it> writes: > | > > | > | if (TREE_CODE (decl) == VAR_DECL) > | > | ! error ("%Jcannot declare variable `%D' to be of abstract type `%T'", > | > | ! decl, decl, type); > | > > | > That is an abuse of %J. Use error_at instead. > | [etc] > | > | I told Giovanni that error/warning/pedwarn_at were to be eliminated in > | favor of %J. Did I misunderstand what you've said about these in the > | past? > > I don't know exactly what you're referring to. But, the repetition of > the "decl" is not right and if any removal should lead to that, then > that removal should not happen. I don't understand this. The %J consumes one argument and the %D consumes another one. Why is this "wrong"? > error ("%Jdeclaration '%D' is nonsensical", decl, decl); // WRONG > error_at ("declaration '%+D' is nonsensical", decl); // Good The %+D notation is tidier, but I had understood you to be of the opinion that it was not worth the implementation mess, nor the baggage of carrying around two different sets of diagnostic functions. (If we ever get Chiaki's indexed-parameters-notation patch laid down, it will also allow implementing %+x cleanly and for all languages.) zw ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) 2004-02-10 3:43 ` Zack Weinberg @ 2004-02-10 4:27 ` Gabriel Dos Reis 2004-02-10 4:31 ` Zack Weinberg 2004-02-21 13:45 ` Gabriel Dos Reis 2004-02-21 13:45 ` Zack Weinberg 1 sibling, 2 replies; 46+ messages in thread From: Gabriel Dos Reis @ 2004-02-10 4:27 UTC (permalink / raw) To: Zack Weinberg; +Cc: Giovanni Bajo, gcc-patches "Zack Weinberg" <zack@codesourcery.com> writes: | Gabriel Dos Reis <gdr@integrable-solutions.net> writes: | | > "Zack Weinberg" <zack@codesourcery.com> writes: | > | > | Gabriel Dos Reis <gdr@integrable-solutions.net> writes: | > | | > | > "Giovanni Bajo" <giovannibajo@libero.it> writes: | > | > | > | > | if (TREE_CODE (decl) == VAR_DECL) | > | > | ! error ("%Jcannot declare variable `%D' to be of abstract type `%T'", | > | > | ! decl, decl, type); | > | > | > | > That is an abuse of %J. Use error_at instead. | > | [etc] | > | | > | I told Giovanni that error/warning/pedwarn_at were to be eliminated in | > | favor of %J. Did I misunderstand what you've said about these in the | > | past? | > | > I don't know exactly what you're referring to. But, the repetition of | > the "decl" is not right and if any removal should lead to that, then | > that removal should not happen. | | I don't understand this. The %J consumes one argument and the %D | consumes another one. Why is this "wrong"? The repetition oan information that is already there. Any scheme that requires repetition of information is wrong. | > error ("%Jdeclaration '%D' is nonsensical", decl, decl); // WRONG | > error_at ("declaration '%+D' is nonsensical", decl); // Good | | The %+D notation is tidier, but I had understood you to be of the | opinion that it was not worth the implementation mess, nor the baggage | of carrying around two different sets of diagnostic functions. If you have error() recognizing '%+' properly then error_at() should go. The whole purpose of xxx_at() begin there is that xxx() does not support yet '%+'. -- Gaby ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) 2004-02-10 4:27 ` Gabriel Dos Reis @ 2004-02-10 4:31 ` Zack Weinberg 2004-02-10 4:58 ` Gabriel Dos Reis 2004-02-21 13:45 ` Zack Weinberg 2004-02-21 13:45 ` Gabriel Dos Reis 1 sibling, 2 replies; 46+ messages in thread From: Zack Weinberg @ 2004-02-10 4:31 UTC (permalink / raw) To: Gabriel Dos Reis; +Cc: Giovanni Bajo, gcc-patches Gabriel Dos Reis <gdr@integrable-solutions.net> writes: > "Zack Weinberg" <zack@codesourcery.com> writes: > | Gabriel Dos Reis <gdr@integrable-solutions.net> writes: > | > I don't know exactly what you're referring to. But, the repetition of > | > the "decl" is not right and if any removal should lead to that, then > | > that removal should not happen. > | > | I don't understand this. The %J consumes one argument and the %D > | consumes another one. Why is this "wrong"? > > The repetition oan information that is already there. Any scheme that > requires repetition of information is wrong. Well, this is not consistent with what you have said before, and I wonder why you didn't object to %J when it was originally added. We have %J...%D all over the C front end these days. zw ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) 2004-02-10 4:31 ` Zack Weinberg @ 2004-02-10 4:58 ` Gabriel Dos Reis 2004-02-10 6:47 ` Zack Weinberg 2004-02-21 13:45 ` Gabriel Dos Reis 2004-02-21 13:45 ` Zack Weinberg 1 sibling, 2 replies; 46+ messages in thread From: Gabriel Dos Reis @ 2004-02-10 4:58 UTC (permalink / raw) To: Zack Weinberg; +Cc: Giovanni Bajo, gcc-patches "Zack Weinberg" <zack@codesourcery.com> writes: | Gabriel Dos Reis <gdr@integrable-solutions.net> writes: | > "Zack Weinberg" <zack@codesourcery.com> writes: | > | Gabriel Dos Reis <gdr@integrable-solutions.net> writes: | > | > I don't know exactly what you're referring to. But, the repetition of | > | > the "decl" is not right and if any removal should lead to that, then | > | > that removal should not happen. | > | | > | I don't understand this. The %J consumes one argument and the %D | > | consumes another one. Why is this "wrong"? | > | > The repetition oan information that is already there. Any scheme that | > requires repetition of information is wrong. | | Well, this is not consistent with what you have said before, and I Again, I don't understand what you're talking about. I have never been a fan of information/code duplication. Did you see me advocating for information/code duplication in the past? I want to get rid of xxx_at() if xxx() works correctly. | wonder why you didn't object to %J when it was originally added. We Most certainly because when it was added, it was to replace previous attempts to remove xxx_with_decl() that where using %s when actually a tree was being passed. | have %J...%D all over the C front end these days. Yes, but I do not believe the situation is comparable. The C front-end never had xxx_at(). -- Gaby ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) 2004-02-10 4:58 ` Gabriel Dos Reis @ 2004-02-10 6:47 ` Zack Weinberg 2004-02-10 10:05 ` Gabriel Dos Reis 2004-02-21 13:45 ` Zack Weinberg 2004-02-21 13:45 ` Gabriel Dos Reis 1 sibling, 2 replies; 46+ messages in thread From: Zack Weinberg @ 2004-02-10 6:47 UTC (permalink / raw) To: Gabriel Dos Reis; +Cc: Giovanni Bajo, gcc-patches Gabriel Dos Reis <gdr@integrable-solutions.net> writes: > | Gabriel Dos Reis <gdr@integrable-solutions.net> writes: > | > The repetition oan information that is already there. Any scheme that > | > requires repetition of information is wrong. > | > | Well, this is not consistent with what you have said before, and I > > Again, I don't understand what you're talking about. You were quite adamant in the past that xxx_at() must go. So adamant, in fact, that I thought it was already dead. My position is: * locate_error is a nasty thing that should go away. * ", decl, decl" isn't pretty but it isn't as nasty as locate_error. * Consistency between the C and C++ front ends is desirable. and most importantly * Why are we holding up a bug fix with this stupid argument anyway? zw ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) 2004-02-10 6:47 ` Zack Weinberg @ 2004-02-10 10:05 ` Gabriel Dos Reis 2004-02-10 10:36 ` Zack Weinberg 2004-02-21 13:45 ` Gabriel Dos Reis 2004-02-21 13:45 ` Zack Weinberg 1 sibling, 2 replies; 46+ messages in thread From: Gabriel Dos Reis @ 2004-02-10 10:05 UTC (permalink / raw) To: Zack Weinberg; +Cc: Giovanni Bajo, gcc-patches "Zack Weinberg" <zack@codesourcery.com> writes: | Gabriel Dos Reis <gdr@integrable-solutions.net> writes: | > | Gabriel Dos Reis <gdr@integrable-solutions.net> writes: | > | > The repetition oan information that is already there. Any scheme that | > | > requires repetition of information is wrong. | > | | > | Well, this is not consistent with what you have said before, and I | > | > Again, I don't understand what you're talking about. | | You were quite adamant in the past that xxx_at() must go. So adamant, | in fact, that I thought it was already dead. | | My position is: | | * locate_error is a nasty thing that should go away. | * ", decl, decl" isn't pretty but it isn't as nasty as locate_error. | * Consistency between the C and C++ front ends is desirable. | | and most importantly | | * Why are we holding up a bug fix with this stupid argument anyway? What I find stupid is your current position. Quite frankly, it is incomprehensible to me. You came in keeping saying that I used to be yyy in the past so zzz must be. Even when I explained what I mean. In fact, it is -you- holding up a patch with this stupid argument that I used to be xxx. I think that xxx_at() should be used until xxx() supports '%+', in particular I recommend Giovanni makes the modification I suggested. -- Gaby ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) 2004-02-10 10:05 ` Gabriel Dos Reis @ 2004-02-10 10:36 ` Zack Weinberg 2004-02-10 10:57 ` Gabriel Dos Reis 2004-02-21 13:45 ` Zack Weinberg 2004-02-21 13:45 ` Gabriel Dos Reis 1 sibling, 2 replies; 46+ messages in thread From: Zack Weinberg @ 2004-02-10 10:36 UTC (permalink / raw) To: Gabriel Dos Reis; +Cc: Giovanni Bajo, gcc-patches If you changed your mind you could just say so, rather than screaming at me. zw ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) 2004-02-10 10:36 ` Zack Weinberg @ 2004-02-10 10:57 ` Gabriel Dos Reis 2004-02-21 13:45 ` Gabriel Dos Reis 2004-02-21 13:45 ` Zack Weinberg 1 sibling, 1 reply; 46+ messages in thread From: Gabriel Dos Reis @ 2004-02-10 10:57 UTC (permalink / raw) To: Zack Weinberg; +Cc: Giovanni Bajo, gcc-patches "Zack Weinberg" <zack@codesourcery.com> writes: | If you changed your mind you could just say so, rather than screaming | at me. (1) I'm not screaming at you. (2) I did not change my mind. I'm still of the opinion that xxx_at must go, but not before xxxx() supports '%+'. (3) Why do you keep saying I used to be xxx? -- Gaby ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) 2004-02-10 10:57 ` Gabriel Dos Reis @ 2004-02-21 13:45 ` Gabriel Dos Reis 0 siblings, 0 replies; 46+ messages in thread From: Gabriel Dos Reis @ 2004-02-21 13:45 UTC (permalink / raw) To: Zack Weinberg; +Cc: Giovanni Bajo, gcc-patches "Zack Weinberg" <zack@codesourcery.com> writes: | If you changed your mind you could just say so, rather than screaming | at me. (1) I'm not screaming at you. (2) I did not change my mind. I'm still of the opinion that xxx_at must go, but not before xxxx() supports '%+'. (3) Why do you keep saying I used to be xxx? -- Gaby ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) 2004-02-10 10:36 ` Zack Weinberg 2004-02-10 10:57 ` Gabriel Dos Reis @ 2004-02-21 13:45 ` Zack Weinberg 1 sibling, 0 replies; 46+ messages in thread From: Zack Weinberg @ 2004-02-21 13:45 UTC (permalink / raw) To: Gabriel Dos Reis; +Cc: Giovanni Bajo, gcc-patches If you changed your mind you could just say so, rather than screaming at me. zw ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) 2004-02-10 10:05 ` Gabriel Dos Reis 2004-02-10 10:36 ` Zack Weinberg @ 2004-02-21 13:45 ` Gabriel Dos Reis 1 sibling, 0 replies; 46+ messages in thread From: Gabriel Dos Reis @ 2004-02-21 13:45 UTC (permalink / raw) To: Zack Weinberg; +Cc: Giovanni Bajo, gcc-patches "Zack Weinberg" <zack@codesourcery.com> writes: | Gabriel Dos Reis <gdr@integrable-solutions.net> writes: | > | Gabriel Dos Reis <gdr@integrable-solutions.net> writes: | > | > The repetition oan information that is already there. Any scheme that | > | > requires repetition of information is wrong. | > | | > | Well, this is not consistent with what you have said before, and I | > | > Again, I don't understand what you're talking about. | | You were quite adamant in the past that xxx_at() must go. So adamant, | in fact, that I thought it was already dead. | | My position is: | | * locate_error is a nasty thing that should go away. | * ", decl, decl" isn't pretty but it isn't as nasty as locate_error. | * Consistency between the C and C++ front ends is desirable. | | and most importantly | | * Why are we holding up a bug fix with this stupid argument anyway? What I find stupid is your current position. Quite frankly, it is incomprehensible to me. You came in keeping saying that I used to be yyy in the past so zzz must be. Even when I explained what I mean. In fact, it is -you- holding up a patch with this stupid argument that I used to be xxx. I think that xxx_at() should be used until xxx() supports '%+', in particular I recommend Giovanni makes the modification I suggested. -- Gaby ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) 2004-02-10 6:47 ` Zack Weinberg 2004-02-10 10:05 ` Gabriel Dos Reis @ 2004-02-21 13:45 ` Zack Weinberg 1 sibling, 0 replies; 46+ messages in thread From: Zack Weinberg @ 2004-02-21 13:45 UTC (permalink / raw) To: Gabriel Dos Reis; +Cc: Giovanni Bajo, gcc-patches Gabriel Dos Reis <gdr@integrable-solutions.net> writes: > | Gabriel Dos Reis <gdr@integrable-solutions.net> writes: > | > The repetition oan information that is already there. Any scheme that > | > requires repetition of information is wrong. > | > | Well, this is not consistent with what you have said before, and I > > Again, I don't understand what you're talking about. You were quite adamant in the past that xxx_at() must go. So adamant, in fact, that I thought it was already dead. My position is: * locate_error is a nasty thing that should go away. * ", decl, decl" isn't pretty but it isn't as nasty as locate_error. * Consistency between the C and C++ front ends is desirable. and most importantly * Why are we holding up a bug fix with this stupid argument anyway? zw ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) 2004-02-10 4:58 ` Gabriel Dos Reis 2004-02-10 6:47 ` Zack Weinberg @ 2004-02-21 13:45 ` Gabriel Dos Reis 1 sibling, 0 replies; 46+ messages in thread From: Gabriel Dos Reis @ 2004-02-21 13:45 UTC (permalink / raw) To: Zack Weinberg; +Cc: Giovanni Bajo, gcc-patches "Zack Weinberg" <zack@codesourcery.com> writes: | Gabriel Dos Reis <gdr@integrable-solutions.net> writes: | > "Zack Weinberg" <zack@codesourcery.com> writes: | > | Gabriel Dos Reis <gdr@integrable-solutions.net> writes: | > | > I don't know exactly what you're referring to. But, the repetition of | > | > the "decl" is not right and if any removal should lead to that, then | > | > that removal should not happen. | > | | > | I don't understand this. The %J consumes one argument and the %D | > | consumes another one. Why is this "wrong"? | > | > The repetition oan information that is already there. Any scheme that | > requires repetition of information is wrong. | | Well, this is not consistent with what you have said before, and I Again, I don't understand what you're talking about. I have never been a fan of information/code duplication. Did you see me advocating for information/code duplication in the past? I want to get rid of xxx_at() if xxx() works correctly. | wonder why you didn't object to %J when it was originally added. We Most certainly because when it was added, it was to replace previous attempts to remove xxx_with_decl() that where using %s when actually a tree was being passed. | have %J...%D all over the C front end these days. Yes, but I do not believe the situation is comparable. The C front-end never had xxx_at(). -- Gaby ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) 2004-02-10 4:31 ` Zack Weinberg 2004-02-10 4:58 ` Gabriel Dos Reis @ 2004-02-21 13:45 ` Zack Weinberg 1 sibling, 0 replies; 46+ messages in thread From: Zack Weinberg @ 2004-02-21 13:45 UTC (permalink / raw) To: Gabriel Dos Reis; +Cc: Giovanni Bajo, gcc-patches Gabriel Dos Reis <gdr@integrable-solutions.net> writes: > "Zack Weinberg" <zack@codesourcery.com> writes: > | Gabriel Dos Reis <gdr@integrable-solutions.net> writes: > | > I don't know exactly what you're referring to. But, the repetition of > | > the "decl" is not right and if any removal should lead to that, then > | > that removal should not happen. > | > | I don't understand this. The %J consumes one argument and the %D > | consumes another one. Why is this "wrong"? > > The repetition oan information that is already there. Any scheme that > requires repetition of information is wrong. Well, this is not consistent with what you have said before, and I wonder why you didn't object to %J when it was originally added. We have %J...%D all over the C front end these days. zw ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) 2004-02-10 4:27 ` Gabriel Dos Reis 2004-02-10 4:31 ` Zack Weinberg @ 2004-02-21 13:45 ` Gabriel Dos Reis 1 sibling, 0 replies; 46+ messages in thread From: Gabriel Dos Reis @ 2004-02-21 13:45 UTC (permalink / raw) To: Zack Weinberg; +Cc: Giovanni Bajo, gcc-patches "Zack Weinberg" <zack@codesourcery.com> writes: | Gabriel Dos Reis <gdr@integrable-solutions.net> writes: | | > "Zack Weinberg" <zack@codesourcery.com> writes: | > | > | Gabriel Dos Reis <gdr@integrable-solutions.net> writes: | > | | > | > "Giovanni Bajo" <giovannibajo@libero.it> writes: | > | > | > | > | if (TREE_CODE (decl) == VAR_DECL) | > | > | ! error ("%Jcannot declare variable `%D' to be of abstract type `%T'", | > | > | ! decl, decl, type); | > | > | > | > That is an abuse of %J. Use error_at instead. | > | [etc] | > | | > | I told Giovanni that error/warning/pedwarn_at were to be eliminated in | > | favor of %J. Did I misunderstand what you've said about these in the | > | past? | > | > I don't know exactly what you're referring to. But, the repetition of | > the "decl" is not right and if any removal should lead to that, then | > that removal should not happen. | | I don't understand this. The %J consumes one argument and the %D | consumes another one. Why is this "wrong"? The repetition oan information that is already there. Any scheme that requires repetition of information is wrong. | > error ("%Jdeclaration '%D' is nonsensical", decl, decl); // WRONG | > error_at ("declaration '%+D' is nonsensical", decl); // Good | | The %+D notation is tidier, but I had understood you to be of the | opinion that it was not worth the implementation mess, nor the baggage | of carrying around two different sets of diagnostic functions. If you have error() recognizing '%+' properly then error_at() should go. The whole purpose of xxx_at() begin there is that xxx() does not support yet '%+'. -- Gaby ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) 2004-02-10 3:43 ` Zack Weinberg 2004-02-10 4:27 ` Gabriel Dos Reis @ 2004-02-21 13:45 ` Zack Weinberg 1 sibling, 0 replies; 46+ messages in thread From: Zack Weinberg @ 2004-02-21 13:45 UTC (permalink / raw) To: Gabriel Dos Reis; +Cc: Giovanni Bajo, gcc-patches Gabriel Dos Reis <gdr@integrable-solutions.net> writes: > "Zack Weinberg" <zack@codesourcery.com> writes: > > | Gabriel Dos Reis <gdr@integrable-solutions.net> writes: > | > | > "Giovanni Bajo" <giovannibajo@libero.it> writes: > | > > | > | if (TREE_CODE (decl) == VAR_DECL) > | > | ! error ("%Jcannot declare variable `%D' to be of abstract type `%T'", > | > | ! decl, decl, type); > | > > | > That is an abuse of %J. Use error_at instead. > | [etc] > | > | I told Giovanni that error/warning/pedwarn_at were to be eliminated in > | favor of %J. Did I misunderstand what you've said about these in the > | past? > > I don't know exactly what you're referring to. But, the repetition of > the "decl" is not right and if any removal should lead to that, then > that removal should not happen. I don't understand this. The %J consumes one argument and the %D consumes another one. Why is this "wrong"? > error ("%Jdeclaration '%D' is nonsensical", decl, decl); // WRONG > error_at ("declaration '%+D' is nonsensical", decl); // Good The %+D notation is tidier, but I had understood you to be of the opinion that it was not worth the implementation mess, nor the baggage of carrying around two different sets of diagnostic functions. (If we ever get Chiaki's indexed-parameters-notation patch laid down, it will also allow implementing %+x cleanly and for all languages.) zw ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) 2004-02-10 2:50 ` Gabriel Dos Reis 2004-02-10 3:43 ` Zack Weinberg @ 2004-02-21 13:45 ` Gabriel Dos Reis 1 sibling, 0 replies; 46+ messages in thread From: Gabriel Dos Reis @ 2004-02-21 13:45 UTC (permalink / raw) To: Zack Weinberg; +Cc: Giovanni Bajo, gcc-patches "Zack Weinberg" <zack@codesourcery.com> writes: | Gabriel Dos Reis <gdr@integrable-solutions.net> writes: | | > "Giovanni Bajo" <giovannibajo@libero.it> writes: | > | > | if (TREE_CODE (decl) == VAR_DECL) | > | ! error ("%Jcannot declare variable `%D' to be of abstract type `%T'", | > | ! decl, decl, type); | > | > That is an abuse of %J. Use error_at instead. | [etc] | | I told Giovanni that error/warning/pedwarn_at were to be eliminated in | favor of %J. Did I misunderstand what you've said about these in the | past? I don't know exactly what you're referring to. But, the repetition of the "decl" is not right and if any removal should lead to that, then that removal should not happen. There are some uses of error_at which are wrong. E.g. error_at ("foo is nonsensical", decl); // WRONG error ("%Jfoo is nonsensical", decl); // Good error ("%Jdeclaration '%D' is nonsensical", decl, decl); // WRONG error_at ("declaration '%+D' is nonsensical", decl); // Good -- Gaby ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) 2004-02-10 2:47 ` Zack Weinberg 2004-02-10 2:50 ` Gabriel Dos Reis @ 2004-02-21 13:45 ` Zack Weinberg 1 sibling, 0 replies; 46+ messages in thread From: Zack Weinberg @ 2004-02-21 13:45 UTC (permalink / raw) To: Gabriel Dos Reis; +Cc: Giovanni Bajo, gcc-patches Gabriel Dos Reis <gdr@integrable-solutions.net> writes: > "Giovanni Bajo" <giovannibajo@libero.it> writes: > > | if (TREE_CODE (decl) == VAR_DECL) > | ! error ("%Jcannot declare variable `%D' to be of abstract type `%T'", > | ! decl, decl, type); > > That is an abuse of %J. Use error_at instead. [etc] I told Giovanni that error/warning/pedwarn_at were to be eliminated in favor of %J. Did I misunderstand what you've said about these in the past? zw ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) 2004-02-10 2:40 ` Gabriel Dos Reis 2004-02-10 2:47 ` Zack Weinberg @ 2004-02-10 3:12 ` Giovanni Bajo 2004-02-10 3:14 ` Gabriel Dos Reis 2004-02-21 13:45 ` Giovanni Bajo 2004-02-21 13:45 ` Gabriel Dos Reis 2 siblings, 2 replies; 46+ messages in thread From: Giovanni Bajo @ 2004-02-10 3:12 UTC (permalink / raw) To: Gabriel Dos Reis; +Cc: gcc-patches Gabriel Dos Reis wrote: > As a side comment, I don't understand why you went through that > change. This is the rationale for my changes to abstract_virtual_errors. * Use of %J (will be error_at): With my patch, abstract_virtual_errors can be deferred to an indefinite moment (when the type is completed). Without this change, the error message appears on the last line of the completed type, which is wrong: we need to show the user where is the parameter whose type is abstract. Then, the other messages about the pure virtual functions will point her to the functions which need to be overridden, and thus to the type. * Rewording There is no such thing as an "abstract virtual function". Standard calls it "pure virtual function". * Use of inform vs error Those messages are just additional information provided to help the user, they are not part of the error message proper. Moreover, I heard someone wanted to have a -fno-notes mode one day, so it makes sense to be able to shut those off. This is also why I added the word "abstract" on the first error line, so that it is now a complete error message, and the notes are really optional. Before, the first line would have been unclear, were the notes disabled for some reason. Sorry, I should have mentioned these things in the patch message. Giovanni Bajo ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) 2004-02-10 3:12 ` Giovanni Bajo @ 2004-02-10 3:14 ` Gabriel Dos Reis 2004-02-10 17:50 ` [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) - Take 2 Giovanni Bajo 2004-02-21 13:45 ` [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) Gabriel Dos Reis 2004-02-21 13:45 ` Giovanni Bajo 1 sibling, 2 replies; 46+ messages in thread From: Gabriel Dos Reis @ 2004-02-10 3:14 UTC (permalink / raw) To: Giovanni Bajo; +Cc: gcc-patches "Giovanni Bajo" <giovannibajo@libero.it> writes: | Gabriel Dos Reis wrote: | | > As a side comment, I don't understand why you went through that | > change. | | This is the rationale for my changes to abstract_virtual_errors. | | * Use of %J (will be error_at): | With my patch, abstract_virtual_errors can be deferred to an indefinite moment | (when the type is completed). Without this change, the error message appears on | the last line of the completed type, which is wrong: we need to show the user | where is the parameter whose type is abstract. Then, the other messages about | the pure virtual functions will point her to the functions which need to be | overridden, and thus to the type. That makes sense. | * Rewording | There is no such thing as an "abstract virtual function". Standard calls it | "pure virtual function". I agree too. I've been seeing those messages these last days (personal experience forgetting to define dozens of pure virtual functions in a very complex class hierarchy). They look indeed indeed very odd. | * Use of inform vs error | Those messages are just additional information provided to help the user, they | are not part of the error message proper. Moreover, I heard someone wanted to | have a -fno-notes mode one day, so it makes sense to be able to shut those off. | This is also why I added the word "abstract" on the first error line, so that | it is now a complete error message, and the notes are really optional. Before, | the first line would have been unclear, were the notes disabled for some | reason. This makes sense. | Sorry, I should have mentioned these things in the patch message. No need to sorry. What I don't understand is the use of '%J' in lieu of xxx_at. Thanks, -- Gaby ^ permalink raw reply [flat|nested] 46+ messages in thread
* [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) - Take 2 2004-02-10 3:14 ` Gabriel Dos Reis @ 2004-02-10 17:50 ` Giovanni Bajo 2004-02-10 19:24 ` Gabriel Dos Reis ` (2 more replies) 2004-02-21 13:45 ` [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) Gabriel Dos Reis 1 sibling, 3 replies; 46+ messages in thread From: Giovanni Bajo @ 2004-02-10 17:50 UTC (permalink / raw) To: Gabriel Dos Reis; +Cc: gcc-patches Gabriel Dos Reis wrote: > What I don't understand is the use of '%J' in lieu of xxx_at. This is a revised patch, which uses the form suggested by you. It also now emits the first note line ("because the following virtual..." / "since it has pure...") on the declaration of the type which is found to be abstract, which can be a good reference for the user: in fact, the pure virtual function declarations could be in a differnt point, many files away from the type which the user is trying to instantiate. I can happily revert back to %J if it turns out to be better for some reason. Actually, everybody can in a few seconds, and I reckon the discussion about it is orthogonal to my patch. Retested on i686-pc-linux-gnu with no new regressions. OK for mainline? ---------- quote ----------- Alas, this solution is partial, mainly because it doesn't get into account array types. In fact, parameters with array type are decayed into pointer types within grokdeclarator (even before a PARM_DECL is built), so they never get registered into the incomplete_vars list. A solution for this might be extracting the standard decays out of grokdeclarator, and having grokparms call it. There is also another user of grokdeclarator(PARM) (in pt.c, to build template parameters), which should be updated as well. Comments on this plan are highly appreciated. Another case we don't handle yet is when the type of the parameter is dependent. In that case, we probably need to check for abstractness upon instantiation, while tsubsting. I'll investigate. ---------- end quote ----------- Any comments on this? Thanks, Giovanni Bajo 2004-02-10 Giovanni Bajo <giovannibajo@gcc.gnu.org> PR c++/2204 * decl.c (cp_finish_decl): Extract logic to strip array and pointer to array types into... (strip_array_and_pointer_to_array_types): New function. (grokparms): Register PARM_DECLs of incomplete types for later check, using... (maybe_register_incomplete_parms): New function. (complete_vars): When completing the type for a PARM_DECL, check for abstractness. Fix a typo in the comment. * typeck2.c (abstract_virtual_errors): Reword diagnostics, make them appear at the correct location. 2004-02-10 Giovanni Bajo <giovannibajo@gcc.gnu.org> PR c++/2204 * g++.dg/other/abstract2.C: New test. * g++.old-deja/g++.robertl/eb4.C: Adjust error markers. * g++.old-deja/g++.other/decl3.C: Likewise. Index: cp/decl.c =================================================================== RCS file: /cvs/gcc/gcc/gcc/cp/decl.c,v retrieving revision 1.1179 diff -c -3 -p -r1.1179 decl.c *** cp/decl.c 2 Feb 2004 16:53:02 -0000 1.1179 --- cp/decl.c 10 Feb 2004 17:10:04 -0000 *************** static void expand_static_init (tree, tr *** 120,125 **** --- 120,127 ---- static tree next_initializable_field (tree); static tree reshape_init (tree, tree *); static tree build_typename_type (tree, tree, tree); + static void maybe_register_incomplete_parm (tree); + static tree strip_array_and_pointer_to_array_types (tree); /* Erroneous argument lists can use this *IFF* they do not modify it. */ tree error_mark_list; *************** cp_finish_decl (tree decl, tree init, tr *** 4883,4901 **** || TREE_CODE (type) == METHOD_TYPE) abstract_virtuals_error (decl, strip_array_types (TREE_TYPE (type))); ! else if (POINTER_TYPE_P (type) || TREE_CODE (type) == ARRAY_TYPE) ! { ! /* If it's either a pointer or an array type, strip through all ! of them but the last one. If the last is an array type, issue ! an error if the element type is abstract. */ ! while (POINTER_TYPE_P (TREE_TYPE (type)) ! || TREE_CODE (TREE_TYPE (type)) == ARRAY_TYPE) ! type = TREE_TYPE (type); ! if (TREE_CODE (type) == ARRAY_TYPE) ! abstract_virtuals_error (decl, TREE_TYPE (type)); ! } ! else ! abstract_virtuals_error (decl, type); if (TREE_CODE (decl) == FUNCTION_DECL || TREE_TYPE (decl) == error_mark_node) --- 4885,4894 ---- || TREE_CODE (type) == METHOD_TYPE) abstract_virtuals_error (decl, strip_array_types (TREE_TYPE (type))); ! else ! abstract_virtuals_error (decl, ! strip_array_and_pointer_to_array_types ! (type)); if (TREE_CODE (decl) == FUNCTION_DECL || TREE_TYPE (decl) == error_mark_node) *************** grokparms (tree first_parm) *** 8669,8674 **** --- 8660,8667 ---- type = build_pointer_type (type); TREE_TYPE (decl) = type; } + else if (!COMPLETE_TYPE_P (type) || currently_open_class (type)) + maybe_register_incomplete_parm (decl); else if (abstract_virtuals_error (decl, type)) any_error = 1; /* Seems like a good idea. */ else if (POINTER_TYPE_P (type)) *************** finish_method (tree decl) *** 11049,11055 **** return decl; } ! \f /* VAR is a VAR_DECL. If its type is incomplete, remember VAR so that we can lay it out later, when and if its type becomes complete. */ --- 11042,11098 ---- return decl; } ! ! /* Strip all the array/pointer-to-array types from TYPE. This is used to ! extract the underlying type from declarations such as: ! ! A a[2]; ! A (*a)[2]; ! A (**a[2])[2]; ! ! which are invalid if 'A' is an abstract type. */ ! ! static tree ! strip_array_and_pointer_to_array_types (tree type) ! { ! if (POINTER_TYPE_P (type) || TREE_CODE (type) == ARRAY_TYPE) ! { ! /* If it's either a pointer or an array type, strip through all ! of them but the last one. If the last is an array type, then ! we can return its element type. */ ! while (POINTER_TYPE_P (TREE_TYPE (type)) ! || TREE_CODE (TREE_TYPE (type)) == ARRAY_TYPE) ! type = TREE_TYPE (type); ! if (TREE_CODE (type) == ARRAY_TYPE) ! return TREE_TYPE (type); ! } ! return type; ! } ! ! /* PARM is a PARM_DECL, whose type is incomplete at the point of parsing. ! We keep track of these because we must issue an error if, upon ! completition, the type ends up being abstract. */ ! ! static void ! maybe_register_incomplete_parm (tree var) ! { ! my_friendly_assert (TREE_CODE (var) == PARM_DECL, 20040208); ! ! /* If the type is dependent, there is nothing we can do here. The ! diagnostic will have to be issued during template substitution. */ ! if (!dependent_type_p (TREE_TYPE (var))) ! { ! tree inner_type = TREE_TYPE (var); ! ! /* FIXME: We currently don't get any ARRAY_TYPE because grokdeclarator ! decays them to POINTER_TYPE too early. */ ! inner_type = strip_array_and_pointer_to_array_types (inner_type); ! inner_type = TYPE_MAIN_VARIANT (inner_type); ! ! if (!COMPLETE_TYPE_P (inner_type) && CLASS_TYPE_P (inner_type)) ! incomplete_vars = tree_cons (inner_type, var, incomplete_vars); ! } ! } /* VAR is a VAR_DECL. If its type is incomplete, remember VAR so that we can lay it out later, when and if its type becomes complete. */ *************** maybe_register_incomplete_var (tree var) *** 11064,11074 **** && DECL_EXTERNAL (var)) { tree inner_type = TREE_TYPE (var); ! while (TREE_CODE (inner_type) == ARRAY_TYPE) inner_type = TREE_TYPE (inner_type); inner_type = TYPE_MAIN_VARIANT (inner_type); ! if ((!COMPLETE_TYPE_P (inner_type) && CLASS_TYPE_P (inner_type)) /* RTTI TD entries are created while defining the type_info. */ || (TYPE_LANG_SPECIFIC (inner_type) --- 11107,11117 ---- && DECL_EXTERNAL (var)) { tree inner_type = TREE_TYPE (var); ! while (TREE_CODE (inner_type) == ARRAY_TYPE) inner_type = TREE_TYPE (inner_type); inner_type = TYPE_MAIN_VARIANT (inner_type); ! if ((!COMPLETE_TYPE_P (inner_type) && CLASS_TYPE_P (inner_type)) /* RTTI TD entries are created while defining the type_info. */ || (TYPE_LANG_SPECIFIC (inner_type) *************** maybe_register_incomplete_var (tree var) *** 11078,11084 **** } /* Called when a class type (given by TYPE) is defined. If there are ! any existing VAR_DECLs whose type hsa been completed by this declaration, update them now. */ void --- 11121,11127 ---- } /* Called when a class type (given by TYPE) is defined. If there are ! any existing VAR_DECLs whose type has been completed by this declaration, update them now. */ void *************** complete_vars (tree type) *** 11095,11100 **** --- 11138,11148 ---- /* Complete the type of the variable. The VAR_DECL itself will be laid out in expand_expr. */ complete_type (TREE_TYPE (var)); + /* If it is a PARM_DECL, we need to emit an error if the + type is abstract, because this is why we registered it + here. */ + if (TREE_CODE (var) == PARM_DECL) + abstract_virtuals_error (var, TREE_TYPE (var)); /* Remove this entry from the list. */ *list = TREE_CHAIN (*list); } Index: cp/typeck2.c =================================================================== RCS file: /cvs/gcc/gcc/gcc/cp/typeck2.c,v retrieving revision 1.154 diff -c -3 -p -r1.154 typeck2.c *** cp/typeck2.c 22 Jan 2004 00:03:52 -0000 1.154 --- cp/typeck2.c 10 Feb 2004 17:10:07 -0000 *************** abstract_virtuals_error (tree decl, tree *** 149,182 **** return 0; if (TREE_CODE (decl) == VAR_DECL) ! error ("cannot declare variable `%D' to be of type `%T'", ! decl, type); else if (TREE_CODE (decl) == PARM_DECL) ! error ("cannot declare parameter `%D' to be of type `%T'", ! decl, type); else if (TREE_CODE (decl) == FIELD_DECL) ! error ("cannot declare field `%D' to be of type `%T'", ! decl, type); else if (TREE_CODE (decl) == FUNCTION_DECL && TREE_CODE (TREE_TYPE (decl)) == METHOD_TYPE) ! error ("invalid return type for member function `%#D'", decl); else if (TREE_CODE (decl) == FUNCTION_DECL) ! error ("invalid return type for function `%#D'", decl); } else ! error ("cannot allocate an object of type `%T'", type); /* Only go through this once. */ if (TREE_PURPOSE (u) == NULL_TREE) { TREE_PURPOSE (u) = error_mark_node; ! error (" because the following virtual functions are abstract:"); for (tu = u; tu; tu = TREE_CHAIN (tu)) ! cp_error_at ("\t%#D", TREE_VALUE (tu)); } else ! error (" since type `%T' has abstract virtual functions", type); return 1; } --- 149,187 ---- return 0; if (TREE_CODE (decl) == VAR_DECL) ! cp_error_at ("cannot declare variable `%+D' to be of abstract " ! "type `%T'", decl, type); else if (TREE_CODE (decl) == PARM_DECL) ! cp_error_at ("cannot declare parameter `%+D' to be of abstract " ! "type `%T'", decl, type); else if (TREE_CODE (decl) == FIELD_DECL) ! cp_error_at ("cannot declare field `%+D' to be of abstract " ! "type `%T'", decl, type); else if (TREE_CODE (decl) == FUNCTION_DECL && TREE_CODE (TREE_TYPE (decl)) == METHOD_TYPE) ! cp_error_at ("invalid abstract return type for member function `%+#D'", ! decl); else if (TREE_CODE (decl) == FUNCTION_DECL) ! cp_error_at ("invalid abstract return type for function `%+#D'", ! decl); } else ! error ("cannot allocate an object of abstract type `%T'", type); /* Only go through this once. */ if (TREE_PURPOSE (u) == NULL_TREE) { TREE_PURPOSE (u) = error_mark_node; ! inform ("%J because the following virtual functions are pure " ! "within `%T':", TYPE_MAIN_DECL (type), type); ! for (tu = u; tu; tu = TREE_CHAIN (tu)) ! inform ("%J\t%#D", TREE_VALUE (tu), TREE_VALUE (tu)); } else ! inform ("%J since type `%T' has pure virtual functions", ! TYPE_MAIN_DECL (type), type); return 1; } Index: testsuite/g++.old-deja/g++.other/decl3.C =================================================================== RCS file: /cvs/gcc/gcc/gcc/testsuite/g++.old-deja/g++.other/decl3.C,v retrieving revision 1.4 diff -c -3 -p -r1.4 decl3.C *** testsuite/g++.old-deja/g++.other/decl3.C 1 May 2003 02:02:49 -0000 1.4 --- testsuite/g++.old-deja/g++.other/decl3.C 10 Feb 2004 17:10:07 -0000 *************** *** 6,13 **** // We should not allow arrays of abstract type. [class.abstract/2] ! struct cow_t { ! virtual void f()=0; // { dg-error "" } abstract }; --- 6,13 ---- // We should not allow arrays of abstract type. [class.abstract/2] ! struct cow_t { // { dg-error "" } note ! virtual void f()=0; // { dg-error "" } pure }; Index: testsuite/g++.old-deja/g++.robertl/eb4.C =================================================================== RCS file: /cvs/gcc/gcc/gcc/testsuite/g++.old-deja/g++.robertl/eb4.C,v retrieving revision 1.6 diff -c -3 -p -r1.6 eb4.C *** testsuite/g++.old-deja/g++.robertl/eb4.C 1 May 2003 02:02:58 -0000 1.6 --- testsuite/g++.old-deja/g++.robertl/eb4.C 10 Feb 2004 17:10:07 -0000 *************** public: *** 17,23 **** }; class some_derived : public some_base ! { public: class derived_func_args; void func(derived_func_args &); --- 17,23 ---- }; class some_derived : public some_base ! { // { dg-error "" } note public: class derived_func_args; void func(derived_func_args &); // { dg-do compile } // Contributed by Gabriel Dos Reis <gdr at integrable-solutions dot net> // PR c++/2204: Check for parameters of abstract type in function declarations. namespace N1 { struct X; struct Y1 { void g(X parm1); // { dg-error "abstract" } void g(X parm2[2]); // { dg-error "abstract" "" { xfail *-*-* } } }; template <int N> struct Y2 { void g(X parm3); // { dg-error "abstract" } void g(X parm4[2]); // { dg-error "abstract" "" { xfail *-*-* } } }; struct X { // { dg-error "note" } virtual void xfunc(void) = 0; // { dg-error "note" } }; } namespace N2 { struct X1 { // { dg-error "note" } virtual void xfunc(void) = 0; // { dg-error "note" } void g(X1 parm5); // { dg-error "abstract" } void g(X1 parm6[2]); // { dg-error "abstract" "" { xfail *-*-* } } }; template <int N> struct X2 { // { dg-error "note" "" { xfail *-*-* } } virtual void xfunc(void) = 0; // { dg-error "note" "" { xfail *-*-* } } void g(X2 parm7); // { dg-error "abstract" "" { xfail *-*-* } } void g(X2 parm8[2]); // { dg-error "abstract" "" { xfail *-*-* } } }; } namespace N3 { struct X { // { dg-error "note" "" } virtual void xfunc(void) = 0; // { dg-error "note" } }; void g(X parm9); // { dg-error "abstract" } void g(X parm10[2]); // { dg-error "abstract" "" { xfail *-*-* } } template <int N> void g(X parm11); // { dg-error "abstract" } template <int N> void g(X parm12[2]); // { dg-error "abstract" "" { xfail *-*-* } } } ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) - Take 2 2004-02-10 17:50 ` [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) - Take 2 Giovanni Bajo @ 2004-02-10 19:24 ` Gabriel Dos Reis 2004-02-21 13:45 ` Gabriel Dos Reis 2004-02-21 13:45 ` Giovanni Bajo 2004-04-30 21:56 ` Jason Merrill 2 siblings, 1 reply; 46+ messages in thread From: Gabriel Dos Reis @ 2004-02-10 19:24 UTC (permalink / raw) To: Giovanni Bajo; +Cc: gcc-patches "Giovanni Bajo" <giovannibajo@libero.it> writes: | Gabriel Dos Reis wrote: | | > What I don't understand is the use of '%J' in lieu of xxx_at. | | This is a revised patch, which uses the form suggested by you. It also now | emits the first note line ("because the following virtual..." / "since it has | pure...") on the declaration of the type which is found to be abstract, which | can be a good reference for the user: in fact, the pure virtual function | declarations could be in a differnt point, many files away from the type which | the user is trying to instantiate. I totally agree (at least, that has been my case these last days). | I can happily revert back to %J if it turns out to be better for some reason. | Actually, everybody can in a few seconds, and I reckon the discussion about it | is orthogonal to my patch. | | Retested on i686-pc-linux-gnu with no new regressions. OK for mainline? The diagnostic part is OK with me (I think that the rest also makes sense). However, you'll need Jason or Mark's approval for the rest. | ---------- quote ----------- | Alas, this solution is partial, mainly because it doesn't get into account | array types. In fact, parameters with array type are decayed into pointer types | within grokdeclarator (even before a PARM_DECL is built), so they never get | registered into the incomplete_vars list. A solution for this might be | extracting the standard decays out of grokdeclarator, and having grokparms call | it. There is also another user of grokdeclarator(PARM) (in pt.c, to build | template parameters), which should be updated as well. Comments on this plan | are highly appreciated. | | Another case we don't handle yet is when the type of the parameter is | dependent. In that case, we probably need to check for abstractness upon | instantiation, while tsubsting. I'll investigate. | ---------- end quote ----------- | Any comments on this? I think I would have gone with Jason's suggestion -- putting those parameter types in a separate list -- but your partial solution does no harm I suppose. -- Gaby ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) - Take 2 2004-02-10 19:24 ` Gabriel Dos Reis @ 2004-02-21 13:45 ` Gabriel Dos Reis 0 siblings, 0 replies; 46+ messages in thread From: Gabriel Dos Reis @ 2004-02-21 13:45 UTC (permalink / raw) To: Giovanni Bajo; +Cc: gcc-patches "Giovanni Bajo" <giovannibajo@libero.it> writes: | Gabriel Dos Reis wrote: | | > What I don't understand is the use of '%J' in lieu of xxx_at. | | This is a revised patch, which uses the form suggested by you. It also now | emits the first note line ("because the following virtual..." / "since it has | pure...") on the declaration of the type which is found to be abstract, which | can be a good reference for the user: in fact, the pure virtual function | declarations could be in a differnt point, many files away from the type which | the user is trying to instantiate. I totally agree (at least, that has been my case these last days). | I can happily revert back to %J if it turns out to be better for some reason. | Actually, everybody can in a few seconds, and I reckon the discussion about it | is orthogonal to my patch. | | Retested on i686-pc-linux-gnu with no new regressions. OK for mainline? The diagnostic part is OK with me (I think that the rest also makes sense). However, you'll need Jason or Mark's approval for the rest. | ---------- quote ----------- | Alas, this solution is partial, mainly because it doesn't get into account | array types. In fact, parameters with array type are decayed into pointer types | within grokdeclarator (even before a PARM_DECL is built), so they never get | registered into the incomplete_vars list. A solution for this might be | extracting the standard decays out of grokdeclarator, and having grokparms call | it. There is also another user of grokdeclarator(PARM) (in pt.c, to build | template parameters), which should be updated as well. Comments on this plan | are highly appreciated. | | Another case we don't handle yet is when the type of the parameter is | dependent. In that case, we probably need to check for abstractness upon | instantiation, while tsubsting. I'll investigate. | ---------- end quote ----------- | Any comments on this? I think I would have gone with Jason's suggestion -- putting those parameter types in a separate list -- but your partial solution does no harm I suppose. -- Gaby ^ permalink raw reply [flat|nested] 46+ messages in thread
* [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) - Take 2 2004-02-10 17:50 ` [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) - Take 2 Giovanni Bajo 2004-02-10 19:24 ` Gabriel Dos Reis @ 2004-02-21 13:45 ` Giovanni Bajo 2004-04-30 21:56 ` Jason Merrill 2 siblings, 0 replies; 46+ messages in thread From: Giovanni Bajo @ 2004-02-21 13:45 UTC (permalink / raw) To: Gabriel Dos Reis; +Cc: gcc-patches Gabriel Dos Reis wrote: > What I don't understand is the use of '%J' in lieu of xxx_at. This is a revised patch, which uses the form suggested by you. It also now emits the first note line ("because the following virtual..." / "since it has pure...") on the declaration of the type which is found to be abstract, which can be a good reference for the user: in fact, the pure virtual function declarations could be in a differnt point, many files away from the type which the user is trying to instantiate. I can happily revert back to %J if it turns out to be better for some reason. Actually, everybody can in a few seconds, and I reckon the discussion about it is orthogonal to my patch. Retested on i686-pc-linux-gnu with no new regressions. OK for mainline? ---------- quote ----------- Alas, this solution is partial, mainly because it doesn't get into account array types. In fact, parameters with array type are decayed into pointer types within grokdeclarator (even before a PARM_DECL is built), so they never get registered into the incomplete_vars list. A solution for this might be extracting the standard decays out of grokdeclarator, and having grokparms call it. There is also another user of grokdeclarator(PARM) (in pt.c, to build template parameters), which should be updated as well. Comments on this plan are highly appreciated. Another case we don't handle yet is when the type of the parameter is dependent. In that case, we probably need to check for abstractness upon instantiation, while tsubsting. I'll investigate. ---------- end quote ----------- Any comments on this? Thanks, Giovanni Bajo 2004-02-10 Giovanni Bajo <giovannibajo@gcc.gnu.org> PR c++/2204 * decl.c (cp_finish_decl): Extract logic to strip array and pointer to array types into... (strip_array_and_pointer_to_array_types): New function. (grokparms): Register PARM_DECLs of incomplete types for later check, using... (maybe_register_incomplete_parms): New function. (complete_vars): When completing the type for a PARM_DECL, check for abstractness. Fix a typo in the comment. * typeck2.c (abstract_virtual_errors): Reword diagnostics, make them appear at the correct location. 2004-02-10 Giovanni Bajo <giovannibajo@gcc.gnu.org> PR c++/2204 * g++.dg/other/abstract2.C: New test. * g++.old-deja/g++.robertl/eb4.C: Adjust error markers. * g++.old-deja/g++.other/decl3.C: Likewise. Index: cp/decl.c =================================================================== RCS file: /cvs/gcc/gcc/gcc/cp/decl.c,v retrieving revision 1.1179 diff -c -3 -p -r1.1179 decl.c *** cp/decl.c 2 Feb 2004 16:53:02 -0000 1.1179 --- cp/decl.c 10 Feb 2004 17:10:04 -0000 *************** static void expand_static_init (tree, tr *** 120,125 **** --- 120,127 ---- static tree next_initializable_field (tree); static tree reshape_init (tree, tree *); static tree build_typename_type (tree, tree, tree); + static void maybe_register_incomplete_parm (tree); + static tree strip_array_and_pointer_to_array_types (tree); /* Erroneous argument lists can use this *IFF* they do not modify it. */ tree error_mark_list; *************** cp_finish_decl (tree decl, tree init, tr *** 4883,4901 **** || TREE_CODE (type) == METHOD_TYPE) abstract_virtuals_error (decl, strip_array_types (TREE_TYPE (type))); ! else if (POINTER_TYPE_P (type) || TREE_CODE (type) == ARRAY_TYPE) ! { ! /* If it's either a pointer or an array type, strip through all ! of them but the last one. If the last is an array type, issue ! an error if the element type is abstract. */ ! while (POINTER_TYPE_P (TREE_TYPE (type)) ! || TREE_CODE (TREE_TYPE (type)) == ARRAY_TYPE) ! type = TREE_TYPE (type); ! if (TREE_CODE (type) == ARRAY_TYPE) ! abstract_virtuals_error (decl, TREE_TYPE (type)); ! } ! else ! abstract_virtuals_error (decl, type); if (TREE_CODE (decl) == FUNCTION_DECL || TREE_TYPE (decl) == error_mark_node) --- 4885,4894 ---- || TREE_CODE (type) == METHOD_TYPE) abstract_virtuals_error (decl, strip_array_types (TREE_TYPE (type))); ! else ! abstract_virtuals_error (decl, ! strip_array_and_pointer_to_array_types ! (type)); if (TREE_CODE (decl) == FUNCTION_DECL || TREE_TYPE (decl) == error_mark_node) *************** grokparms (tree first_parm) *** 8669,8674 **** --- 8660,8667 ---- type = build_pointer_type (type); TREE_TYPE (decl) = type; } + else if (!COMPLETE_TYPE_P (type) || currently_open_class (type)) + maybe_register_incomplete_parm (decl); else if (abstract_virtuals_error (decl, type)) any_error = 1; /* Seems like a good idea. */ else if (POINTER_TYPE_P (type)) *************** finish_method (tree decl) *** 11049,11055 **** return decl; } ! \f /* VAR is a VAR_DECL. If its type is incomplete, remember VAR so that we can lay it out later, when and if its type becomes complete. */ --- 11042,11098 ---- return decl; } ! ! /* Strip all the array/pointer-to-array types from TYPE. This is used to ! extract the underlying type from declarations such as: ! ! A a[2]; ! A (*a)[2]; ! A (**a[2])[2]; ! ! which are invalid if 'A' is an abstract type. */ ! ! static tree ! strip_array_and_pointer_to_array_types (tree type) ! { ! if (POINTER_TYPE_P (type) || TREE_CODE (type) == ARRAY_TYPE) ! { ! /* If it's either a pointer or an array type, strip through all ! of them but the last one. If the last is an array type, then ! we can return its element type. */ ! while (POINTER_TYPE_P (TREE_TYPE (type)) ! || TREE_CODE (TREE_TYPE (type)) == ARRAY_TYPE) ! type = TREE_TYPE (type); ! if (TREE_CODE (type) == ARRAY_TYPE) ! return TREE_TYPE (type); ! } ! return type; ! } ! ! /* PARM is a PARM_DECL, whose type is incomplete at the point of parsing. ! We keep track of these because we must issue an error if, upon ! completition, the type ends up being abstract. */ ! ! static void ! maybe_register_incomplete_parm (tree var) ! { ! my_friendly_assert (TREE_CODE (var) == PARM_DECL, 20040208); ! ! /* If the type is dependent, there is nothing we can do here. The ! diagnostic will have to be issued during template substitution. */ ! if (!dependent_type_p (TREE_TYPE (var))) ! { ! tree inner_type = TREE_TYPE (var); ! ! /* FIXME: We currently don't get any ARRAY_TYPE because grokdeclarator ! decays them to POINTER_TYPE too early. */ ! inner_type = strip_array_and_pointer_to_array_types (inner_type); ! inner_type = TYPE_MAIN_VARIANT (inner_type); ! ! if (!COMPLETE_TYPE_P (inner_type) && CLASS_TYPE_P (inner_type)) ! incomplete_vars = tree_cons (inner_type, var, incomplete_vars); ! } ! } /* VAR is a VAR_DECL. If its type is incomplete, remember VAR so that we can lay it out later, when and if its type becomes complete. */ *************** maybe_register_incomplete_var (tree var) *** 11064,11074 **** && DECL_EXTERNAL (var)) { tree inner_type = TREE_TYPE (var); ! while (TREE_CODE (inner_type) == ARRAY_TYPE) inner_type = TREE_TYPE (inner_type); inner_type = TYPE_MAIN_VARIANT (inner_type); ! if ((!COMPLETE_TYPE_P (inner_type) && CLASS_TYPE_P (inner_type)) /* RTTI TD entries are created while defining the type_info. */ || (TYPE_LANG_SPECIFIC (inner_type) --- 11107,11117 ---- && DECL_EXTERNAL (var)) { tree inner_type = TREE_TYPE (var); ! while (TREE_CODE (inner_type) == ARRAY_TYPE) inner_type = TREE_TYPE (inner_type); inner_type = TYPE_MAIN_VARIANT (inner_type); ! if ((!COMPLETE_TYPE_P (inner_type) && CLASS_TYPE_P (inner_type)) /* RTTI TD entries are created while defining the type_info. */ || (TYPE_LANG_SPECIFIC (inner_type) *************** maybe_register_incomplete_var (tree var) *** 11078,11084 **** } /* Called when a class type (given by TYPE) is defined. If there are ! any existing VAR_DECLs whose type hsa been completed by this declaration, update them now. */ void --- 11121,11127 ---- } /* Called when a class type (given by TYPE) is defined. If there are ! any existing VAR_DECLs whose type has been completed by this declaration, update them now. */ void *************** complete_vars (tree type) *** 11095,11100 **** --- 11138,11148 ---- /* Complete the type of the variable. The VAR_DECL itself will be laid out in expand_expr. */ complete_type (TREE_TYPE (var)); + /* If it is a PARM_DECL, we need to emit an error if the + type is abstract, because this is why we registered it + here. */ + if (TREE_CODE (var) == PARM_DECL) + abstract_virtuals_error (var, TREE_TYPE (var)); /* Remove this entry from the list. */ *list = TREE_CHAIN (*list); } Index: cp/typeck2.c =================================================================== RCS file: /cvs/gcc/gcc/gcc/cp/typeck2.c,v retrieving revision 1.154 diff -c -3 -p -r1.154 typeck2.c *** cp/typeck2.c 22 Jan 2004 00:03:52 -0000 1.154 --- cp/typeck2.c 10 Feb 2004 17:10:07 -0000 *************** abstract_virtuals_error (tree decl, tree *** 149,182 **** return 0; if (TREE_CODE (decl) == VAR_DECL) ! error ("cannot declare variable `%D' to be of type `%T'", ! decl, type); else if (TREE_CODE (decl) == PARM_DECL) ! error ("cannot declare parameter `%D' to be of type `%T'", ! decl, type); else if (TREE_CODE (decl) == FIELD_DECL) ! error ("cannot declare field `%D' to be of type `%T'", ! decl, type); else if (TREE_CODE (decl) == FUNCTION_DECL && TREE_CODE (TREE_TYPE (decl)) == METHOD_TYPE) ! error ("invalid return type for member function `%#D'", decl); else if (TREE_CODE (decl) == FUNCTION_DECL) ! error ("invalid return type for function `%#D'", decl); } else ! error ("cannot allocate an object of type `%T'", type); /* Only go through this once. */ if (TREE_PURPOSE (u) == NULL_TREE) { TREE_PURPOSE (u) = error_mark_node; ! error (" because the following virtual functions are abstract:"); for (tu = u; tu; tu = TREE_CHAIN (tu)) ! cp_error_at ("\t%#D", TREE_VALUE (tu)); } else ! error (" since type `%T' has abstract virtual functions", type); return 1; } --- 149,187 ---- return 0; if (TREE_CODE (decl) == VAR_DECL) ! cp_error_at ("cannot declare variable `%+D' to be of abstract " ! "type `%T'", decl, type); else if (TREE_CODE (decl) == PARM_DECL) ! cp_error_at ("cannot declare parameter `%+D' to be of abstract " ! "type `%T'", decl, type); else if (TREE_CODE (decl) == FIELD_DECL) ! cp_error_at ("cannot declare field `%+D' to be of abstract " ! "type `%T'", decl, type); else if (TREE_CODE (decl) == FUNCTION_DECL && TREE_CODE (TREE_TYPE (decl)) == METHOD_TYPE) ! cp_error_at ("invalid abstract return type for member function `%+#D'", ! decl); else if (TREE_CODE (decl) == FUNCTION_DECL) ! cp_error_at ("invalid abstract return type for function `%+#D'", ! decl); } else ! error ("cannot allocate an object of abstract type `%T'", type); /* Only go through this once. */ if (TREE_PURPOSE (u) == NULL_TREE) { TREE_PURPOSE (u) = error_mark_node; ! inform ("%J because the following virtual functions are pure " ! "within `%T':", TYPE_MAIN_DECL (type), type); ! for (tu = u; tu; tu = TREE_CHAIN (tu)) ! inform ("%J\t%#D", TREE_VALUE (tu), TREE_VALUE (tu)); } else ! inform ("%J since type `%T' has pure virtual functions", ! TYPE_MAIN_DECL (type), type); return 1; } Index: testsuite/g++.old-deja/g++.other/decl3.C =================================================================== RCS file: /cvs/gcc/gcc/gcc/testsuite/g++.old-deja/g++.other/decl3.C,v retrieving revision 1.4 diff -c -3 -p -r1.4 decl3.C *** testsuite/g++.old-deja/g++.other/decl3.C 1 May 2003 02:02:49 -0000 1.4 --- testsuite/g++.old-deja/g++.other/decl3.C 10 Feb 2004 17:10:07 -0000 *************** *** 6,13 **** // We should not allow arrays of abstract type. [class.abstract/2] ! struct cow_t { ! virtual void f()=0; // { dg-error "" } abstract }; --- 6,13 ---- // We should not allow arrays of abstract type. [class.abstract/2] ! struct cow_t { // { dg-error "" } note ! virtual void f()=0; // { dg-error "" } pure }; Index: testsuite/g++.old-deja/g++.robertl/eb4.C =================================================================== RCS file: /cvs/gcc/gcc/gcc/testsuite/g++.old-deja/g++.robertl/eb4.C,v retrieving revision 1.6 diff -c -3 -p -r1.6 eb4.C *** testsuite/g++.old-deja/g++.robertl/eb4.C 1 May 2003 02:02:58 -0000 1.6 --- testsuite/g++.old-deja/g++.robertl/eb4.C 10 Feb 2004 17:10:07 -0000 *************** public: *** 17,23 **** }; class some_derived : public some_base ! { public: class derived_func_args; void func(derived_func_args &); --- 17,23 ---- }; class some_derived : public some_base ! { // { dg-error "" } note public: class derived_func_args; void func(derived_func_args &); // { dg-do compile } // Contributed by Gabriel Dos Reis <gdr at integrable-solutions dot net> // PR c++/2204: Check for parameters of abstract type in function declarations. namespace N1 { struct X; struct Y1 { void g(X parm1); // { dg-error "abstract" } void g(X parm2[2]); // { dg-error "abstract" "" { xfail *-*-* } } }; template <int N> struct Y2 { void g(X parm3); // { dg-error "abstract" } void g(X parm4[2]); // { dg-error "abstract" "" { xfail *-*-* } } }; struct X { // { dg-error "note" } virtual void xfunc(void) = 0; // { dg-error "note" } }; } namespace N2 { struct X1 { // { dg-error "note" } virtual void xfunc(void) = 0; // { dg-error "note" } void g(X1 parm5); // { dg-error "abstract" } void g(X1 parm6[2]); // { dg-error "abstract" "" { xfail *-*-* } } }; template <int N> struct X2 { // { dg-error "note" "" { xfail *-*-* } } virtual void xfunc(void) = 0; // { dg-error "note" "" { xfail *-*-* } } void g(X2 parm7); // { dg-error "abstract" "" { xfail *-*-* } } void g(X2 parm8[2]); // { dg-error "abstract" "" { xfail *-*-* } } }; } namespace N3 { struct X { // { dg-error "note" "" } virtual void xfunc(void) = 0; // { dg-error "note" } }; void g(X parm9); // { dg-error "abstract" } void g(X parm10[2]); // { dg-error "abstract" "" { xfail *-*-* } } template <int N> void g(X parm11); // { dg-error "abstract" } template <int N> void g(X parm12[2]); // { dg-error "abstract" "" { xfail *-*-* } } } ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) - Take 2 2004-02-10 17:50 ` [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) - Take 2 Giovanni Bajo 2004-02-10 19:24 ` Gabriel Dos Reis 2004-02-21 13:45 ` Giovanni Bajo @ 2004-04-30 21:56 ` Jason Merrill 2004-06-11 13:52 ` [C++ PATCH] [PR2204] Check for parameters of abstract types - Take 3 Giovanni Bajo 2 siblings, 1 reply; 46+ messages in thread From: Jason Merrill @ 2004-04-30 21:56 UTC (permalink / raw) To: Giovanni Bajo; +Cc: gcc-patches On Tue, 10 Feb 2004 18:37:31 +0100, "Giovanni Bajo" <giovannibajo@libero.it> wrote: > Alas, this solution is partial, mainly because it doesn't get into account > array types. In fact, parameters with array type are decayed into pointer types > within grokdeclarator (even before a PARM_DECL is built), so they never get > registered into the incomplete_vars list. A solution for this might be > extracting the standard decays out of grokdeclarator, and having grokparms call > it. There is also another user of grokdeclarator(PARM) (in pt.c, to build > template parameters), which should be updated as well. Comments on this plan > are highly appreciated. There's a much simpler solution: just check for abstract types in create_array_type_for_decl. 8.3.4 Arrays [dcl.array] T is called the array element type; this type shall not be a reference type, the (possibly cv-qualified) type void, a function type or an abstract class type. This should also make strip_array_and_pointer_to_array_types unnecessary. > Another case we don't handle yet is when the type of the parameter is > dependent. In that case, we probably need to check for abstractness upon > instantiation, while tsubsting. I'll investigate. Please. Jason ^ permalink raw reply [flat|nested] 46+ messages in thread
* [C++ PATCH] [PR2204] Check for parameters of abstract types - Take 3 2004-04-30 21:56 ` Jason Merrill @ 2004-06-11 13:52 ` Giovanni Bajo 2004-06-13 16:01 ` Mark Mitchell 0 siblings, 1 reply; 46+ messages in thread From: Giovanni Bajo @ 2004-06-11 13:52 UTC (permalink / raw) To: Jason Merrill; +Cc: gcc-patches Jason Merrill wrote: [http://gcc.gnu.org/ml/gcc-patches/2004-02/msg00953.html] >> Alas, this solution is partial, mainly because it doesn't get into >> account >> array types. In fact, parameters with array type are decayed into >> pointer types within grokdeclarator (even before a PARM_DECL is >> built), so they never get registered into the incomplete_vars list. >> A solution for this might be extracting the standard decays out of >> grokdeclarator, and having grokparms call it. There is also another >> user of grokdeclarator(PARM) (in pt.c, to build template >> parameters), which should be updated as well. Comments on this plan >> are highly appreciated. > > There's a much simpler solution: just check for abstract types in > create_array_type_for_decl. I tried this but it occurred to me that if I checked for abstract types in grokdeclarator immediatly after the big loop where the type for the declaration is constructed (and where create_array_type_for_decl is called for the ARRAY_REF case), I could catch both the array and the non-array case at the same time, unifying the check. In fact, it works beautifully, so I removed the other check for abstractness from grokparms. The only quirk is that I need to create a PARM_DECL just for the purpose of storing it in the incomplete_vars list (it cannot be done with the real PARM_DECL because that's already decayed from array to pointer), but it doesn't seem too much of a problem to me. >> Another case we don't handle yet is when the type of the parameter is >> dependent. In that case, we probably need to check for abstractness >> upon instantiation, while tsubsting. I'll investigate. > > Please. I added support for simple cases, where the dependent type contains pure virtual functions and it is used as a type for parameters. In this case, I am emitting an error before instantiation (as soon as the type is completely defined). I still miss cases where the pure virtuals come from base classes or specializations of the template (that is, non trivial cases). In this situation, something will have to be done at instantiation time, but I will leave this as a followup. The hunk in abstract_virtual_errors is needed so that we emit diagnostic such as "cannot define parameter 'parm' to be of type 'X (*)[2]' because 'X' is abstract": basically, in that line of the diagnostic we want to print the real type of the parameter, not the innest type which is abstract. Also, removing the check for dependent_type_p does not hurt anything, because there was already a check in place for CLASSTYPE_PURE_VIRTUALS(t), which is usually empty for dependent types. Tested on i686-pc-linux-gnu, OK for mainline? Giovanni Bajo cp/ PR c++/2204 * decl.c (cp_finish_decl): Extract logic to strip array and pointer to array types into... (strip_array_and_pointer_to_array_types): New function. (grokparms): Don't check for abstract types here. (grokdeclarator): Check for abstract types when grokking a parameter. (maybe_register_incomplete_parms): New function. (complete_vars): When completing the type for a PARM_DECL, check for abstractness. Fix a typo in the comment. * class.c (finish_struct): Construct CLASSTYPE_PURE_VIRTUALS for templates, and call complete_vars to check for abstractness. * typeck2.c (abstract_virtuals_error): Do not skip dependend types. Print real type of variables in diagnostic. testsuite/ PR c++/2204 * g++.dg/other/abstract2.C: New test. Index: decl.c =================================================================== RCS file: /cvs/gcc/gcc/gcc/cp/decl.c,v retrieving revision 1.1206 diff -c -3 -p -r1.1206 decl.c *** decl.c 13 May 2004 06:40:16 -0000 1.1206 --- decl.c 11 Jun 2004 10:45:08 -0000 *************** static void expand_static_init (tree, tr *** 120,125 **** --- 120,127 ---- static tree next_initializable_field (tree); static tree reshape_init (tree, tree *); static tree build_typename_type (tree, tree, tree); + static void maybe_register_incomplete_parm (tree); + static tree strip_array_and_pointer_to_array_types (tree); /* Erroneous argument lists can use this *IFF* they do not modify it. */ tree error_mark_list; *************** cp_finish_decl (tree decl, tree init, tr *** 4833,4851 **** || TREE_CODE (type) == METHOD_TYPE) abstract_virtuals_error (decl, strip_array_types (TREE_TYPE (type))); ! else if (POINTER_TYPE_P (type) || TREE_CODE (type) == ARRAY_TYPE) ! { ! /* If it's either a pointer or an array type, strip through all ! of them but the last one. If the last is an array type, issue ! an error if the element type is abstract. */ ! while (POINTER_TYPE_P (TREE_TYPE (type)) ! || TREE_CODE (TREE_TYPE (type)) == ARRAY_TYPE) ! type = TREE_TYPE (type); ! if (TREE_CODE (type) == ARRAY_TYPE) ! abstract_virtuals_error (decl, TREE_TYPE (type)); ! } ! else ! abstract_virtuals_error (decl, type); if (TREE_CODE (decl) == FUNCTION_DECL || TREE_TYPE (decl) == error_mark_node) --- 4835,4844 ---- || TREE_CODE (type) == METHOD_TYPE) abstract_virtuals_error (decl, strip_array_types (TREE_TYPE (type))); ! else ! abstract_virtuals_error (decl, ! strip_array_and_pointer_to_array_types ! (type)); if (TREE_CODE (decl) == FUNCTION_DECL || TREE_TYPE (decl) == error_mark_node) *************** grokdeclarator (tree declarator, *** 7703,7708 **** --- 7696,7718 ---- type = error_mark_node; } + /* If it is a parameter, we need to check whether the type is abstract + or not, and issue an error if it is. */ + if ((decl_context == PARM || decl_context == CATCHPARM) + && type != error_mark_node) + { + tree t = strip_array_and_pointer_to_array_types (type); + tree decl = cp_build_parm_decl (declarator, type); + + /* If the type is not complete at this point, register the parameter + declaration among the incomplete variables, so that it will get + checked when the type is completed. */ + if (!COMPLETE_TYPE_P (t) || currently_open_class (t)) + maybe_register_incomplete_parm (decl); + else + abstract_virtuals_error (decl, t); + } + if ((decl_context == FIELD || decl_context == PARM) && !processing_template_decl && variably_modified_type_p (type)) *************** grokparms (tree first_parm, tree *parms) *** 8636,8643 **** type = build_pointer_type (type); TREE_TYPE (decl) = type; } - else if (abstract_virtuals_error (decl, type)) - any_error = 1; /* Seems like a good idea. */ else if (POINTER_TYPE_P (type)) { /* [dcl.fct]/6, parameter types cannot contain pointers --- 8646,8651 ---- *************** finish_method (tree decl) *** 10966,10972 **** return decl; } ! \f /* VAR is a VAR_DECL. If its type is incomplete, remember VAR so that we can lay it out later, when and if its type becomes complete. */ --- 10974,11024 ---- return decl; } ! ! /* Strip all the array/pointer-to-array types from TYPE. This is used to ! extract the underlying type from declarations such as: ! ! A a[2]; ! A (*a)[2]; ! A (**a[2])[2]; ! ! which are invalid if 'A' is an abstract type. */ ! ! static tree ! strip_array_and_pointer_to_array_types (tree type) ! { ! if (POINTER_TYPE_P (type) || TREE_CODE (type) == ARRAY_TYPE) ! { ! /* If it's either a pointer or an array type, strip through all ! of them but the last one. If the last is an array type, then ! we can return its element type. */ ! while (POINTER_TYPE_P (TREE_TYPE (type)) ! || TREE_CODE (TREE_TYPE (type)) == ARRAY_TYPE) ! type = TREE_TYPE (type); ! if (TREE_CODE (type) == ARRAY_TYPE) ! return TREE_TYPE (type); ! } ! return type; ! } ! ! /* PARM is a PARM_DECL, whose type is incomplete at the point of parsing. ! We keep track of these because we must issue an error if, upon ! completition, the type ends up being abstract. */ ! ! static void ! maybe_register_incomplete_parm (tree var) ! { ! tree inner_type; ! ! my_friendly_assert (TREE_CODE (var) == PARM_DECL, 20040208); ! ! inner_type = TREE_TYPE (var); ! inner_type = strip_array_and_pointer_to_array_types (inner_type); ! inner_type = TYPE_MAIN_VARIANT (inner_type); ! ! if (!COMPLETE_TYPE_P (inner_type) && CLASS_TYPE_P (inner_type)) ! incomplete_vars = tree_cons (inner_type, var, incomplete_vars); ! } /* VAR is a VAR_DECL. If its type is incomplete, remember VAR so that we can lay it out later, when and if its type becomes complete. */ *************** maybe_register_incomplete_var (tree var) *** 10981,10991 **** && DECL_EXTERNAL (var)) { tree inner_type = TREE_TYPE (var); ! while (TREE_CODE (inner_type) == ARRAY_TYPE) inner_type = TREE_TYPE (inner_type); inner_type = TYPE_MAIN_VARIANT (inner_type); ! if ((!COMPLETE_TYPE_P (inner_type) && CLASS_TYPE_P (inner_type)) /* RTTI TD entries are created while defining the type_info. */ || (TYPE_LANG_SPECIFIC (inner_type) --- 11033,11043 ---- && DECL_EXTERNAL (var)) { tree inner_type = TREE_TYPE (var); ! while (TREE_CODE (inner_type) == ARRAY_TYPE) inner_type = TREE_TYPE (inner_type); inner_type = TYPE_MAIN_VARIANT (inner_type); ! if ((!COMPLETE_TYPE_P (inner_type) && CLASS_TYPE_P (inner_type)) /* RTTI TD entries are created while defining the type_info. */ || (TYPE_LANG_SPECIFIC (inner_type) *************** maybe_register_incomplete_var (tree var) *** 10995,11001 **** } /* Called when a class type (given by TYPE) is defined. If there are ! any existing VAR_DECLs whose type hsa been completed by this declaration, update them now. */ void --- 11047,11053 ---- } /* Called when a class type (given by TYPE) is defined. If there are ! any existing VAR_DECLs whose type has been completed by this declaration, update them now. */ void *************** complete_vars (tree type) *** 11012,11017 **** --- 11064,11076 ---- /* Complete the type of the variable. The VAR_DECL itself will be laid out in expand_expr. */ complete_type (TREE_TYPE (var)); + /* If it is a PARM_DECL, we need to emit an error if the + type is abstract, because this is why we registered it + here. */ + if (TREE_CODE (var) == PARM_DECL) + abstract_virtuals_error (var, + strip_array_and_pointer_to_array_types + (TREE_TYPE (var))); /* Remove this entry from the list. */ *list = TREE_CHAIN (*list); } Index: class.c =================================================================== RCS file: /cvs/gcc/gcc/gcc/cp/class.c,v retrieving revision 1.611 diff -c -3 -p -r1.611 class.c *** class.c 13 May 2004 06:40:13 -0000 1.611 --- class.c 11 Jun 2004 10:45:10 -0000 *************** finish_struct (tree t, tree attributes) *** 5243,5250 **** --- 5243,5266 ---- if (processing_template_decl) { + tree x; + finish_struct_methods (t); TYPE_SIZE (t) = bitsize_zero_node; + + /* We need to emit an error message if this type was used as a parameter + and it is an abstract type, even if it is a template. We construct + a simple CLASSTYPE_PURE_VIRTUALS list without taking bases into + account and we call complete_vars with this type, which will check + the PARM_DECLS. Note that while the type is being defined, + CLASSTYPE_PURE_VIRTUALS contains the list of the inline friends + (see CLASSTYPE_INLINE_FRIENDS) so we need to clear it. */ + CLASSTYPE_PURE_VIRTUALS (t) = NULL_TREE; + for (x = TYPE_METHODS (t); x; x = TREE_CHAIN (x)) + if (DECL_PURE_VIRTUAL_P (x)) + CLASSTYPE_PURE_VIRTUALS (t) + = tree_cons (NULL_TREE, x, CLASSTYPE_PURE_VIRTUALS (t)); + complete_vars (t); } else finish_struct_1 (t); Index: typeck2.c =================================================================== RCS file: /cvs/gcc/gcc/gcc/cp/typeck2.c,v retrieving revision 1.159 diff -c -3 -p -r1.159 typeck2.c *** typeck2.c 11 Jun 2004 03:11:05 -0000 1.159 --- typeck2.c 11 Jun 2004 03:32:56 -0000 *************** abstract_virtuals_error (tree decl, tree *** 137,147 **** CLASSTYPE_PURE_VIRTUALS holds the inline friends. */ return 0; - if (dependent_type_p (type)) - /* For a dependent type, we do not yet know which functions are pure - virtuals. */ - return 0; - u = CLASSTYPE_PURE_VIRTUALS (type); if (decl) { --- 137,142 ---- *************** abstract_virtuals_error (tree decl, tree *** 150,162 **** if (TREE_CODE (decl) == VAR_DECL) cp_error_at ("cannot declare variable `%+D' to be of abstract " ! "type `%T'", decl, type); else if (TREE_CODE (decl) == PARM_DECL) cp_error_at ("cannot declare parameter `%+D' to be of abstract " ! "type `%T'", decl, type); else if (TREE_CODE (decl) == FIELD_DECL) cp_error_at ("cannot declare field `%+D' to be of abstract " ! "type `%T'", decl, type); else if (TREE_CODE (decl) == FUNCTION_DECL && TREE_CODE (TREE_TYPE (decl)) == METHOD_TYPE) cp_error_at ("invalid abstract return type for member function `%+#D'", --- 145,157 ---- if (TREE_CODE (decl) == VAR_DECL) cp_error_at ("cannot declare variable `%+D' to be of abstract " ! "type `%T'", decl, TREE_TYPE (decl)); else if (TREE_CODE (decl) == PARM_DECL) cp_error_at ("cannot declare parameter `%+D' to be of abstract " ! "type `%T'", decl, TREE_TYPE (decl)); else if (TREE_CODE (decl) == FIELD_DECL) cp_error_at ("cannot declare field `%+D' to be of abstract " ! "type `%T'", decl, TREE_TYPE (decl)); else if (TREE_CODE (decl) == FUNCTION_DECL && TREE_CODE (TREE_TYPE (decl)) == METHOD_TYPE) cp_error_at ("invalid abstract return type for member function `%+#D'", // { dg-do compile } // Contributed by Gabriel Dos Reis <gdr at integrable-solutions dot net> // PR c++/2204: Check for parameters of abstract type in function declarations. namespace N1 { struct X; struct Y1 { void g(X parm1); // { dg-error "abstract" } void g(X parm2[2]); // { dg-error "abstract" } void g(X (*parm3)[2]); // { dg-error "abstract" } }; template <int N> struct Y2 { void g(X parm4); // { dg-error "abstract" } void g(X parm5[2]); // { dg-error "abstract" } void g(X (*parm6)[2]); // { dg-error "abstract" } }; struct X { // { dg-error "note" } virtual void xfunc(void) = 0; // { dg-error "note" } }; } namespace N2 { struct X1 { // { dg-error "note" } virtual void xfunc(void) = 0; // { dg-error "note" } void g(X1 parm7); // { dg-error "abstract" } void g(X1 parm8[2]); // { dg-error "abstract" } void g(X1 (*parm9)[2]); // { dg-error "abstract" } }; template <int N> struct X2 { // { dg-error "note" } virtual void xfunc(void) = 0; // { dg-error "note" } void g(X2 parm10); // { dg-error "abstract" } void g(X2 parm11[2]); // { dg-error "abstract" } void g(X2 (*parm12)[2]); // { dg-error "abstract" } }; } namespace N3 { struct X { // { dg-error "note" "" } virtual void xfunc(void) = 0; // { dg-error "note" } }; void g(X parm13); // { dg-error "abstract" } void g(X parm14[2]); // { dg-error "abstract" } void g(X (*parm15)[2]); // { dg-error "abstract" } template <int N> void g(X parm16); // { dg-error "abstract" } template <int N> void g(X parm17[2]); // { dg-error "abstract" } template <int N> void g(X (*parm18)[2]); // { dg-error "abstract" } } ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [C++ PATCH] [PR2204] Check for parameters of abstract types - Take 3 2004-06-11 13:52 ` [C++ PATCH] [PR2204] Check for parameters of abstract types - Take 3 Giovanni Bajo @ 2004-06-13 16:01 ` Mark Mitchell 2004-06-14 22:12 ` Jason Merrill 0 siblings, 1 reply; 46+ messages in thread From: Mark Mitchell @ 2004-06-13 16:01 UTC (permalink / raw) To: Giovanni Bajo; +Cc: Jason Merrill, gcc-patches Giovanni Bajo wrote: >Jason Merrill wrote: > >[http://gcc.gnu.org/ml/gcc-patches/2004-02/msg00953.html] > > > >>>Alas, this solution is partial, mainly because it doesn't get into >>>account >>>array types. In fact, parameters with array type are decayed into >>>pointer types within grokdeclarator (even before a PARM_DECL is >>>built), so they never get registered into the incomplete_vars list. >>>A solution for this might be extracting the standard decays out of >>>grokdeclarator, and having grokparms call it. There is also another >>>user of grokdeclarator(PARM) (in pt.c, to build template >>>parameters), which should be updated as well. Comments on this plan >>>are highly appreciated. >>> >>> >>There's a much simpler solution: just check for abstract types in >>create_array_type_for_decl. >> >> > >I tried this but it occurred to me that if I checked for abstract types in >grokdeclarator immediatly after the big loop where the type for the declaration >is constructed (and where create_array_type_for_decl is called for the >ARRAY_REF case), I could catch both the array and the non-array case at the >same time, unifying the check. In fact, it works beautifully, so I removed the >other check for abstractness from grokparms. The only quirk is that I need to >create a PARM_DECL just for the purpose of storing it in the incomplete_vars >list (it cannot be done with the real PARM_DECL because that's already decayed >from array to pointer), but it doesn't seem too much of a problem to me. > > It is a problem -- DECLs are big, and so creating extra DECLs is a problem. We should find another way. -- Mark Mitchell CodeSourcery, LLC (916) 791-8304 mark@codesourcery.com ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [C++ PATCH] [PR2204] Check for parameters of abstract types - Take 3 2004-06-13 16:01 ` Mark Mitchell @ 2004-06-14 22:12 ` Jason Merrill 2004-06-15 2:40 ` Giovanni Bajo 0 siblings, 1 reply; 46+ messages in thread From: Jason Merrill @ 2004-06-14 22:12 UTC (permalink / raw) To: Mark Mitchell; +Cc: Giovanni Bajo, gcc-patches On Sun, 13 Jun 2004 01:05:16 -0700, Mark Mitchell <mark@codesourcery.com> wrote: > Giovanni Bajo wrote: > >>Jason Merrill wrote: >>>There's a much simpler solution: just check for abstract types in >>>create_array_type_for_decl. >> >>I tried this but it occurred to me that if I checked for abstract types in >>grokdeclarator immediatly after the big loop where the type for the declaration >>is constructed (and where create_array_type_for_decl is called for the >>ARRAY_REF case), I could catch both the array and the non-array case at the >>same time, unifying the check. In fact, it works beautifully, so I removed the >>other check for abstractness from grokparms. The only quirk is that I need to >>create a PARM_DECL just for the purpose of storing it in the incomplete_vars >>list (it cannot be done with the real PARM_DECL because that's already decayed >>from array to pointer), but it doesn't seem too much of a problem to me. >> > It is a problem -- DECLs are big, and so creating extra DECLs is a problem. > > We should find another way. I suggested another way. It is in fact a constraint violation to have an array of abstract types regardless of where it appears, so we really should be checking in create_array_type_for_decl regardless of what we do for parms. 8.3.4 Arrays [dcl.array] 1 In a declaration T D where D has the form D1 [constant-expressionopt] and the type of the identifier in the declaration T D1 is "derived- declarator-type-list T," then the type of the identifier of D is an array type. T is called the array element type; this type shall not be a reference type, the (possibly cv-qualified) type void, a function type or an abstract class type. Jason ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [C++ PATCH] [PR2204] Check for parameters of abstract types - Take 3 2004-06-14 22:12 ` Jason Merrill @ 2004-06-15 2:40 ` Giovanni Bajo 2004-06-15 2:49 ` Mark Mitchell 0 siblings, 1 reply; 46+ messages in thread From: Giovanni Bajo @ 2004-06-15 2:40 UTC (permalink / raw) To: Mark Mitchell, Jason Merrill; +Cc: gcc-patches Jason Merrill wrote: > I suggested another way. It is in fact a constraint violation to > have an array of abstract types regardless of where it appears, so we really > should be checking in create_array_type_for_decl regardless of what we do for > parms. Yes, that was my first approacch, but there was no declaration at that point either, and I needed one to register the parameter in the incomplete_vars list. So I moved the test outside, in the called for create_array_type_for_decl, which is grokdeclarator, a few lines below (where we also have the complete type of the symbol being declared). Of course, now that I know I must find a solution which does not involve creating a declaration, I can move back the test there. I will have to create a different incomplete_vars list which stores couples of IDENTIFIER_NODE and TREE_TYPE instead of declarations, for the purpose of delayed evaluation of abstractness. Giovanni Bajo ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [C++ PATCH] [PR2204] Check for parameters of abstract types - Take 3 2004-06-15 2:40 ` Giovanni Bajo @ 2004-06-15 2:49 ` Mark Mitchell 2004-06-15 2:58 ` Giovanni Bajo 2004-06-15 7:29 ` [C++ PATCH] [PR2204] Check for parameters of abstract types -Take 3 Gabriel Dos Reis 0 siblings, 2 replies; 46+ messages in thread From: Mark Mitchell @ 2004-06-15 2:49 UTC (permalink / raw) To: Giovanni Bajo; +Cc: Jason Merrill, gcc-patches Giovanni Bajo wrote: > I will have to create a different incomplete_vars list which stores couples of > IDENTIFIER_NODE and TREE_TYPE instead of declarations, for the purpose of > delayed evaluation of abstractness. I'm sure I'm being dense, but can you back up and explain why we need to save away information at all? Are we trying to detect cases like struct S; void f(S s); struct S { virtual void g() = 0; }; where "f" has a parameter with an abstract type? Does the standard actually require us to diagnose this case? Presumably, we would already diagnose it if someone tried to call or define "f"? Thanks, -- Mark Mitchell CodeSourcery, LLC mark@codesourcery.com ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [C++ PATCH] [PR2204] Check for parameters of abstract types - Take 3 2004-06-15 2:49 ` Mark Mitchell @ 2004-06-15 2:58 ` Giovanni Bajo 2004-06-15 3:04 ` Mark Mitchell 2004-06-15 7:39 ` [C++ PATCH] [PR2204] Check for parameters of abstract types - Take 3 Gabriel Dos Reis 2004-06-15 7:29 ` [C++ PATCH] [PR2204] Check for parameters of abstract types -Take 3 Gabriel Dos Reis 1 sibling, 2 replies; 46+ messages in thread From: Giovanni Bajo @ 2004-06-15 2:58 UTC (permalink / raw) To: Mark Mitchell; +Cc: Jason Merrill, gcc-patches Mark Mitchell wrote: >> I will have to create a different incomplete_vars list which stores >> couples of IDENTIFIER_NODE and TREE_TYPE instead of declarations, >> for the purpose of delayed evaluation of abstractness. > > I'm sure I'm being dense, but can you back up and explain why we need > to save away information at all? Yes. See also comment #3 in Bugzilla, which I quote: ------------------------ The problem is that we currently check for abstractness of parameter types in gorkparms, which is way too early for the current class. Putting it into start_function is a stop-gap, but 10.4/3 says that *declarations* are ill- formed, there is no need for a definition. ------------------------ [class.abstract]/3: ------------------------ An abstract class shall not be used as a parameter type, as a function return type, or as the type of an explicit conversion. Pointers and references to an abstract class can be declared. [Example: shape x; // error: object of abstract class shape* p; // OK shape f(); // error void g(shape); // error shape& h(shape&); // OK --- end example] ------------------------ It is not actually as explicit as I recalled, but I guess that the meaning is obvious: the declarations are ill-formed, there is no need for definition. Also, EDG agrees and diagnose such declarations. > Are we trying to detect cases like > > struct S; > > void f(S s); > > struct S { > virtual void g() = 0; > }; > > where "f" has a parameter with an abstract type? Exactly. If you look in the testcase of my patch, it checks for some slightly different situations which require delayed evaluation of abstract types. The testcase was carefully constructed thinking of different situations. > Does the standard actually require us to diagnose this case? This is my understanding. > Presumably, we would already diagnose it if someone tried to call or > define "f"? Yes, we do that at the definition point. Calling f() is impossible: if use *s with a S*, we get an error because we cannot allocate the temporary. Giovanni Bajo ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [C++ PATCH] [PR2204] Check for parameters of abstract types - Take 3 2004-06-15 2:58 ` Giovanni Bajo @ 2004-06-15 3:04 ` Mark Mitchell 2004-06-15 3:06 ` Giovanni Bajo ` (2 more replies) 2004-06-15 7:39 ` [C++ PATCH] [PR2204] Check for parameters of abstract types - Take 3 Gabriel Dos Reis 1 sibling, 3 replies; 46+ messages in thread From: Mark Mitchell @ 2004-06-15 3:04 UTC (permalink / raw) To: Giovanni Bajo; +Cc: Jason Merrill, gcc-patches Giovanni Bajo wrote: > Exactly. If you look in the testcase of my patch, it checks for some slightly > different situations which require delayed evaluation of abstract types. The > testcase was carefully constructed thinking of different situations. > >>Presumably, we would already diagnose it if someone tried to call or >>define "f"? > > > Yes, we do that at the definition point. Calling f() is impossible: if use *s > with a S*, we get an error because we cannot allocate the temporary. Yes, that's what I meant. Curiously, EDG diagnoses the example I gave, but not: struct S; extern S s; struct S { virtual void g() = 0; }; Anyhow, why not just keep a list of all VAR_DECLs and FUNCTION_DECLs that have a parameter of an incomplete type, attached to that incomplete type? Then, when a type is completed, if it is abstract, run down the list issuing error messages. After the type is complete, clear the list. To save space, use a hash table mapping class types to these lists, rather than using space in every class type. -- Mark Mitchell CodeSourcery, LLC mark@codesourcery.com ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [C++ PATCH] [PR2204] Check for parameters of abstract types - Take 3 2004-06-15 3:04 ` Mark Mitchell @ 2004-06-15 3:06 ` Giovanni Bajo 2004-06-15 4:20 ` Jason Merrill 2004-06-15 7:39 ` [C++ PATCH] [PR2204] Check for parameters of abstract types -Take 3 Gabriel Dos Reis 2 siblings, 0 replies; 46+ messages in thread From: Giovanni Bajo @ 2004-06-15 3:06 UTC (permalink / raw) To: Mark Mitchell; +Cc: Jason Merrill, gcc-patches Mark Mitchell wrote: > Curiously, EDG diagnoses the example I gave, but not: > > struct S; > > extern S s; > > struct S { virtual void g() = 0; }; I would call this a bug in EDG. > Anyhow, why not just keep a list of all VAR_DECLs and FUNCTION_DECLs > that have a parameter of an incomplete type, attached to that > incomplete type? Then, when a type is completed, if it is abstract, run down the > list issuing error messages. After the type is complete, clear the > list. This is basically what I am doing in my patch. Instead of using a list attacched to each type, I'm reusing the existing global incomplete_vars list, which keeps declaration of incomplete types, so that we can layout stuff when the types are completed. It was something that was already in place, I just added my own decls to them, and added the check for abstractness. Replacing the list with a hash table is just an optimization which could be done as a follow up patch I believe. But there is a problem in reusing existing decls: we never construct a PARM_DECL with array type, because it gets decayed to pointer before we construct the declaration. My patch was coping with this by constructing a "fake" PARM_DECL with array type to be inserted into the list (or attacched to the type [or hash table], using your suggestion). Since this happens to be too memory hungry, I was going to change the list into couples of [IDENTIFIER_NODE, TYPE]. Giovanni Bajo ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [C++ PATCH] [PR2204] Check for parameters of abstract types - Take 3 2004-06-15 3:04 ` Mark Mitchell 2004-06-15 3:06 ` Giovanni Bajo @ 2004-06-15 4:20 ` Jason Merrill 2004-06-21 14:37 ` [C++ PATCH] [PR2204] Check for parameters of abstract types - Take 4 Giovanni Bajo 2004-06-15 7:39 ` [C++ PATCH] [PR2204] Check for parameters of abstract types -Take 3 Gabriel Dos Reis 2 siblings, 1 reply; 46+ messages in thread From: Jason Merrill @ 2004-06-15 4:20 UTC (permalink / raw) To: Mark Mitchell; +Cc: Giovanni Bajo, gcc-patches On Mon, 14 Jun 2004 17:56:19 -0700, Mark Mitchell <mark@codesourcery.com> wrote: > Anyhow, why not just keep a list of all VAR_DECLs and FUNCTION_DECLs that > have a parameter of an incomplete type, attached to that incomplete type? > Then, when a type is completed, if it is abstract, run down the list > issuing error messages. After the type is complete, clear the list. To > save space, use a hash table mapping class types to these lists, rather > than using space in every class type. Sounds like this checking could be combine with the existing use of incomplete_vars. Jason ^ permalink raw reply [flat|nested] 46+ messages in thread
* [C++ PATCH] [PR2204] Check for parameters of abstract types - Take 4 2004-06-15 4:20 ` Jason Merrill @ 2004-06-21 14:37 ` Giovanni Bajo 0 siblings, 0 replies; 46+ messages in thread From: Giovanni Bajo @ 2004-06-21 14:37 UTC (permalink / raw) To: Jason Merrill, Mark Mitchell; +Cc: gcc-patches Jason Merrill wrote: >> Anyhow, why not just keep a list of all VAR_DECLs and FUNCTION_DECLs >> that have a parameter of an incomplete type, attached to that >> incomplete type? Then, when a type is completed, if it is abstract, >> run down the list >> issuing error messages. After the type is complete, clear the list. >> To >> save space, use a hash table mapping class types to these lists, >> rather >> than using space in every class type. > > Sounds like this checking could be combine with the existing use of > incomplete_vars. Yup, in fact it is not so different from what I was doing before. This is the updated patch. This time, I am hiding the deferral of the check for abstractness in case of incomplete type within abstract_virtuals_error, so that callers do not have to bother checking. I am also following Mark's advice to use a hash table instead of a list, and Jason's advice to insert a check within create_array_type_for_decl to handle array types (which creates a less accurate diagnostic, but still good enough). The new structure I created (struct pending_abstract_type) is needed basically because I have to store the locus to emit a diagnostic at the right place. For DECLs, the locus is already present within the DECL itself, but for IDENTIFIER_NODEs it is not. In case you wonder, it is create_array_type_for_decl which passes IDENTIFIER_NODEs to abstract_virtuals_error. Also, there are some situations where the declaration can be NULL (for instance, when creating an array type within a functional cast). As a follow-up patch, we can change incomplete_vars to use a hash table too, or merge it with pending_abstract_type. Bootstrapped & tested on i686-pc-linux-gnu with no new regressions, OK for mainline? Giovanni Bajo cp/ PR c++/2204 * config-lang.in (gtfiles): Add typeck2.c. * Make-lang.in: Tweak typeck2.c dependencies, and add rule for gt-cp-typeck2.h. * cp-tree.h: Declare complete_type_check_abstract. * typeck2.c (pat_calc_hash, pat_compare, complete_type_check_abstract): New functions. (abstract_virtuals_error): If the type is abstract, register the declaration within abstract_pending_vars for further checks. Inspect also dependent types. Handle IDENTIFIER_NODEs as decl. * decl.c (cp_finish_decl): Do not strip array types. (create_array_type_for_decl): Check for abstractness of the element type. (complete_vars): Call complete_type_check_abstract. * class.c (finish_struct): Prepare a list of virtual functions for template types, and call complete_vars on it to check for abstractness. testsuite/ PR c++/2204 * g++.dg/other/abstract2.C: New test. Index: config-lang.in =================================================================== RCS file: /cvs/gcc/gcc/gcc/cp/config-lang.in,v retrieving revision 1.21 diff -c -3 -p -r1.21 config-lang.in *** config-lang.in 27 Feb 2004 00:31:47 -0000 1.21 --- config-lang.in 21 Jun 2004 10:32:45 -0000 *************** stagestuff="g++\$(exeext) g++-cross\$(ex *** 34,37 **** target_libs="target-libstdc++-v3 target-gperf" ! gtfiles="\$(srcdir)/cp/mangle.c \$(srcdir)/cp/name-lookup.h \$(srcdir)/cp/name-lookup.c \$(srcdir)/cp/cp-tree.h \$(srcdir)/cp/decl.h \$(srcdir)/cp/lex.h \$(srcdir)/cp/call.c \$(srcdir)/cp/decl.c \$(srcdir)/cp/decl2.c \$(srcdir)/cp/pt.c \$(srcdir)/cp/repo.c \$(srcdir)/cp/semantics.c \$(srcdir)/cp/tree.c \$(srcdir)/cp/parser.c \$(srcdir)/cp/method.c \$(srcdir)/c-common.c \$(srcdir)/c-common.h \$(srcdir)/c-lex.c \$(srcdir)/c-pragma.c" --- 34,37 ---- target_libs="target-libstdc++-v3 target-gperf" ! gtfiles="\$(srcdir)/cp/mangle.c \$(srcdir)/cp/name-lookup.h \$(srcdir)/cp/name-lookup.c \$(srcdir)/cp/cp-tree.h \$(srcdir)/cp/decl.h \$(srcdir)/cp/lex.h \$(srcdir)/cp/call.c \$(srcdir)/cp/decl.c \$(srcdir)/cp/decl2.c \$(srcdir)/cp/pt.c \$(srcdir)/cp/repo.c \$(srcdir)/cp/semantics.c \$(srcdir)/cp/tree.c \$(srcdir)/cp/parser.c \$(srcdir)/cp/method.c \$(srcdir)/cp/typeck2.c \$(srcdir)/c-common.c \$(srcdir)/c-common.h \$(srcdir)/c-lex.c \$(srcdir)/c-pragma.c" Index: Make-lang.in =================================================================== RCS file: /cvs/gcc/gcc/gcc/cp/Make-lang.in,v retrieving revision 1.183 diff -c -3 -p -r1.183 Make-lang.in *** Make-lang.in 24 May 2004 10:50:43 -0000 1.183 --- Make-lang.in 21 Jun 2004 10:32:45 -0000 *************** $(srcdir)/cp/cfns.h: $(srcdir)/cp/cfns.g *** 99,105 **** gtype-cp.h gt-cp-call.h gt-cp-decl.h gt-cp-decl2.h : s-gtype; @true gt-cp-pt.h gt-cp-repo.h gt-cp-parser.h gt-cp-method.h : s-gtype; @true ! gt-cp-tree.h gt-cp-mangle.h gt-cp-name-lookup.h: s-gtype; @true #\f # Build hooks: --- 99,105 ---- gtype-cp.h gt-cp-call.h gt-cp-decl.h gt-cp-decl2.h : s-gtype; @true gt-cp-pt.h gt-cp-repo.h gt-cp-parser.h gt-cp-method.h : s-gtype; @true ! gt-cp-tree.h gt-cp-mangle.h gt-cp-name-lookup.h gt-cp-typeck2.h: s-gtype; @true #\f # Build hooks: *************** cp/decl.o: cp/decl.c $(CXX_TREE_H) $(TM_ *** 227,233 **** cp/decl2.o: cp/decl2.c $(CXX_TREE_H) $(TM_H) flags.h cp/lex.h cp/decl.h $(EXPR_H) \ output.h except.h toplev.h $(RTL_H) c-common.h gt-cp-decl2.h cgraph.h cp/typeck2.o: cp/typeck2.c $(CXX_TREE_H) $(TM_H) flags.h toplev.h output.h $(TM_P_H) \ ! diagnostic.h cp/typeck.o: cp/typeck.c $(CXX_TREE_H) $(TM_H) flags.h $(RTL_H) $(EXPR_H) toplev.h \ diagnostic.h convert.h cp/class.o: cp/class.c $(CXX_TREE_H) $(TM_H) flags.h toplev.h $(RTL_H) $(TARGET_H) convert.h --- 227,233 ---- cp/decl2.o: cp/decl2.c $(CXX_TREE_H) $(TM_H) flags.h cp/lex.h cp/decl.h $(EXPR_H) \ output.h except.h toplev.h $(RTL_H) c-common.h gt-cp-decl2.h cgraph.h cp/typeck2.o: cp/typeck2.c $(CXX_TREE_H) $(TM_H) flags.h toplev.h output.h $(TM_P_H) \ ! diagnostic.h gt-cp-typeck2.h cp/typeck.o: cp/typeck.c $(CXX_TREE_H) $(TM_H) flags.h $(RTL_H) $(EXPR_H) toplev.h \ diagnostic.h convert.h cp/class.o: cp/class.c $(CXX_TREE_H) $(TM_H) flags.h toplev.h $(RTL_H) $(TARGET_H) convert.h Index: cp-tree.h =================================================================== RCS file: /cvs/gcc/gcc/gcc/cp/cp-tree.h,v retrieving revision 1.977 diff -c -3 -p -r1.977 cp-tree.h *** cp-tree.h 16 Jun 2004 01:21:29 -0000 1.977 --- cp-tree.h 21 Jun 2004 10:32:46 -0000 *************** extern void cxx_incomplete_type_error ( *** 4275,4280 **** --- 4275,4281 ---- extern tree error_not_base_type (tree, tree); extern tree binfo_or_else (tree, tree); extern void readonly_error (tree, const char *, int); + extern void complete_type_check_abstract (tree); extern int abstract_virtuals_error (tree, tree); extern tree store_init_value (tree, tree); Index: typeck2.c =================================================================== RCS file: /cvs/gcc/gcc/gcc/cp/typeck2.c,v retrieving revision 1.160 diff -c -3 -p -r1.160 typeck2.c *** typeck2.c 16 Jun 2004 01:21:35 -0000 1.160 --- typeck2.c 21 Jun 2004 10:32:46 -0000 *************** readonly_error (tree arg, const char* st *** 118,123 **** --- 118,236 ---- (*fn) ("%s of read-only location", string); } + \f + /* Structure that holds information about declarations whose type was + incomplete and we could not check whether it was abstract or not. */ + + struct pending_abstract_type GTY((chain_next ("%h.next"))) + { + /* Declaration which we are checking for abstractness. It is either + a DECL node, or an IDENTIFIER_NODE if we do not have a full + declaration available. */ + tree decl; + + /* Type which will be checked for abstractness. */ + tree type; + + /* Position of the declaration. This is only needed for IDENTIFIER_NODEs, + because DECLs already carry locus information. */ + location_t locus; + + /* Link to the next element in list. */ + struct pending_abstract_type* next; + }; + + + /* Compute the hash value of the node VAL. This function is used by the + hash table abstract_pending_vars. */ + + static hashval_t + pat_calc_hash (const void* val) + { + const struct pending_abstract_type* pat = val; + return (hashval_t) TYPE_UID (pat->type); + } + + + /* Compare node VAL1 with the type VAL2. This function is used by the + hash table abstract_pending_vars. */ + + static int + pat_compare (const void* val1, const void* val2) + { + const struct pending_abstract_type* pat1 = val1; + tree type2 = (tree)val2; + + return (pat1->type == type2); + } + + /* Hash table that maintains pending_abstract_type nodes, for which we still + need to check for type abstractness. The key of the table is the type + of the declaration. */ + static GTY ((param_is (struct pending_abstract_type))) + htab_t abstract_pending_vars = NULL; + + + /* This function is called after TYPE is completed, and will check if there + are pending declarations for which we still need to verify the abstractness + of TYPE, and emit a diagnostic (through abstract_virtuals_error) if TYPE + turned out to be incomplete. */ + + void + complete_type_check_abstract (tree type) + { + void **slot; + struct pending_abstract_type *pat; + location_t cur_loc = input_location; + + my_friendly_assert (COMPLETE_TYPE_P (type), 20040620_3); + + if (!abstract_pending_vars) + return; + + /* Retrieve the list of pending declarations for this type. */ + slot = htab_find_slot_with_hash (abstract_pending_vars, type, + (hashval_t)TYPE_UID (type), NO_INSERT); + if (!slot) + return; + pat = (struct pending_abstract_type*)*slot; + my_friendly_assert (pat, 20040620_2); + + /* If the type is not abstract, do not do anything. */ + if (CLASS_TYPE_P (type) || CLASSTYPE_PURE_VIRTUALS (type)) + { + struct pending_abstract_type *prev = 0, *next; + + /* Reverse the list to emit the errors in top-down order. */ + for (; pat; pat = next) + { + next = pat->next; + pat->next = prev; + prev = pat; + } + pat = prev; + + /* Go through the list, and call abstract_virtuals_error for each + element: it will issue a diagostic if the type is abstract. */ + while (pat) + { + my_friendly_assert (type == pat->type, 20040620_4); + + /* Tweak input_location so that the diagnostic appears at the correct + location. Notice that this is only needed if the decl is an + IDENTIFIER_NODE, otherwise cp_error_at. */ + input_location = pat->locus; + abstract_virtuals_error (pat->decl, pat->type); + pat = pat->next; + } + } + + htab_clear_slot (abstract_pending_vars, slot); + + input_location = cur_loc; + } + + /* If TYPE has abstract virtual functions, issue an error about trying to create an object of that type. DECL is the object declared, or NULL_TREE if the declaration is unavailable. Returns 1 if an error *************** abstract_virtuals_error (tree decl, tree *** 129,135 **** tree u; tree tu; ! if (!CLASS_TYPE_P (type) || !CLASSTYPE_PURE_VIRTUALS (type)) return 0; if (!TYPE_SIZE (type)) --- 242,286 ---- tree u; tree tu; ! /* This function applies only to classes. Any other entity can never ! be abstract. */ ! if (!CLASS_TYPE_P (type)) ! return 0; ! ! /* If the type is incomplete, we register it within a hash table, ! so that we can check again once it is completed. This makes sense ! only for objects for which we have a declaration or at least a ! name. */ ! if (!COMPLETE_TYPE_P (type)) ! { ! void **slot; ! struct pending_abstract_type *pat; ! ! my_friendly_assert (!decl || (DECL_P (decl) ! || TREE_CODE (decl) == IDENTIFIER_NODE), ! 20040620_1); ! ! if (!abstract_pending_vars) ! abstract_pending_vars = htab_create_ggc (31, &pat_calc_hash, ! &pat_compare, NULL); ! ! slot = htab_find_slot_with_hash (abstract_pending_vars, type, ! (hashval_t)TYPE_UID (type), INSERT); ! ! pat = ggc_alloc (sizeof (struct pending_abstract_type)); ! pat->type = type; ! pat->decl = decl; ! pat->locus = ((decl && DECL_P (decl)) ! ? DECL_SOURCE_LOCATION (decl) ! : input_location); ! ! pat->next = *slot; ! *slot = pat; ! ! return 0; ! } ! ! if (!CLASSTYPE_PURE_VIRTUALS (type)) return 0; if (!TYPE_SIZE (type)) *************** abstract_virtuals_error (tree decl, tree *** 137,147 **** CLASSTYPE_PURE_VIRTUALS holds the inline friends. */ return 0; - if (dependent_type_p (type)) - /* For a dependent type, we do not yet know which functions are pure - virtuals. */ - return 0; - u = CLASSTYPE_PURE_VIRTUALS (type); if (decl) { --- 288,293 ---- *************** abstract_virtuals_error (tree decl, tree *** 164,169 **** --- 310,319 ---- else if (TREE_CODE (decl) == FUNCTION_DECL) cp_error_at ("invalid abstract return type for function `%+#D'", decl); + else if (TREE_CODE (decl) == IDENTIFIER_NODE) + /* Here we do not have location information, so use error instead + of cp_error_at. */ + error ("invalid abstract type `%T' for `%E'", type, decl); else cp_error_at ("invalid abstract type for `%+D'", decl); } *************** require_complete_eh_spec_types (tree fnt *** 1405,1407 **** --- 1555,1560 ---- } } } + + \f + #include "gt-cp-typeck2.h" Index: decl.c =================================================================== RCS file: /cvs/gcc/gcc/gcc/cp/decl.c,v retrieving revision 1.1218 diff -c -3 -p -r1.1218 decl.c *** decl.c 16 Jun 2004 01:21:30 -0000 1.1218 --- decl.c 21 Jun 2004 10:32:49 -0000 *************** cp_finish_decl (tree decl, tree init, tr *** 4866,4886 **** make_rtl_for_nonlocal_decl (decl, init, asmspec); if (TREE_CODE (type) == FUNCTION_TYPE || TREE_CODE (type) == METHOD_TYPE) ! abstract_virtuals_error (decl, ! strip_array_types (TREE_TYPE (type))); ! else if (POINTER_TYPE_P (type) || TREE_CODE (type) == ARRAY_TYPE) ! { ! /* If it's either a pointer or an array type, strip through all ! of them but the last one. If the last is an array type, issue ! an error if the element type is abstract. */ ! while (POINTER_TYPE_P (TREE_TYPE (type)) ! || TREE_CODE (TREE_TYPE (type)) == ARRAY_TYPE) ! type = TREE_TYPE (type); ! if (TREE_CODE (type) == ARRAY_TYPE) ! abstract_virtuals_error (decl, TREE_TYPE (type)); ! } else abstract_virtuals_error (decl, type); --- 4866,4877 ---- make_rtl_for_nonlocal_decl (decl, init, asmspec); + /* Check for abstractness of the type. Notice that there is no + need to strip array types here since the check for those types + is already done within create_array_type_for_decl. */ if (TREE_CODE (type) == FUNCTION_TYPE || TREE_CODE (type) == METHOD_TYPE) ! abstract_virtuals_error (decl, TREE_TYPE (type)); else abstract_virtuals_error (decl, type); *************** create_array_type_for_decl (tree name, t *** 6254,6259 **** --- 6245,6255 ---- if (size) itype = compute_array_index_type (name, size); + /* [dcl.array] + T is called the array element type; this type shall not be [...] an + abstract class type. */ + abstract_virtuals_error (name, type); + return build_cplus_array_type (type, itype); } *************** complete_vars (tree type) *** 11051,11056 **** --- 11047,11055 ---- else list = &TREE_CHAIN (*list); } + + /* Check for pending declarations which may have abstract type. */ + complete_type_check_abstract (type); } /* If DECL is of a type which needs a cleanup, build that cleanup Index: class.c =================================================================== RCS file: /cvs/gcc/gcc/gcc/cp/class.c,v retrieving revision 1.616 diff -c -3 -p -r1.616 class.c *** class.c 31 May 2004 08:42:31 -0000 1.616 --- class.c 21 Jun 2004 10:32:51 -0000 *************** finish_struct (tree t, tree attributes) *** 5260,5267 **** --- 5260,5283 ---- if (processing_template_decl) { + tree x; + finish_struct_methods (t); TYPE_SIZE (t) = bitsize_zero_node; + + /* We need to emit an error message if this type was used as a parameter + and it is an abstract type, even if it is a template. We construct + a simple CLASSTYPE_PURE_VIRTUALS list without taking bases into + account and we call complete_vars with this type, which will check + the PARM_DECLS. Note that while the type is being defined, + CLASSTYPE_PURE_VIRTUALS contains the list of the inline friends + (see CLASSTYPE_INLINE_FRIENDS) so we need to clear it. */ + CLASSTYPE_PURE_VIRTUALS (t) = NULL_TREE; + for (x = TYPE_METHODS (t); x; x = TREE_CHAIN (x)) + if (DECL_PURE_VIRTUAL_P (x)) + CLASSTYPE_PURE_VIRTUALS (t) + = tree_cons (NULL_TREE, x, CLASSTYPE_PURE_VIRTUALS (t)); + complete_vars (t); } else finish_struct_1 (t); // { dg-do compile } // Contributed by Gabriel Dos Reis <gdr at integrable-solutions dot net> // PR c++/2204: Check for parameters of abstract type in function declarations. namespace N1 { struct X; struct Y1 { void g(X parm1); // { dg-error "abstract" } void g(X parm2[2]); // { dg-error "abstract" } void g(X (*parm3)[2]); // { dg-error "abstract" } }; template <int N> struct Y2 { void g(X parm4); // { dg-error "abstract" } void g(X parm5[2]); // { dg-error "abstract" } void g(X (*parm6)[2]); // { dg-error "abstract" } }; struct X { // { dg-error "note" } virtual void xfunc(void) = 0; // { dg-error "note" } }; } namespace N2 { struct X1 { // { dg-error "note" } virtual void xfunc(void) = 0; // { dg-error "note" } void g(X1 parm7); // { dg-error "abstract" } void g(X1 parm8[2]); // { dg-error "abstract" } void g(X1 (*parm9)[2]); // { dg-error "abstract" } }; template <int N> struct X2 { // { dg-error "note" } virtual void xfunc(void) = 0; // { dg-error "note" } void g(X2 parm10); // { dg-error "abstract" } void g(X2 parm11[2]); // { dg-error "abstract" } void g(X2 (*parm12)[2]); // { dg-error "abstract" } }; } namespace N3 { struct X { // { dg-error "note" "" } virtual void xfunc(void) = 0; // { dg-error "note" } }; void g(X parm13); // { dg-error "abstract" } void g(X parm14[2]); // { dg-error "abstract" } void g(X (*parm15)[2]); // { dg-error "abstract" } template <int N> void g(X parm16); // { dg-error "abstract" } template <int N> void g(X parm17[2]); // { dg-error "abstract" } template <int N> void g(X (*parm18)[2]); // { dg-error "abstract" } } ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [C++ PATCH] [PR2204] Check for parameters of abstract types -Take 3 2004-06-15 3:04 ` Mark Mitchell 2004-06-15 3:06 ` Giovanni Bajo 2004-06-15 4:20 ` Jason Merrill @ 2004-06-15 7:39 ` Gabriel Dos Reis 2 siblings, 0 replies; 46+ messages in thread From: Gabriel Dos Reis @ 2004-06-15 7:39 UTC (permalink / raw) To: Mark Mitchell; +Cc: Giovanni Bajo, Jason Merrill, gcc-patches You Wrote Mark Mitchell > Giovanni Bajo wrote: > >> Exactly. If you look in the testcase of my patch, it checks for some >> slightly >> different situations which require delayed evaluation of abstract types. >> The >> testcase was carefully constructed thinking of different situations. >> >>>Presumably, we would already diagnose it if someone tried to call or >>>define "f"? >> >> >> Yes, we do that at the definition point. Calling f() is impossible: if >> use *s >> with a S*, we get an error because we cannot allocate the temporary. > > Yes, that's what I meant. > > Curiously, EDG diagnoses the example I gave, but not: I think that is a bug in EDG in diagnosing your earlier example. Your very example can be expanded in combining two translation units: // 1.C struct S; void f(S); // 2.C struct S { virtual void g() = 0; }; int main() { return 0; } How do you diagnose the declaration in 1.C, if you follow Giovanni's reading? ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [C++ PATCH] [PR2204] Check for parameters of abstract types - Take 3 2004-06-15 2:58 ` Giovanni Bajo 2004-06-15 3:04 ` Mark Mitchell @ 2004-06-15 7:39 ` Gabriel Dos Reis 1 sibling, 0 replies; 46+ messages in thread From: Gabriel Dos Reis @ 2004-06-15 7:39 UTC (permalink / raw) To: Giovanni Bajo; +Cc: Mark Mitchell, Jason Merrill, gcc-patches You Wrote Giovanni Bajo > Mark Mitchell wrote: > >>> I will have to create a different incomplete_vars list which stores >>> couples of IDENTIFIER_NODE and TREE_TYPE instead of declarations, >>> for the purpose of delayed evaluation of abstractness. >> >> I'm sure I'm being dense, but can you back up and explain why we need >> to save away information at all? > > Yes. See also comment #3 in Bugzilla, which I quote: > > ------------------------ > The problem is that we currently check for abstractness of parameter types > in > gorkparms, which is way too early for the current class. Putting it into > start_function is a stop-gap, but 10.4/3 says that *declarations* are ill- > formed, there is no need for a definition. yes, but that paragraph does not apply to the example given by Mark -- declarations at non-class scope are processed in there order of appearance. By the time you see the declaration for f, lookup says that S is incomplete and we don't know it is abstract. When S is completed, that happens after the declaration of f so it does not affect the well-form-ness of that declaration. ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [C++ PATCH] [PR2204] Check for parameters of abstract types -Take 3 2004-06-15 2:49 ` Mark Mitchell 2004-06-15 2:58 ` Giovanni Bajo @ 2004-06-15 7:29 ` Gabriel Dos Reis 1 sibling, 0 replies; 46+ messages in thread From: Gabriel Dos Reis @ 2004-06-15 7:29 UTC (permalink / raw) To: Mark Mitchell; +Cc: Giovanni Bajo, Jason Merrill, gcc-patches You Wrote Mark Mitchell > Giovanni Bajo wrote: > >> I will have to create a different incomplete_vars list which stores >> couples of >> IDENTIFIER_NODE and TREE_TYPE instead of declarations, for the purpose >> of >> delayed evaluation of abstractness. > > I'm sure I'm being dense, but can you back up and explain why we need to > save away information at all? > > Are we trying to detect cases like > > struct S; > > void f(S s); > > struct S { > virtual void g() = 0; > }; > > where "f" has a parameter with an abstract type? > > Does the standard actually require us to diagnose this case? No, it does not require we diagnose the declaration of f -- and it can't require that in a resaonably simple way. I don't believe we should try to diagnose that. > Presumably, we would already diagnose it if someone tried to call or > define "f"? A call to f requires S to be both complete and non-abstract. The same goes for the definition of f. But by the time user calls f, we either error because f is undeclared, or S is incomplete or we know S is abstract. If all those cases, things follow naturally, The trickier cases are when we have nested classes in templates. ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) 2004-02-10 3:14 ` Gabriel Dos Reis 2004-02-10 17:50 ` [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) - Take 2 Giovanni Bajo @ 2004-02-21 13:45 ` Gabriel Dos Reis 1 sibling, 0 replies; 46+ messages in thread From: Gabriel Dos Reis @ 2004-02-21 13:45 UTC (permalink / raw) To: Giovanni Bajo; +Cc: gcc-patches "Giovanni Bajo" <giovannibajo@libero.it> writes: | Gabriel Dos Reis wrote: | | > As a side comment, I don't understand why you went through that | > change. | | This is the rationale for my changes to abstract_virtual_errors. | | * Use of %J (will be error_at): | With my patch, abstract_virtual_errors can be deferred to an indefinite moment | (when the type is completed). Without this change, the error message appears on | the last line of the completed type, which is wrong: we need to show the user | where is the parameter whose type is abstract. Then, the other messages about | the pure virtual functions will point her to the functions which need to be | overridden, and thus to the type. That makes sense. | * Rewording | There is no such thing as an "abstract virtual function". Standard calls it | "pure virtual function". I agree too. I've been seeing those messages these last days (personal experience forgetting to define dozens of pure virtual functions in a very complex class hierarchy). They look indeed indeed very odd. | * Use of inform vs error | Those messages are just additional information provided to help the user, they | are not part of the error message proper. Moreover, I heard someone wanted to | have a -fno-notes mode one day, so it makes sense to be able to shut those off. | This is also why I added the word "abstract" on the first error line, so that | it is now a complete error message, and the notes are really optional. Before, | the first line would have been unclear, were the notes disabled for some | reason. This makes sense. | Sorry, I should have mentioned these things in the patch message. No need to sorry. What I don't understand is the use of '%J' in lieu of xxx_at. Thanks, -- Gaby ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) 2004-02-10 3:12 ` Giovanni Bajo 2004-02-10 3:14 ` Gabriel Dos Reis @ 2004-02-21 13:45 ` Giovanni Bajo 1 sibling, 0 replies; 46+ messages in thread From: Giovanni Bajo @ 2004-02-21 13:45 UTC (permalink / raw) To: Gabriel Dos Reis; +Cc: gcc-patches Gabriel Dos Reis wrote: > As a side comment, I don't understand why you went through that > change. This is the rationale for my changes to abstract_virtual_errors. * Use of %J (will be error_at): With my patch, abstract_virtual_errors can be deferred to an indefinite moment (when the type is completed). Without this change, the error message appears on the last line of the completed type, which is wrong: we need to show the user where is the parameter whose type is abstract. Then, the other messages about the pure virtual functions will point her to the functions which need to be overridden, and thus to the type. * Rewording There is no such thing as an "abstract virtual function". Standard calls it "pure virtual function". * Use of inform vs error Those messages are just additional information provided to help the user, they are not part of the error message proper. Moreover, I heard someone wanted to have a -fno-notes mode one day, so it makes sense to be able to shut those off. This is also why I added the word "abstract" on the first error line, so that it is now a complete error message, and the notes are really optional. Before, the first line would have been unclear, were the notes disabled for some reason. Sorry, I should have mentioned these things in the patch message. Giovanni Bajo ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) 2004-02-10 2:40 ` Gabriel Dos Reis 2004-02-10 2:47 ` Zack Weinberg 2004-02-10 3:12 ` Giovanni Bajo @ 2004-02-21 13:45 ` Gabriel Dos Reis 2 siblings, 0 replies; 46+ messages in thread From: Gabriel Dos Reis @ 2004-02-21 13:45 UTC (permalink / raw) To: Giovanni Bajo; +Cc: gcc-patches "Giovanni Bajo" <giovannibajo@libero.it> writes: | if (TREE_CODE (decl) == VAR_DECL) | ! error ("%Jcannot declare variable `%D' to be of abstract type `%T'", | ! decl, decl, type); That is an abuse of %J. Use error_at instead. E.g. error_at ("cannot declare variable `%+D' to be of abstract type `%T'", decl, type); | ! error ("%Jcannot declare parameter `%D' to be of abstract type `%T'", | ! decl, decl, type); Ditto. | else if (TREE_CODE (decl) == FIELD_DECL) | ! error ("%Jcannot declare field `%D' to be of abstract type `%T'", | ! decl, decl, type); Ditto. | else if (TREE_CODE (decl) == FUNCTION_DECL | && TREE_CODE (TREE_TYPE (decl)) == METHOD_TYPE) | ! error ("%Jinvalid abstract return type for member function `%#D'", | ! decl, decl); Ditto. | else if (TREE_CODE (decl) == FUNCTION_DECL) | ! error ("%Jinvalid abstract return type for function `%#D'", | ! decl, decl); Ditto. As a side comment, I don't understand why you went through that change. -- Gaby ^ permalink raw reply [flat|nested] 46+ messages in thread
* [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) 2004-02-10 0:37 [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) Giovanni Bajo 2004-02-10 2:40 ` Gabriel Dos Reis @ 2004-02-21 13:45 ` Giovanni Bajo 1 sibling, 0 replies; 46+ messages in thread From: Giovanni Bajo @ 2004-02-21 13:45 UTC (permalink / raw) To: gcc-patches Hello, this is a partial fix for PR2204, which is about checking if PARM_DECLs in function declarations have abstract types. Currently, the check is being performed in grokparms, which is fine for many cases, but it's way too early if the type is the class being defined, or an incomplete type. In that case, we need to defer the check later, when/if the type is completed. To do this, I allow PARM_DECLS to be registered in the incomplete_vars list (by grokparms) and check for abstractness upon type completition (finish_struct_1, which calls complete_vars). Notice that we cannot check it in start_function, because the program is ill-formed even if there is no definition for the function. Alas, this solution is partial, mainly because it doesn't get into account array types. In fact, parameters with array type are decayed into pointer types within grokdeclarator (even before a PARM_DECL is built), so they never get registered into the incomplete_vars list. A solution for this might be extracting the standard decays out of grokdeclarator, and having grokparms call it. There is also another user of grokdeclarator(PARM) (in pt.c, to build template parameters), which should be updated as well. Comments on this plan are highly appreciated. Another case we don't handle yet is when the type of the parameter is dependent. In that case, we probably need to check for abstractness upon instantiation, while tsubsting. I'll investigate. Meanwhile, this tested succesfully on i686-pc-linux-gnu. The testcase have a handful of xfails for the cases we don't handle yet. OK for mainline? Giovanni Bajo 2004-02-10 Giovanni Bajo <giovannibajo@gcc.gnu.org> PR c++/2204 * decl.c (cp_finish_decl): Extract logic to strip array and pointer to array types into... (strip_array_and_pointer_to_array_types): New function. (grokparms): Register PARM_DECLs of incomplete types for later check, using... (maybe_register_incomplete_parms): New function. (complete_vars): When completing the type for a PARM_DECL, check for abstractness. Fix a typo in the comment. * typeck2.c (abstract_virtual_errors): Reword diagnostics, make them appear at the correct location. 2004-02-10 Giovanni Bajo <giovannibajo@gcc.gnu.org> PR c++/2204 * g++.dg/other/abstract2.C: New test. Index: decl.c =================================================================== RCS file: /cvs/gcc/gcc/gcc/cp/decl.c,v retrieving revision 1.1179 diff -c -3 -p -r1.1179 decl.c *** decl.c 2 Feb 2004 16:53:02 -0000 1.1179 --- decl.c 9 Feb 2004 23:27:43 -0000 *************** static void expand_static_init (tree, tr *** 120,125 **** --- 120,127 ---- static tree next_initializable_field (tree); static tree reshape_init (tree, tree *); static tree build_typename_type (tree, tree, tree); + static void maybe_register_incomplete_parm (tree); + static tree strip_array_and_pointer_to_array_types (tree); /* Erroneous argument lists can use this *IFF* they do not modify it. */ tree error_mark_list; *************** cp_finish_decl (tree decl, tree init, tr *** 4883,4901 **** || TREE_CODE (type) == METHOD_TYPE) abstract_virtuals_error (decl, strip_array_types (TREE_TYPE (type))); ! else if (POINTER_TYPE_P (type) || TREE_CODE (type) == ARRAY_TYPE) ! { ! /* If it's either a pointer or an array type, strip through all ! of them but the last one. If the last is an array type, issue ! an error if the element type is abstract. */ ! while (POINTER_TYPE_P (TREE_TYPE (type)) ! || TREE_CODE (TREE_TYPE (type)) == ARRAY_TYPE) ! type = TREE_TYPE (type); ! if (TREE_CODE (type) == ARRAY_TYPE) ! abstract_virtuals_error (decl, TREE_TYPE (type)); ! } ! else ! abstract_virtuals_error (decl, type); if (TREE_CODE (decl) == FUNCTION_DECL || TREE_TYPE (decl) == error_mark_node) --- 4885,4894 ---- || TREE_CODE (type) == METHOD_TYPE) abstract_virtuals_error (decl, strip_array_types (TREE_TYPE (type))); ! else ! abstract_virtuals_error (decl, ! strip_array_and_pointer_to_array_types ! (type)); if (TREE_CODE (decl) == FUNCTION_DECL || TREE_TYPE (decl) == error_mark_node) *************** grokparms (tree first_parm) *** 8669,8674 **** --- 8660,8667 ---- type = build_pointer_type (type); TREE_TYPE (decl) = type; } + else if (!COMPLETE_TYPE_P (type) || currently_open_class (type)) + maybe_register_incomplete_parm (decl); else if (abstract_virtuals_error (decl, type)) any_error = 1; /* Seems like a good idea. */ else if (POINTER_TYPE_P (type)) *************** finish_method (tree decl) *** 11049,11055 **** return decl; } ! \f /* VAR is a VAR_DECL. If its type is incomplete, remember VAR so that we can lay it out later, when and if its type becomes complete. */ --- 11042,11098 ---- return decl; } ! ! /* Strip all the array/pointer-to-array types from TYPE. This is used to ! extract the underlying type from declarations such as: ! ! A a[2]; ! A (*a)[2]; ! A (**a[2])[2]; ! ! which are invalid if 'A' is an abstract type. */ ! ! static tree ! strip_array_and_pointer_to_array_types (tree type) ! { ! if (POINTER_TYPE_P (type) || TREE_CODE (type) == ARRAY_TYPE) ! { ! /* If it's either a pointer or an array type, strip through all ! of them but the last one. If the last is an array type, then ! we can return its element type. */ ! while (POINTER_TYPE_P (TREE_TYPE (type)) ! || TREE_CODE (TREE_TYPE (type)) == ARRAY_TYPE) ! type = TREE_TYPE (type); ! if (TREE_CODE (type) == ARRAY_TYPE) ! return TREE_TYPE (type); ! } ! return type; ! } ! ! /* PARM is a PARM_DECL, whose type is incomplete at the point of parsing. ! We keep track of these because we must issue an error if, upon ! completition, the type ends up being abstract. */ ! ! static void ! maybe_register_incomplete_parm (tree var) ! { ! my_friendly_assert (TREE_CODE (var) == PARM_DECL, 20040208); ! ! /* If the type is dependent, there is nothing we can do here. The ! diagnostic will have to be issued during template substitution. */ ! if (!dependent_type_p (TREE_TYPE (var))) ! { ! tree inner_type = TREE_TYPE (var); ! ! /* FIXME: We currently don't get any ARRAY_TYPE because grokdeclarator ! decays them to POINTER_TYPE too early. */ ! inner_type = strip_array_and_pointer_to_array_types (inner_type); ! inner_type = TYPE_MAIN_VARIANT (inner_type); ! ! if (!COMPLETE_TYPE_P (inner_type) && CLASS_TYPE_P (inner_type)) ! incomplete_vars = tree_cons (inner_type, var, incomplete_vars); ! } ! } /* VAR is a VAR_DECL. If its type is incomplete, remember VAR so that we can lay it out later, when and if its type becomes complete. */ *************** maybe_register_incomplete_var (tree var) *** 11078,11084 **** } /* Called when a class type (given by TYPE) is defined. If there are ! any existing VAR_DECLs whose type hsa been completed by this declaration, update them now. */ void --- 11121,11127 ---- } /* Called when a class type (given by TYPE) is defined. If there are ! any existing VAR_DECLs whose type has been completed by this declaration, update them now. */ void *************** complete_vars (tree type) *** 11095,11100 **** --- 11138,11148 ---- /* Complete the type of the variable. The VAR_DECL itself will be laid out in expand_expr. */ complete_type (TREE_TYPE (var)); + /* If it is a PARM_DECL, we need to emit an error if the + type is abstract, because this is why we registered it + here. */ + if (TREE_CODE (var) == PARM_DECL) + abstract_virtuals_error (var, TREE_TYPE (var)); /* Remove this entry from the list. */ *list = TREE_CHAIN (*list); } Index: typeck2.c =================================================================== RCS file: /cvs/gcc/gcc/gcc/cp/typeck2.c,v retrieving revision 1.154 diff -c -3 -p -r1.154 typeck2.c *** typeck2.c 22 Jan 2004 00:03:52 -0000 1.154 --- typeck2.c 9 Feb 2004 23:27:46 -0000 *************** abstract_virtuals_error (tree decl, tree *** 149,182 **** return 0; if (TREE_CODE (decl) == VAR_DECL) ! error ("cannot declare variable `%D' to be of type `%T'", ! decl, type); else if (TREE_CODE (decl) == PARM_DECL) ! error ("cannot declare parameter `%D' to be of type `%T'", ! decl, type); else if (TREE_CODE (decl) == FIELD_DECL) ! error ("cannot declare field `%D' to be of type `%T'", ! decl, type); else if (TREE_CODE (decl) == FUNCTION_DECL && TREE_CODE (TREE_TYPE (decl)) == METHOD_TYPE) ! error ("invalid return type for member function `%#D'", decl); else if (TREE_CODE (decl) == FUNCTION_DECL) ! error ("invalid return type for function `%#D'", decl); } else ! error ("cannot allocate an object of type `%T'", type); /* Only go through this once. */ if (TREE_PURPOSE (u) == NULL_TREE) { TREE_PURPOSE (u) = error_mark_node; ! error (" because the following virtual functions are abstract:"); for (tu = u; tu; tu = TREE_CHAIN (tu)) ! cp_error_at ("\t%#D", TREE_VALUE (tu)); } else ! error (" since type `%T' has abstract virtual functions", type); return 1; } --- 149,196 ---- return 0; if (TREE_CODE (decl) == VAR_DECL) ! error ("%Jcannot declare variable `%D' to be of abstract type `%T'", ! decl, decl, type); else if (TREE_CODE (decl) == PARM_DECL) ! error ("%Jcannot declare parameter `%D' to be of abstract type `%T'", ! decl, decl, type); else if (TREE_CODE (decl) == FIELD_DECL) ! error ("%Jcannot declare field `%D' to be of abstract type `%T'", ! decl, decl, type); else if (TREE_CODE (decl) == FUNCTION_DECL && TREE_CODE (TREE_TYPE (decl)) == METHOD_TYPE) ! error ("%Jinvalid abstract return type for member function `%#D'", ! decl, decl); else if (TREE_CODE (decl) == FUNCTION_DECL) ! error ("%Jinvalid abstract return type for function `%#D'", ! decl, decl); } else ! error ("cannot allocate an object of abstract type `%T'", type); /* Only go through this once. */ if (TREE_PURPOSE (u) == NULL_TREE) { TREE_PURPOSE (u) = error_mark_node; ! if (decl) ! inform ("%J because the following virtual functions are pure:", ! decl); ! else ! inform (" because the following virtual functions are pure:"); ! for (tu = u; tu; tu = TREE_CHAIN (tu)) ! inform ("%J\t%#D", TREE_VALUE (tu), TREE_VALUE (tu)); } else ! { ! if (decl) ! inform ("%J since type `%T' has pure virtual functions", ! decl, type); ! else ! inform (" since type `%T' has pure virtual functions", ! type); ! } return 1; } // { dg-do compile } // Contributed by Gabriel Dos Reis <gdr at integrable-solutions dot net> // PR c++/2204: Check for parameters of abstract type in function declarations. namespace N1 { struct X; struct Y1 { void g(X parm1); // { dg-error "abstract" } void g(X parm2[2]); // { dg-error "abstract" "" { xfail *-*-* } } }; template <int N> struct Y2 { void g(X parm3); // { dg-error "abstract" } void g(X parm4[2]); // { dg-error "abstract" "" { xfail *-*-* } } }; struct X { virtual void xfunc(void) = 0; // { dg-error "note" } }; } namespace N2 { struct X1 { virtual void xfunc(void) = 0; // { dg-error "note" } void g(X1 parm5); // { dg-error "abstract" } void g(X1 parm6[2]); // { dg-error "abstract" "" { xfail *-*-* } } }; template <int N> struct X2 { virtual void xfunc(void) = 0; // { dg-error "note" "" { xfail *-*-* } } void g(X2 parm7); // { dg-error "abstract" "" { xfail *-*-* } } void g(X2 parm8[2]); // { dg-error "abstract" "" { xfail *-*-* } } }; } namespace N3 { struct X { virtual void xfunc(void) = 0; // { dg-error "note" } }; void g(X parm9); // { dg-error "abstract" } void g(X parm10[2]); // { dg-error "abstract" "" { xfail *-*-* } } template <int N> void g(X parm11); // { dg-error "abstract" } template <int N> void g(X parm12[2]); // { dg-error "abstract" "" { xfail *-*-* } } } // { dg-excess-errors "following virtual functions|has pure virtual functions" } ^ permalink raw reply [flat|nested] 46+ messages in thread
end of thread, other threads:[~2004-06-21 11:44 UTC | newest] Thread overview: 46+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2004-02-10 0:37 [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) Giovanni Bajo 2004-02-10 2:40 ` Gabriel Dos Reis 2004-02-10 2:47 ` Zack Weinberg 2004-02-10 2:50 ` Gabriel Dos Reis 2004-02-10 3:43 ` Zack Weinberg 2004-02-10 4:27 ` Gabriel Dos Reis 2004-02-10 4:31 ` Zack Weinberg 2004-02-10 4:58 ` Gabriel Dos Reis 2004-02-10 6:47 ` Zack Weinberg 2004-02-10 10:05 ` Gabriel Dos Reis 2004-02-10 10:36 ` Zack Weinberg 2004-02-10 10:57 ` Gabriel Dos Reis 2004-02-21 13:45 ` Gabriel Dos Reis 2004-02-21 13:45 ` Zack Weinberg 2004-02-21 13:45 ` Gabriel Dos Reis 2004-02-21 13:45 ` Zack Weinberg 2004-02-21 13:45 ` Gabriel Dos Reis 2004-02-21 13:45 ` Zack Weinberg 2004-02-21 13:45 ` Gabriel Dos Reis 2004-02-21 13:45 ` Zack Weinberg 2004-02-21 13:45 ` Gabriel Dos Reis 2004-02-21 13:45 ` Zack Weinberg 2004-02-10 3:12 ` Giovanni Bajo 2004-02-10 3:14 ` Gabriel Dos Reis 2004-02-10 17:50 ` [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) - Take 2 Giovanni Bajo 2004-02-10 19:24 ` Gabriel Dos Reis 2004-02-21 13:45 ` Gabriel Dos Reis 2004-02-21 13:45 ` Giovanni Bajo 2004-04-30 21:56 ` Jason Merrill 2004-06-11 13:52 ` [C++ PATCH] [PR2204] Check for parameters of abstract types - Take 3 Giovanni Bajo 2004-06-13 16:01 ` Mark Mitchell 2004-06-14 22:12 ` Jason Merrill 2004-06-15 2:40 ` Giovanni Bajo 2004-06-15 2:49 ` Mark Mitchell 2004-06-15 2:58 ` Giovanni Bajo 2004-06-15 3:04 ` Mark Mitchell 2004-06-15 3:06 ` Giovanni Bajo 2004-06-15 4:20 ` Jason Merrill 2004-06-21 14:37 ` [C++ PATCH] [PR2204] Check for parameters of abstract types - Take 4 Giovanni Bajo 2004-06-15 7:39 ` [C++ PATCH] [PR2204] Check for parameters of abstract types -Take 3 Gabriel Dos Reis 2004-06-15 7:39 ` [C++ PATCH] [PR2204] Check for parameters of abstract types - Take 3 Gabriel Dos Reis 2004-06-15 7:29 ` [C++ PATCH] [PR2204] Check for parameters of abstract types -Take 3 Gabriel Dos Reis 2004-02-21 13:45 ` [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) Gabriel Dos Reis 2004-02-21 13:45 ` Giovanni Bajo 2004-02-21 13:45 ` Gabriel Dos Reis 2004-02-21 13:45 ` Giovanni Bajo
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).