Doug, I'm sorry, can you have a look to this one too? I'm slightly confused, we have PR 36052, and then PR 30601, ... Seems that the warning is intended?!? Do we have to change bind?!?

By the way, apparently, even after the fix for c++/32256 and c++/32368, warnings keep coming from inside the system headers... Any idea why, Tom? Another case would be this one:

  http://gcc.gnu.org/ml/libstdc++/2008-06/msg00080.html

Is it because these warnings are really spuriously coming from the middle-end per the audit trail of the latter issue, PR 36633 ?

Thus, assuming we generically want this warning (as I understand from the audit trails of PR 30601), the real issue is fixing the C++ front-end to not pass bogus stuff to the middle-end? Or the two issues are different?

One final remark: if we add -Wsystem-headers to the command line *many* more type qualifiers ignored warning are emitted, a tiny part somehow escapes...

Or this pure C++ case is really just a missing TREE_NO_WARNING set somewhere (check_return_expr?)

My guess is that comment #3 is the right theory, because
this warning is issued from the front end.  I did not
investigate deeply though.

Thanks Tom. In fact, yesterday I was writing without remembering my past analyses of this type of issue, with system header warnings not suppressed: TREE_NO_WARNING is *not* generically uses for that. Everything boils down to DECL_IN_SYSTEM_HEADER  on the decl instead.

Now, the warning at issue is the *only* one directly emitted from pt.c and the following draft works for me.

What do you think, Mark?

Otherwise, elsewhere we are dealing with these issues by going through the saved_in_system_header dance...

