On 12/16/2015 07:19 PM, David Malcolm wrote:
In the C FE, c_parser_statement_after_labels passes "xloc" to
c_finish_return, which is the location of the first token
within the returned expression.

Hence we don't get a full underline for the following:

diagnostic-range-bad-return.c:34:10: warning: function returns address of local 
variable [-Wreturn-local-addr]
    return &some_local;
           ^

This feels like a bug; this patch fixes it to use the location of
the expr if available, and to fall back to xloc otherwise, giving
us underlining of the full expression:

diagnostic-range-bad-return.c:34:10: warning: function returns address of local 
variable [-Wreturn-local-addr]
    return &some_local;
           ^~~~~~~~~~~

The testcase also adds some coverage for underlining the
"return" token for the cases where we're warning about th
erroneous presence/absence of a return value.

As an additional tweak, it struck me that we could be more
user-friendly for these latter diagnostics by issuing a note
about where the function was declared, so this patch also adds
an inform for these cases:

diagnostic-range-bad-return.c: In function 'missing_return_value':
diagnostic-range-bad-return.c:31:3: warning: 'return' with no value, in 
function returning non-void
    return; /* { dg-warning "'return' with no value, in function returning 
non-void" } */
    ^~~~~~

diagnostic-range-bad-return.c:29:5: note: declared here
  int missing_return_value (void)
      ^~~~~~~~~~~~~~~~~~~~

(ideally we'd put the underline on the return type, but that location
isn't captured)

This latter part of the patch is an enhancement rather than a
bugfix, though FWIW, and I'm not sure I can argue this with a
straight face, the tweak was posted as part of:
   "[PATCH 16/22] C/C++ frontend: use tree ranges in various diagnostics"
in https://gcc.gnu.org/ml/gcc-patches/2015-09/msg00745.html
during stage 1.  Hopefully low risk, and a small usability improvement;
but if this is pushing it, it'd be simple to split this up and only
do the bug fix.

Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu;
adds 12 PASS results to gcc.sum.

OK for trunk for stage 3?

gcc/c/ChangeLog:
        * c-parser.c (c_parser_statement_after_labels): When calling
        c_finish_return, Use the return expression's location if it has
        one, falling back to the location of the first token within it.
        * c-typeck.c (c_finish_return): When issuing warnings about
        the incorrect presence/absence of a return value, issue a note
        showing the declaration of the function.
This is fine. I think the first is pretty easy to justify. THe second is harder. Again I think it's very low risk and has user-visible benfits.

Jeff

Reply via email to