public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [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).