Re: [RFC] missing return warnings
On 06/09/07, Jan Hubicka [EMAIL PROTECTED] wrote:
I wonder what we want to do here - I guess we can either make the
warning unconditional and declare it as two indpendent things or we can
just drop the feature since user will get properly warned on every
function he actually uses.
What would be preferred solution here?
My preferred solution would be that TREE_NO_WARNING did actually
prevent to emit a duplicate warning.
tree-cfg.c (execute_warn_function_return)
--
/* If we see return; in some basic block, then we do reach the end
without returning a value. */
else if (warn_return_type
!TREE_NO_WARNING (cfun-decl)
EDGE_COUNT (EXIT_BLOCK_PTR-preds) 0
!VOID_TYPE_P (TREE_TYPE (TREE_TYPE (cfun-decl
{
FOR_EACH_EDGE (e, ei, EXIT_BLOCK_PTR-preds)
{
tree last = last_stmt (e-src);
if (TREE_CODE (last) == RETURN_EXPR
TREE_OPERAND (last, 0) == NULL
!TREE_NO_WARNING (last))
{
#ifdef USE_MAPPED_LOCATION
location = EXPR_LOCATION (last);
if (location == UNKNOWN_LOCATION)
location = cfun-function_end_locus;
warning (0, %Hcontrol reaches end of non-void function,
location);
#else
locus = EXPR_LOCUS (last);
if (!locus)
locus = cfun-function_end_locus;
warning (0, %Hcontrol reaches end of non-void function, locus);
#endif
TREE_NO_WARNING (cfun-decl) = 1;
break;
}
}
}
Why is that not so? That would also prevent the whole loop from being
executed at all. Do cfun-decl and fndecl point to different things?
If that is difficult then not getting a warning about a function that
is not used does not seem so tragic as long as the middle-end warns
for every case that the front-end would warn (assuming the function is
used). The warning in the middle-end does not depend on optimization
being enabled, does it?
Cheers,
Manuel.
Re: [RFC] missing return warnings
Jan Hubicka wrote:
For C++ there are number of cases I need to go through dealing with
attempts to not instantiate non-inline methods that won't be needed.
I'm not sure exactly what cases you're referring to, but please be
careful. For example, changing if/when we instantiate template
functions, or if/when we generate bodies for implicitly defined
constructors, destructors, or assignment operators would be a semantic
change, and contrary to the standard.
For C it seems to be single case for return warning:
/* PR c++/4872 */
/* { dg-do compile } */
/* { dg-options -Wreturn-type } */
static inline int f() {} /* { dg-warning return missing return } */
In this case we produce missing return warning from frontend.
This however is not truly consistent - if I make the function used, we
actually get two warnings - one from frontend, one from middle end
(that is CFG aware and does not merely check for presence of return
statement in function).
As you know, I don't like the middle end issuing warnings at all. So,
you'll not be surprised that I think we should warn about this function
independent of optimization. People want these warnings to tell them
about possible bugs in their software, not just the ones that affect
them in their current configuration, depending on how much optimization
applies, etc. If we want the middle end to warn about this kind of
case, we should make sure that it does so for all functions, used or not.
--
Mark Mitchell
CodeSourcery
[EMAIL PROTECTED]
(650) 331-3385 x713
Re: [RFC] missing return warnings
Jan Hubicka wrote: I am basically trying to avoid need for DECL_INLINE (while keeping DECL_DECLARED_INLINE and DECL_UNINLINABLE for frontend use) - inliner is now doing all the job himself to figure out what is or isn't needed. That certainly makes sense to me. With the noreturn warning made unconditional: ... With the noreturn warning disabled completely: ... So it seems to be all about those warnings now. OK, that sounds pretty encouraging. Thansk, -- Mark Mitchell CodeSourcery [EMAIL PROTECTED] (650) 331-3385 x713
Re: [RFC] missing return warnings
Jan Hubicka wrote:
For C++ there are number of cases I need to go through dealing with
attempts to not instantiate non-inline methods that won't be needed.
I'm not sure exactly what cases you're referring to, but please be
careful. For example, changing if/when we instantiate template
functions, or if/when we generate bodies for implicitly defined
constructors, destructors, or assignment operators would be a semantic
change, and contrary to the standard.
I am basically trying to avoid need for DECL_INLINE (while keeping
DECL_DECLARED_INLINE and DECL_UNINLINABLE for frontend use) - inliner is
now doing all the job himself to figure out what is or isn't needed.
There seems to be whole a lot of legacy code that is now wrong and/or
unnecesary (such as code preventing optimization of stack assignments on
inlinable functions to aid inliner or a lot of code assuming that only
inlinable functions are deferred).
We still should care about DECL_DECLARED_INLINE, if it affects semantics
of standard, but I would hope the output from frontends to actually not
depend on -finline-functions flag if possible. I am experimenting with
the attached patch, it adds few failures:
With the noreturn warning made unconditional:
g++.dg/warn/pr23075.C (test for excess errors)
g++.old-deja/g++.brendan/crash52.C (test for excess errors)
g++.old-deja/g++.jason/report.C (test for excess errors)
gcc.dg/20040206-1.c (test for warnings, line 10)
gcc.dg/20040206-1.c (test for excess errors)
gcc.dg/pr23075.c (test for warnings, line 14)
gcc.dg/pr23075.c (test for excess errors)
gcc.dg/return-type-1.c (test for excess errors)
gcc.dg/return-type-1.c warning for falling off end of non-void function
(test for warnings, line 9)
gcc.dg/return-type-3.c (test for excess errors)
gcc.dg/return-type-3.c warning for falling off end of non-void function
(test for warnings, line 14)
With the noreturn warning disabled completely:
g++.dg/warn/noreturn-2.C (test for warnings, line 4)
g++.old-deja/g++.bugs/900205_03.C (test for errors, line 29)
g++.old-deja/g++.bugs/900205_03.C (test for errors, line 32)
g++.old-deja/g++.law/operators17.C (test for warnings, line 11)
gcc.dg/Wreturn-type.c (test for excess errors)
gcc.dg/Wreturn-type.c missing return (test for warnings, line 5)
So it seems to be all about those warnings now.
For C it seems to be single case for return warning:
/* PR c++/4872 */
/* { dg-do compile } */
/* { dg-options -Wreturn-type } */
static inline int f() {} /* { dg-warning return missing return } */
In this case we produce missing return warning from frontend.
This however is not truly consistent - if I make the function used, we
actually get two warnings - one from frontend, one from middle end
(that is CFG aware and does not merely check for presence of return
statement in function).
As you know, I don't like the middle end issuing warnings at all. So,
you'll not be surprised that I think we should warn about this function
independent of optimization. People want these warnings to tell them
about possible bugs in their software, not just the ones that affect
them in their current configuration, depending on how much optimization
applies, etc. If we want the middle end to warn about this kind of
case, we should make sure that it does so for all functions, used or not.
Yep, I agree that the warnings should generally come from frontends.
And in fact I was
Re: [RFC] missing return warnings
Jan Hubicka wrote: I am basically trying to avoid need for DECL_INLINE (while keeping DECL_DECLARED_INLINE and DECL_UNINLINABLE for frontend use) - inliner is now doing all the job himself to figure out what is or isn't needed. That certainly makes sense to me. With the noreturn warning made unconditional: ... With the noreturn warning disabled completely: ... So it seems to be all about those warnings now. OK, that sounds pretty encouraging. One problem that I am concerned about is that C++ frontend is actually trying to avoid instantiating some stuff that it knows will not be needed if not inlined. I would kill this logic, but I hope this is the case where the feature is not worth the maintenance cost. I've applied the patch for C++ nightly testing, for tramp3d, the difference seems to be in wash. I will do some proofreading tomorrow and send updated patch. Honza Thansk, -- Mark Mitchell CodeSourcery [EMAIL PROTECTED] (650) 331-3385 x713
Re: [RFC] missing return warnings
Jan Hubicka wrote: One problem that I am concerned about is that C++ frontend is actually trying to avoid instantiating some stuff that it knows will not be needed if not inlined. I would kill this logic, but I hope this is the case where the feature is not worth the maintenance cost. We need to be careful about terms. Instantiating means substituting actual arguments into a template definition to get an actual definition. There's also generating definitions for implicitly-declared class member functions; that doesn't have a great term in the standard, but we can call it defining implicit functions. Which one of these situations are you concerned about? I guess, in a way, it doesn't matter which one. In both cases, the standard says when you must do it. So, we can't change that. I suspect that the code that you're looking at may have just been a front-end optimization. For example, at least at one point, it made GCC a lot slower to have very long instantiation chains: f needs gint, gint needs hint, etc. It was better to instantiate things one at a time than to have a deep stack. But, if hint was inline, then we needed to make sure its body was available when gint was being instantiated so that we could inline it. Now, that's probably no longer true. We can probably always avoid the deep stack, and just let cgraph handle it. -- Mark Mitchell CodeSourcery [EMAIL PROTECTED] (650) 331-3385 x713