Index: pt.c
===================================================================
*** pt.c        (revision 137591)
--- pt.c        (working copy)
*************** tsubst_function_type (tree t,
*** 8768,8773 ****
--- 8768,8774 ----
    if (TYPE_QUALS (return_type) != TYPE_UNQUALIFIED
        && in_decl != NULL_TREE
        && !TREE_NO_WARNING (in_decl)
+       && !DECL_IN_SYSTEM_HEADER (in_decl)
        && (SCALAR_TYPE_P (return_type) || VOID_TYPE_P (return_type)))
      warning (OPT_Wignored_qualifiers,
              "type qualifiers ignored on function return type");

Subject: Re:  Simple std::bind use causes warnings with -Wextra

paolo dot carlini at oracle dot com wrote:

> Thanks Tom. In fact, yesterday I was writing without remembering my past
> analyses of this type of issue, with system header warnings not suppressed:
> TREE_NO_WARNING is *not* generically uses for that. Everything boils down to
> DECL_IN_SYSTEM_HEADER  on the decl instead.

Why is it reasonable for a libstdc++ header to return a cv-qualified 
type, but not for user code to do so?

In general, the system-header hack is to work around things we don't 
control; we need to accept weird code in <stdio.h> because the OS 
distributor controls that, not us.  But, if it's not practical for 
libstdc++ to avoid this warning, then it's probably not practical for 
users to avoid it either, so then I wonder how beneficial the warning 
is.  On the other hand, I'd expect that libstdc++ could just avoid the 
warning, by using a traits class to strip the cv-qualifier?

Thanks,

(In reply to comment #6)
> Subject: Re:  Simple std::bind use causes warnings with -Wextra
> 
> paolo dot carlini at oracle dot com wrote:
> 
> > Thanks Tom. In fact, yesterday I was writing without remembering my past
> > analyses of this type of issue, with system header warnings not suppressed:
> > TREE_NO_WARNING is *not* generically uses for that. Everything boils down to
> > DECL_IN_SYSTEM_HEADER  on the decl instead.
> 
> Why is it reasonable for a libstdc++ header to return a cv-qualified 
> type, but not for user code to do so?
> 
> In general, the system-header hack is to work around things we don't 
> control; we need to accept weird code in <stdio.h> because the OS 
> distributor controls that, not us.  But, if it's not practical for 
> libstdc++ to avoid this warning, then it's probably not practical for 
> users to avoid it either, so then I wonder how beneficial the warning 
> is.  On the other hand, I'd expect that libstdc++ could just avoid the 
> warning, by using a traits class to strip the cv-qualifier?

I'm sorry Mark, I'm not sure to understand.

We are working under the assumption (result of past discussions) that, in general, we do want to warn for return types which are CV-qualified void and builtin types. Ok.

Now, we can certainly make the library a tad more complex and strip those CV-qualifiers. Probably we'll do that, anyway, I'd like to ear Doug, the original author, about it.

On the other hand, we have the #pragma for system headers which is currently able to suppress *any* warning besides this one, having to do with return types. Seems a separate question to me. It looks to me that the pragma should suppress this warning too. Do you agree?

A final remark: for better and worse, our library is not currently -Wsystem-headers clean, not only because we don't have control over the underlying C library (that makes for a separate issue, rather annoying sometimes) but also for other reasons, for example, at times, suppressing any possible false positive would make the code much more complex (and I think we all agree that we absolutely want to avoid warnings from inside the headers), uncertainty about whether we do want a warning or not (e.g., the library would not warn if a core DR is implemented), etc.

Paolo --

You're absolutely right; I'd not thought about the orthogonality between what the library should do and how the compiler should behave.  I agree that the patch is correct; go ahead check it in.

I was also trying to raise the issue of whether we think the warning is useful.  If it's not practical to avoid the warning in the library, then I wonder if it's practical to avoid it other generic-programming code.  But, I agree that this is a separate issue.

Thanks,

-- Mark

Subject: Bug 36760

Author: paolo
Date: Wed Jul  9 15:45:50 2008
New Revision: 137660

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=137660
Log:
/cp
2008-07-09  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/36760
	* pt.c (tsubst_function_type): Don't warn for type qualifiers
	on function return type in case of system header.

/testsuite
2008-07-09  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/36760
	* g++.dg/warn/pragma-system_header5.C: New.
	* g++.dg/warn/pragma-system_header5.h: Likewise.

Added:
    trunk/gcc/testsuite/g++.dg/warn/pragma-system_header5.C
    trunk/gcc/testsuite/g++.dg/warn/pragma-system_header5.h
Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/pt.c
    trunk/gcc/testsuite/ChangeLog

(In reply to comment #8)
> I was also trying to raise the issue of whether we think the warning is useful.
>  If it's not practical to avoid the warning in the library, then I wonder if
> it's practical to avoid it other generic-programming code.

I agree with this. As I mentioned in PR 30601, code like this

  template <typename T> class ArrayView {
    T& operator();
    T operator() const;
  };

is quite common and I don't see a need to make it more complicated than
necessary just for a warning. Yet, this class will trigger a warning if
instantiated as ArrayView<const double> because the return type of the second
operator() is now 'const double'. (Think of the class as providing a view
to a part of an array, so instantiating it with T=const double provides a
view that does not allow the elements to be modified.)

Personally, I find the fact that the compiler now warns about this very
annoying indeed.

W.

Well I didn't want to reopen a discussion which I considered closed. But yes, I also agree. And in case, we should have the change in 4_3-branch too. 

And, by the way, I'm noticing the hard way that return by reference is the most annoying case: if you want to suppress the warnings still deal appropriately with references, it's a huge special casing everywhere, sigh.

Subject: Re:  Simple std::bind use causes warnings with -Wextra

bangerth at dealii dot org wrote:
> ------- Comment #10 from bangerth at dealii dot org  2008-07-09 17:04 -------
> (In reply to comment #8)
>> I was also trying to raise the issue of whether we think the warning is useful.
>>  If it's not practical to avoid the warning in the library, then I wonder if
>> it's practical to avoid it other generic-programming code.
> 
> I agree with this. As I mentioned in PR 30601, code like this
> 
>   template <typename T> class ArrayView {
>     T& operator();
>     T operator() const;
>   };
> 
> is quite common and I don't see a need to make it more complicated than
> necessary just for a warning.

Me neither.  I think writing:

   const int f();

or:

   template <typename T>
   const int f(T);

is probably worth warning about, but maybe we ought to just skip this 
warning when instantiating a template function.  In other words, warn at 
the point of original declaration of the template if it is already 
obviously meaningless at that point to add the cv-qualifier, but not 
warn at instantiation.

(In reply to comment #13)
> is probably worth warning about, but maybe we ought to just skip this 
> warning when instantiating a template function.  In other words, warn at 
> the point of original declaration of the template if it is already 
> obviously meaningless at that point to add the cv-qualifier, but not 
> warn at instantiation.

Ah, I didn't consider this option! Seems also trivial to implement, just remove completely the pt.c version of the warning and keep the one in decl.c. Then, over the next hours, if I don't see objections, I'm going to implement and post it, proposing it for 4_3-branch too, because of the very annoying warning from <functional> per this PR...

(In reply to comment #14)
> Ah, I didn't consider this option! Seems also trivial to implement, just remove
> completely the pt.c version of the warning and keep the one in decl.c. Then,
> over the next hours, if I don't see objections, I'm going to implement and post
> it, proposing it for 4_3-branch too, because of the very annoying warning from
> <functional> per this PR...

I think that would be awesome!
W.

Humm, this case

  template <typename T>
  const int i(T);

is a little more tricky, let's what I can do...

Ignore my last message, I had a spurious #pragma system header in my experiments. Great ;)

Subject: Bug 36760

Author: paolo
Date: Wed Jul  9 20:52:45 2008
New Revision: 137672

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=137672
Log:
/cp
2008-07-09  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/36760
	* pt.c (tsubst_function_type): Remove warning for type qualifiers
	on function return type.

/testsuite
2008-07-09  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/36760
	* g++.dg/warn/Wreturn-type-4.C: Adjust.

Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/pt.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/g++.dg/warn/Wreturn-type-4.C

Fixed for 4.4.0.

(In reply to comment #19)
> Fixed for 4.4.0.

Thanks a lot!

Two questions:
1/ Is the text in the documentation that Dirk Mueller added in the last commit
   of PR 30601 now wrong/outdated?
2/ Does your patch also fix the testcase in PR 36052?

Thanks
 W.

> Two questions:
> 1/ Is the text in the documentation that Dirk Mueller added in the last commit
>    of PR 30601 now wrong/outdated?

I don't know, I'm not a documentation expert, maybe some tweaks will be necessary but I may not be able to come to it in the near future, really quite a few things in my todo list these days, I'm sorry.

> 2/ Does your patch also fix the testcase in PR 36052?

The warning is not emitted anymore, for sure.

(In reply to comment #21)
> > Two questions:
> > 1/ Is the text in the documentation that Dirk Mueller added in the last commit
> >    of PR 30601 now wrong/outdated?
> 
> I don't know, I'm not a documentation expert, maybe some tweaks will be
> necessary but I may not be able to come to it in the near future, really quite
> a few things in my todo list these days, I'm sorry.

Reading through the current text, I think it is sufficiently vague for it
to remain as it is.


> > 2/ Does your patch also fix the testcase in PR 36052?
> 
> The warning is not emitted anymore, for sure.

Great, thanks a lot!
W.