* [C++ RFC/Patch] PR 34938
@ 2014-08-22 17:53 Paolo Carlini
2014-08-22 18:17 ` Jason Merrill
0 siblings, 1 reply; 13+ messages in thread
From: Paolo Carlini @ 2014-08-22 17:53 UTC (permalink / raw)
To: gcc-patches; +Cc: Jason Merrill
[-- Attachment #1: Type: text/plain, Size: 694 bytes --]
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.
//////////////////////////
[-- Attachment #2: patch_34938 --]
[-- Type: text/plain, Size: 4041 bytes --]
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);
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [C++ RFC/Patch] PR 34938
2014-08-22 17:53 [C++ RFC/Patch] PR 34938 Paolo Carlini
@ 2014-08-22 18:17 ` Jason Merrill
2014-08-22 19:19 ` Paolo Carlini
0 siblings, 1 reply; 13+ messages in thread
From: Jason Merrill @ 2014-08-22 18:17 UTC (permalink / raw)
To: Paolo Carlini, gcc-patches
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [C++ RFC/Patch] PR 34938
2014-08-22 18:17 ` Jason Merrill
@ 2014-08-22 19:19 ` Paolo Carlini
2014-08-22 19:27 ` Jason Merrill
0 siblings, 1 reply; 13+ messages in thread
From: Paolo Carlini @ 2014-08-22 19:19 UTC (permalink / raw)
To: Jason Merrill, gcc-patches
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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [C++ RFC/Patch] PR 34938
2014-08-22 19:19 ` Paolo Carlini
@ 2014-08-22 19:27 ` Jason Merrill
2014-08-22 19:33 ` Paolo Carlini
0 siblings, 1 reply; 13+ messages in thread
From: Jason Merrill @ 2014-08-22 19:27 UTC (permalink / raw)
To: Paolo Carlini, gcc-patches
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [C++ RFC/Patch] PR 34938
2014-08-22 19:27 ` Jason Merrill
@ 2014-08-22 19:33 ` Paolo Carlini
2014-08-22 19:48 ` Manuel López-Ibáñez
2014-08-22 20:35 ` Paolo Carlini
0 siblings, 2 replies; 13+ messages in thread
From: Paolo Carlini @ 2014-08-22 19:33 UTC (permalink / raw)
To: Jason Merrill, gcc-patches
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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [C++ RFC/Patch] PR 34938
2014-08-22 19:33 ` Paolo Carlini
@ 2014-08-22 19:48 ` Manuel López-Ibáñez
2014-08-22 19:56 ` Jason Merrill
2014-08-22 20:35 ` Paolo Carlini
1 sibling, 1 reply; 13+ messages in thread
From: Manuel López-Ibáñez @ 2014-08-22 19:48 UTC (permalink / raw)
To: Paolo Carlini; +Cc: Jason Merrill, gcc-patches
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?
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [C++ RFC/Patch] PR 34938
2014-08-22 19:48 ` Manuel López-Ibáñez
@ 2014-08-22 19:56 ` Jason Merrill
0 siblings, 0 replies; 13+ messages in thread
From: Jason Merrill @ 2014-08-22 19:56 UTC (permalink / raw)
To: Manuel López-Ibáñez, Paolo Carlini; +Cc: gcc-patches
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [C++ RFC/Patch] PR 34938
2014-08-22 19:33 ` Paolo Carlini
2014-08-22 19:48 ` Manuel López-Ibáñez
@ 2014-08-22 20:35 ` Paolo Carlini
2014-08-22 20:45 ` Jason Merrill
1 sibling, 1 reply; 13+ messages in thread
From: Paolo Carlini @ 2014-08-22 20:35 UTC (permalink / raw)
To: Jason Merrill, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1625 bytes --]
Hi again,
On 08/22/2014 09:33 PM, Paolo Carlini wrote:
> 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?
Like the below, which I'm finishing testing?
Note that apparently this regressed in 4.9, after some clean ups and C++
work to the diagnostic committed by Gaby. Thus maybe eventually we want
to fix the issue in the release branch too.
Thanks,
Paolo.
PS: Something I also noticed is that we ignore the noreturn attribute in:
typedef void (A::*ptrmf)() __attribute((noreturn));
I don't remember a Bugzilla for that, but I suppose it should be fine.
clang doesn't complain for sure. And what about references?
typedef void (&fref)() __attribute((noreturn));
/////////////////////////
[-- Attachment #2: patch_34938_2 --]
[-- Type: text/plain, Size: 2936 bytes --]
Index: cp/cp-tree.h
===================================================================
--- cp/cp-tree.h (revision 214359)
+++ cp/cp-tree.h (working copy)
@@ -4728,7 +4728,8 @@ enum overload_flags { NO_SPECIAL = 0, DTOR_FLAG, T
TFF_NO_OMIT_DEFAULT_TEMPLATE_ARGUMENTS: do not omit template arguments
identical to their defaults.
TFF_NO_TEMPLATE_BINDINGS: do not print information about the template
- arguments for a function template specialization. */
+ arguments for a function template specialization.
+ TFF_POINTER: we are printing a pointer type. */
#define TFF_PLAIN_IDENTIFIER (0)
#define TFF_SCOPE (1)
@@ -4745,6 +4746,7 @@ enum overload_flags { NO_SPECIAL = 0, DTOR_FLAG, T
#define TFF_UNQUALIFIED_NAME (1 << 11)
#define TFF_NO_OMIT_DEFAULT_TEMPLATE_ARGUMENTS (1 << 12)
#define TFF_NO_TEMPLATE_BINDINGS (1 << 13)
+#define TFF_POINTER (1 << 14)
/* Returns the TEMPLATE_DECL associated to a TEMPLATE_TEMPLATE_PARM
node. */
Index: cp/cxx-pretty-print.h
===================================================================
--- cp/cxx-pretty-print.h (revision 214359)
+++ 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) \
+ pp_c_cv_qualifiers (PP, CV, FT)
#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 214359)
+++ cp/error.c (working copy)
@@ -820,6 +820,8 @@ dump_type_suffix (cxx_pretty_printer *pp, tree t,
if (TREE_CODE (TREE_TYPE (t)) == ARRAY_TYPE
|| TREE_CODE (TREE_TYPE (t)) == FUNCTION_TYPE)
pp_cxx_right_paren (pp);
+ if (TREE_CODE (t) == POINTER_TYPE)
+ flags |= TFF_POINTER;
dump_type_suffix (pp, TREE_TYPE (t), flags);
break;
@@ -839,7 +841,7 @@ 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), flags & TFF_POINTER);
dump_ref_qualifier (pp, t, flags);
dump_exception_spec (pp, TYPE_RAISES_EXCEPTIONS (t), flags);
dump_type_suffix (pp, TREE_TYPE (t), flags);
Index: testsuite/g++.dg/template/pr34938.C
===================================================================
--- testsuite/g++.dg/template/pr34938.C (revision 0)
+++ testsuite/g++.dg/template/pr34938.C (working copy)
@@ -0,0 +1,7 @@
+// PR c++/34938
+
+typedef void (*fptr)() __attribute((noreturn));
+template<int> void foo();
+template<fptr> void bar();
+
+fptr f = bar< foo<0> >; // { dg-error "noreturn" }
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [C++ RFC/Patch] PR 34938
2014-08-22 20:35 ` Paolo Carlini
@ 2014-08-22 20:45 ` Jason Merrill
2014-08-22 21:16 ` Paolo Carlini
0 siblings, 1 reply; 13+ messages in thread
From: Jason Merrill @ 2014-08-22 20:45 UTC (permalink / raw)
To: Paolo Carlini, gcc-patches
Does your patch handle this correctly?
template <class T> struct A { };
A<void()const>*p = 42;
Jason
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [C++ RFC/Patch] PR 34938
2014-08-22 20:45 ` Jason Merrill
@ 2014-08-22 21:16 ` Paolo Carlini
2014-08-23 7:16 ` Paolo Carlini
0 siblings, 1 reply; 13+ messages in thread
From: Paolo Carlini @ 2014-08-22 21:16 UTC (permalink / raw)
To: Jason Merrill, gcc-patches
Hi,
On 08/22/2014 10:45 PM, Jason Merrill wrote:
> Does your patch handle this correctly?
>
> template <class T> struct A { };
> A<void()const>*p = 42;
I would say yes:
34938_2.C:2:20: error: invalid conversion from âintâ to âA<void()
const>*â [-fpermissive]
A<void()const>*p = 42;
But, interestingly, in this case 4.8.x was incorrect, this specific case
isn't a 4.9 regression. I would definitely add this testcase too to the
testsuite.
Paolo.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [C++ RFC/Patch] PR 34938
2014-08-22 21:16 ` Paolo Carlini
@ 2014-08-23 7:16 ` Paolo Carlini
2014-08-23 15:03 ` Paolo Carlini
0 siblings, 1 reply; 13+ messages in thread
From: Paolo Carlini @ 2014-08-23 7:16 UTC (permalink / raw)
To: Jason Merrill, gcc-patches
.. on the other hand, the below is a case which my patchlet, which
simply tracks pointers, does *not* handle correctly:
struct B { };
template <class T> struct A { };
A<void(B::*)()const>*p = 42;
should be fixable by complicating a bit the tracking, telling apart
member pointers.
Paolo.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [C++ RFC/Patch] PR 34938
2014-08-23 7:16 ` Paolo Carlini
@ 2014-08-23 15:03 ` Paolo Carlini
2014-08-25 2:53 ` Jason Merrill
0 siblings, 1 reply; 13+ messages in thread
From: Paolo Carlini @ 2014-08-23 15:03 UTC (permalink / raw)
To: Jason Merrill, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1460 bytes --]
Hi again,
On 08/23/2014 09:16 AM, Paolo Carlini wrote:
> .. on the other hand, the below is a case which my patchlet, which
> simply tracks pointers, does *not* handle correctly:
>
> struct B { };
> template <class T> struct A { };
> A<void(B::*)()const>*p = 42;
>
> should be fixable by complicating a bit the tracking, telling apart
> member pointers.
Thus I finished testing the below. I separated to a new pr34938-2.C all
the cases which 4.9 is already getting right and on which we don't want
to regress.
As you can see, in order to handle correctly snippets like the above in
pr34938-2.C, I added to the pointer check a check TREE_CODE (t) ==
FUNCTION_TYPE, close to the original idea... For a while that made me a
little nervous because I was afraid that we would not print the
attribute for member function pointers when we should, but in fact, as
we ignore it for typedefs of such types (as I reported a couple of
messages ago), we also appear to drop to the floor when no typedefs are
involved, thus, eg we simply accept:
struct A { template<int> void foo(); };
template<void (A::*)() __attribute((noreturn))> void bar();
fptr f1 = bar< &A::foo<0> >;
and if we butcher it a little bit to force an error message (eg, we
change A::foo to be const) there is no trace of the attribute in the
error message anyway. Thus I *think* that given our current status
elsewhere at least, we are fine.
Thanks,
Paolo.
////////////////////////
[-- Attachment #2: patch_34938_3 --]
[-- Type: text/plain, Size: 3628 bytes --]
Index: cp/cp-tree.h
===================================================================
--- cp/cp-tree.h (revision 214396)
+++ cp/cp-tree.h (working copy)
@@ -4728,7 +4728,8 @@ enum overload_flags { NO_SPECIAL = 0, DTOR_FLAG, T
TFF_NO_OMIT_DEFAULT_TEMPLATE_ARGUMENTS: do not omit template arguments
identical to their defaults.
TFF_NO_TEMPLATE_BINDINGS: do not print information about the template
- arguments for a function template specialization. */
+ arguments for a function template specialization.
+ TFF_POINTER: we are printing a pointer type. */
#define TFF_PLAIN_IDENTIFIER (0)
#define TFF_SCOPE (1)
@@ -4745,6 +4746,7 @@ enum overload_flags { NO_SPECIAL = 0, DTOR_FLAG, T
#define TFF_UNQUALIFIED_NAME (1 << 11)
#define TFF_NO_OMIT_DEFAULT_TEMPLATE_ARGUMENTS (1 << 12)
#define TFF_NO_TEMPLATE_BINDINGS (1 << 13)
+#define TFF_POINTER (1 << 14)
/* Returns the TEMPLATE_DECL associated to a TEMPLATE_TEMPLATE_PARM
node. */
Index: cp/cxx-pretty-print.h
===================================================================
--- cp/cxx-pretty-print.h (revision 214396)
+++ 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) \
+ pp_c_cv_qualifiers (PP, CV, FT)
#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 214396)
+++ cp/error.c (working copy)
@@ -820,6 +820,8 @@ dump_type_suffix (cxx_pretty_printer *pp, tree t,
if (TREE_CODE (TREE_TYPE (t)) == ARRAY_TYPE
|| TREE_CODE (TREE_TYPE (t)) == FUNCTION_TYPE)
pp_cxx_right_paren (pp);
+ if (TREE_CODE (t) == POINTER_TYPE)
+ flags |= TFF_POINTER;
dump_type_suffix (pp, TREE_TYPE (t), flags);
break;
@@ -839,7 +841,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
+ && (flags & TFF_POINTER));
dump_ref_qualifier (pp, t, flags);
dump_exception_spec (pp, TYPE_RAISES_EXCEPTIONS (t), flags);
dump_type_suffix (pp, TREE_TYPE (t), flags);
Index: testsuite/g++.dg/template/pr34938-1.C
===================================================================
--- testsuite/g++.dg/template/pr34938-1.C (revision 0)
+++ testsuite/g++.dg/template/pr34938-1.C (working copy)
@@ -0,0 +1,7 @@
+// PR c++/34938
+
+typedef void (*fptr)() __attribute((noreturn));
+template<int> void foo();
+template<fptr> void bar();
+
+fptr f = bar< foo<0> >; // { dg-error "noreturn" }
Index: testsuite/g++.dg/template/pr34938-2.C
===================================================================
--- testsuite/g++.dg/template/pr34938-2.C (revision 0)
+++ testsuite/g++.dg/template/pr34938-2.C (working copy)
@@ -0,0 +1,10 @@
+// PR c++/34938
+
+template <class T> struct A { };
+struct B { };
+
+A<void()const>* p1 = 42; // { dg-error "void\\(\\) const" }
+A<void(B::*)()const>* p2 = 42; // { dg-error "void \\(B::\\*\\)\\(\\) const" }
+
+A<void()volatile>* p3 = 42; // { dg-error "void\\(\\) volatile" }
+A<void(B::*)()volatile>* p4 = 42; // { dg-error "void \\(B::\\*\\)\\(\\) volatile" }
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [C++ RFC/Patch] PR 34938
2014-08-23 15:03 ` Paolo Carlini
@ 2014-08-25 2:53 ` Jason Merrill
0 siblings, 0 replies; 13+ messages in thread
From: Jason Merrill @ 2014-08-25 2:53 UTC (permalink / raw)
To: Paolo Carlini, gcc-patches
OK.
Jason
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2014-08-25 2:53 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-22 17:53 [C++ RFC/Patch] PR 34938 Paolo Carlini
2014-08-22 18:17 ` Jason Merrill
2014-08-22 19:19 ` Paolo Carlini
2014-08-22 19:27 ` Jason Merrill
2014-08-22 19:33 ` Paolo Carlini
2014-08-22 19:48 ` Manuel López-Ibáñez
2014-08-22 19:56 ` Jason Merrill
2014-08-22 20:35 ` Paolo Carlini
2014-08-22 20:45 ` Jason Merrill
2014-08-22 21:16 ` Paolo Carlini
2014-08-23 7:16 ` Paolo Carlini
2014-08-23 15:03 ` Paolo Carlini
2014-08-25 2:53 ` Jason Merrill
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).