| Message ID | 53F78398.2000908@oracle.com |
|---|---|
| State | New |
| Headers | show |
On 08/22/2014 01:53 PM, Paolo Carlini wrote: > maybe this old issue is already fixed. We used to ICE on: > > typedef void (*fptr)() __attribute((noreturn)); > template<int> void foo(); > template<fptr> void bar(); > > fptr f = bar< foo<0> >; > > but lately we simply reject it: > > 34938.C:5:10: error: no matches converting function ‘bar’ to type ‘fptr > {aka void (*)() volatile}’ > fptr f = bar< foo<0> >; > ^ > 34938.C:3:21: note: candidate is: template<void (* <anonymous>)() > volatile> void bar() > template<fptr> void bar(); > ^ > > is that Ok? clang behaves like us, EDG accepts the code. Well, since the attribute is outside the language, I guess we get to decide what it means; C++11 [[noreturn]] is only defined on decls, not types. I think rejecting it makes sense; since bar is not declared as noreturn, assigning its address to a noreturn function pointer seems wrong. > A secondary > issue I noticed is that we print 'volatile' instead of the attribute, > that is fixed by the patchlet below. Currently function-cv-quals on a FUNCTION_TYPE used as a typedef or template parameter have the same encoding as the const and noreturn attributes; to get the printing right you need to know the context of the FUNCTION_TYPE. If it's the type of a pointer, then the attribute is correct. Jason
Hi, On 08/22/2014 08:17 PM, Jason Merrill wrote: > On 08/22/2014 01:53 PM, Paolo Carlini wrote: >> maybe this old issue is already fixed. We used to ICE on: >> >> typedef void (*fptr)() __attribute((noreturn)); >> template<int> void foo(); >> template<fptr> void bar(); >> >> fptr f = bar< foo<0> >; >> >> but lately we simply reject it: >> >> 34938.C:5:10: error: no matches converting function ‘bar’ to type ‘fptr >> {aka void (*)() volatile}’ >> fptr f = bar< foo<0> >; >> ^ >> 34938.C:3:21: note: candidate is: template<void (* <anonymous>)() >> volatile> void bar() >> template<fptr> void bar(); >> ^ >> >> is that Ok? clang behaves like us, EDG accepts the code. > > Well, since the attribute is outside the language, I guess we get to > decide what it means; C++11 [[noreturn]] is only defined on decls, not > types. > > I think rejecting it makes sense; since bar is not declared as > noreturn, assigning its address to a noreturn function pointer seems > wrong. Ok, thanks. Good. >> A secondary >> issue I noticed is that we print 'volatile' instead of the attribute, >> that is fixed by the patchlet below. > > Currently function-cv-quals on a FUNCTION_TYPE used as a typedef or > template parameter have the same encoding as the const and noreturn > attributes; Indeed, this is also my understanding per pp_c_cv_qualifiers. > to get the printing right you need to know the context of the > FUNCTION_TYPE. If it's the type of a pointer, then the attribute is > correct. Ok. Currently in cases like the present one, dump_type_suffix upon a pointer recurses and we end up calling pp_cxx_cv_qualifiers on the given FUNCTION_TYPE / METHOD_TYPE. Thus pp_cxx_cv_qualifiers lacks the pointer context, just sees the latter. Do you think that the current simple setup, thus my patch which just extends it, can be incorrect in some cases? Thanks, Paolo.
On 08/22/2014 03:19 PM, Paolo Carlini wrote: > Ok. Currently in cases like the present one, dump_type_suffix upon a > pointer recurses and we end up calling pp_cxx_cv_qualifiers on the given > FUNCTION_TYPE / METHOD_TYPE. Thus pp_cxx_cv_qualifiers lacks the pointer > context, just sees the latter. Do you think that the current simple > setup, thus my patch which just extends it, can be incorrect in some cases? Yes, I think your patch changes it to be incorrect in different cases than the ones where it's currently incorrect, namely the typedef and template argument cases that I mentioned. Incidentally, I don't understand > + pp_c_ws_string (pp, (func_type && !method_type vs > + pp_c_ws_string (pp, (func_type || method_type Surely the same logic is appropriate for both const and noreturn, and they are represented the same way on both function_ and method_type. Jason
Hi, On 08/22/2014 09:27 PM, Jason Merrill wrote: > On 08/22/2014 03:19 PM, Paolo Carlini wrote: >> Ok. Currently in cases like the present one, dump_type_suffix upon a >> pointer recurses and we end up calling pp_cxx_cv_qualifiers on the given >> FUNCTION_TYPE / METHOD_TYPE. Thus pp_cxx_cv_qualifiers lacks the pointer >> context, just sees the latter. Do you think that the current simple >> setup, thus my patch which just extends it, can be incorrect in some >> cases? > > Yes, I think your patch changes it to be incorrect in different cases > than the ones where it's currently incorrect, namely the typedef and > template argument cases that I mentioned. Let me think about this. I'm not sure to understand what this boils down to, if we have to seriously restructure what we have got or not. Do you think that we have to track during the recursion in dump_type_suffix that the FUNCTION_TYPE / METHOD_TYPE comes from a pointer? > Incidentally, I don't understand > >> + pp_c_ws_string (pp, (func_type && !method_type > vs >> + pp_c_ws_string (pp, (func_type || method_type > > Surely the same logic is appropriate for both const and noreturn, and > they are represented the same way on both function_ and method_type. Ah, Ok, now I see, it's just that volatile member functions aren't *that* common ;) Paolo.
On 22 August 2014 21:33, Paolo Carlini <paolo.carlini@oracle.com> wrote: >> Incidentally, I don't understand >> >>> + pp_c_ws_string (pp, (func_type && !method_type >> >> vs >>> >>> + pp_c_ws_string (pp, (func_type || method_type >> >> >> Surely the same logic is appropriate for both const and noreturn, and they >> are represented the same way on both function_ and method_type. > > Ah, Ok, now I see, it's just that volatile member functions aren't *that* > common ;) Are there actually cases where the qualifiers mean different things for function_type and method_type?
On 08/22/2014 03:47 PM, Manuel López-Ibáñez wrote: > Are there actually cases where the qualifiers mean different things > for function_type and method_type? If a FUNCTION_TYPE is a typedef or template argument, TYPE_READONLY and TYPE_VOLATILE are the function-cv-quals. A plain METHOD_TYPE cannot appear in those contexts. In all other contexts, TYPE_READONLY and TYPE_VOLATILE are attributes const and noreturn. I've tried to move the function-cv-quals out of those flags a couple of times and given up. Jason
Index: c/c-objc-common.c =================================================================== --- c/c-objc-common.c (revision 214335) +++ c/c-objc-common.c (working copy) @@ -165,7 +165,8 @@ c_tree_printer (pretty_printer *pp, text_info *tex return true; case 'v': - pp_c_cv_qualifiers (cpp, va_arg (*text->args_ptr, int), hash); + pp_c_cv_qualifiers (cpp, va_arg (*text->args_ptr, int), hash, + /*method_type*/false); return true; default: Index: c-family/c-pretty-print.c =================================================================== --- c-family/c-pretty-print.c (revision 214335) +++ c-family/c-pretty-print.c (working copy) @@ -168,7 +168,8 @@ pp_c_exclamation (c_pretty_printer *pp) /* Print out the external representation of QUALIFIERS. */ void -pp_c_cv_qualifiers (c_pretty_printer *pp, int qualifiers, bool func_type) +pp_c_cv_qualifiers (c_pretty_printer *pp, int qualifiers, + bool func_type, bool method_type) { const char *p = pp_last_position_in_text (pp); bool previous = false; @@ -192,7 +193,8 @@ void { if (previous) pp_c_whitespace (pp); - pp_c_ws_string (pp, func_type ? "__attribute__((const))" : "const"); + pp_c_ws_string (pp, (func_type && !method_type + ? "__attribute__((const))" : "const")); previous = true; } @@ -200,7 +202,8 @@ void { if (previous) pp_c_whitespace (pp); - pp_c_ws_string (pp, func_type ? "__attribute__((noreturn))" : "volatile"); + pp_c_ws_string (pp, (func_type || method_type + ? "__attribute__((noreturn))" : "volatile")); previous = true; } @@ -273,7 +276,8 @@ pp_c_type_qualifier_list (c_pretty_printer *pp, tr qualifiers = TYPE_QUALS (t); pp_c_cv_qualifiers (pp, qualifiers, - TREE_CODE (t) == FUNCTION_TYPE); + TREE_CODE (t) == FUNCTION_TYPE, + TREE_CODE (t) == METHOD_TYPE); if (!ADDR_SPACE_GENERIC_P (TYPE_ADDR_SPACE (t))) { Index: c-family/c-pretty-print.h =================================================================== --- c-family/c-pretty-print.h (revision 214335) +++ c-family/c-pretty-print.h (working copy) @@ -119,7 +119,8 @@ void pp_c_tree_decl_identifier (c_pretty_printer * void pp_c_function_definition (c_pretty_printer *, tree); void pp_c_attributes (c_pretty_printer *, tree); void pp_c_attributes_display (c_pretty_printer *, tree); -void pp_c_cv_qualifiers (c_pretty_printer *pp, int qualifiers, bool func_type); +void pp_c_cv_qualifiers (c_pretty_printer *pp, int qualifiers, + bool func_type, bool method_type); void pp_c_type_qualifier_list (c_pretty_printer *, tree); void pp_c_parameter_type_list (c_pretty_printer *, tree); void pp_c_specifier_qualifier_list (c_pretty_printer *, tree); Index: cp/cxx-pretty-print.h =================================================================== --- cp/cxx-pretty-print.h (revision 214335) +++ cp/cxx-pretty-print.h (working copy) @@ -59,8 +59,8 @@ struct cxx_pretty_printer : c_pretty_printer #define pp_cxx_cv_qualifier_seq(PP, T) \ pp_c_type_qualifier_list (PP, T) -#define pp_cxx_cv_qualifiers(PP, CV) \ - pp_c_cv_qualifiers (PP, CV, false) +#define pp_cxx_cv_qualifiers(PP, CV, FT, MT) \ + pp_c_cv_qualifiers (PP, CV, FT, MT) #define pp_cxx_whitespace(PP) pp_c_whitespace (PP) #define pp_cxx_left_paren(PP) pp_c_left_paren (PP) Index: cp/error.c =================================================================== --- cp/error.c (revision 214335) +++ cp/error.c (working copy) @@ -839,7 +839,9 @@ dump_type_suffix (cxx_pretty_printer *pp, tree t, dump_parameters (pp, arg, flags & ~TFF_FUNCTION_DEFAULT_ARGUMENTS); pp->padding = pp_before; - pp_cxx_cv_qualifiers (pp, type_memfn_quals (t)); + pp_cxx_cv_qualifiers (pp, type_memfn_quals (t), + TREE_CODE (t) == FUNCTION_TYPE, + TREE_CODE (t) == METHOD_TYPE); dump_ref_qualifier (pp, t, flags); dump_exception_spec (pp, TYPE_RAISES_EXCEPTIONS (t), flags); dump_type_suffix (pp, TREE_TYPE (t), flags);
Hi, maybe this old issue is already fixed. We used to ICE on: typedef void (*fptr)() __attribute((noreturn)); template<int> void foo(); template<fptr> void bar(); fptr f = bar< foo<0> >; but lately we simply reject it: 34938.C:5:10: error: no matches converting function ‘bar’ to type ‘fptr {aka void (*)() volatile}’ fptr f = bar< foo<0> >; ^ 34938.C:3:21: note: candidate is: template<void (* <anonymous>)() volatile> void bar() template<fptr> void bar(); ^ is that Ok? clang behaves like us, EDG accepts the code. A secondary issue I noticed is that we print 'volatile' instead of the attribute, that is fixed by the patchlet below. Thanks, Paolo. //////////////////////////