[Bug fortran/67429] [5/6 Regression] Missing part of error messages.

[pault at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67429

--- Comment #16 from Paul Thomas  ---
Author: pault
Date: Fri Sep  4 18:37:50 2015
New Revision: 227500

URL: https://gcc.gnu.org/viewcvs?rev=227500=gcc=rev
Log:
2015-09-04  Manuel López-Ibáñez  

PR fortran/67429
* error.c (gfc_clear_pp_buffer): Reset last_location, otherwise
caret lines might be skipped when actually giving a diagnostic.

Modified:
trunk/gcc/fortran/ChangeLog
trunk/gcc/fortran/error.c

[Bug fortran/67429] [5/6 Regression] Missing part of error messages.

[pault at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67429

--- Comment #17 from Paul Thomas  ---
Author: pault
Date: Fri Sep  4 18:41:08 2015
New Revision: 227501

URL: https://gcc.gnu.org/viewcvs?rev=227501=gcc=rev
Log:
2015-09-04  Manuel López-Ibáñez  

PR fortran/67429
* error.c (gfc_clear_pp_buffer): Reset last_location, otherwise
caret lines might be skipped when actually giving a diagnostic.

Modified:
branches/gcc-5-branch/gcc/fortran/ChangeLog
branches/gcc-5-branch/gcc/fortran/error.c

[Bug fortran/67429] [5/6 Regression] Missing part of error messages.

[pault at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67429

Paul Thomas  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |FIXED

--- Comment #18 from Paul Thomas  ---
Fixed on trunk and 5-branch. Thanks for raising the PR, Dominique, and thanks
for a quick fix, Manuel.

Paul

[Bug fortran/67429] [5/6 Regression] Missing part of error messages.

[dominiq at lps dot ens.fr]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67429

--- Comment #11 from Dominique d'Humieres  ---
(In reply to Manuel López-Ibáñez  from comment #10)

> > Error: Expecting END BLOCK statement at (1)
> > (null):0: confused by earlier errors, bailing out
>
>
> Even if *I* cannot reproduce this, such an error should be easy to debug:
> Just put a break in diagnostic_report_diagnostic on the line that gives
> this message and go up to find out what triggered this code to run.
> My guess is that you are somehow triggering an ICE while reporting an ICE.
> It would be interesting to understand where that strange location comes
> from (I guess it is UNKNOWN_LOCATION?). And also to harden that code to not
> try to print a null s.file pointer!

Sorry for the confusion I sometime get this kind of error instead of the usual
ICE when using compilers configured with '--enable-checking=release'.

[Bug fortran/67429] [5/6 Regression] Missing part of error messages.

[dominiq at lps dot ens.fr]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67429

--- Comment #12 from Dominique d'Humieres  ---
> > Created attachment 36283 [details]
> > Patch for PR65045 that displays the problem
>
> Paul,
>
> Your patch for gcc/fortran/decl.c does not apply on trunk:
>
>  if (state == COMP_BLOCK)
>
> is now
>
>  while (state == COMP_BLOCK)

With the adjusted patch (IF replaced with WHILE) the ICE in PR65045 is gone and
gfortran retest cleanly, even if the error for the new test is

/opt/gcc/p_work/gcc/testsuite/gfortran.dg/pr65045.f90:7:6: Error: Symbol 'i' at
(1) is not appropriate for an expression
/opt/gcc/p_work/gcc/testsuite/gfortran.dg/pr65045.f90:9:59:

else  ! { dg-error "Unexpected ELSE statement" }
   1
Error: Unexpected ELSE statement at (1)
/opt/gcc/p_work/gcc/testsuite/gfortran.dg/pr65045.f90:10:6: Error: 'i' at (1)
is not a variable
/opt/gcc/p_work/gcc/testsuite/gfortran.dg/pr65045.f90:11:6:

end if! { dg-error "Expecting END BLOCK statement" }
  1
Error: Expecting END BLOCK statement at (1)
/opt/gcc/p_work/gcc/testsuite/gfortran.dg/pr65045.f90:13:8:

 print*,i ! { dg-error "not appropriate for an expression" }
1
Error: Symbol 'i' at (1) is not appropriate for an expression

[Bug fortran/67429] [5/6 Regression] Missing part of error messages.

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67429

Richard Biener  changed:

   What|Removed |Added

   Target Milestone|--- |5.3

[Bug fortran/67429] [5/6 Regression] Missing part of error messages.

[manu at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67429

--- Comment #15 from Manuel López-Ibáñez  ---
(In reply to Paul Thomas from comment #14)
> Do you want to commit or would you like me to do it?

Please do, I'll have little time for gcc in the next weeks. Thanks.

[Bug fortran/67429] [5/6 Regression] Missing part of error messages.

[pault at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67429

--- Comment #14 from Paul Thomas  ---
(In reply to Dominique d'Humieres from comment #13)
> The patch in comment 9 restores the old (correct?) behavior without
> regression.

Indeed and it gets rid of the problems that I have been encountering.

Manuel,

Do you want to commit or would you like me to do it?

Cheers

Paul

[Bug fortran/67429] [5/6 Regression] Missing part of error messages.

[dominiq at lps dot ens.fr]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67429

--- Comment #13 from Dominique d'Humieres  ---
The patch in comment 9 restores the old (correct?) behavior without regression.

[Bug fortran/67429] [5/6 Regression] Missing part of error messages.

[kargl at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67429

kargl at gcc dot gnu.org changed:

   What|Removed |Added

 CC||kargl at gcc dot gnu.org

--- Comment #1 from kargl at gcc dot gnu.org ---
(In reply to Dominique d'Humieres from comment #0)
> When compiling the test in pr65045 with r218570 I get the errors
> 

So, why isn't this a duplicate of PR65045?

[Bug fortran/67429] [5/6 Regression] Missing part of error messages.

[manu at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67429

--- Comment #9 from Manuel López-Ibáñez  ---
Even with the original patch applied to r227391, I cannot reproduce the error
that you get. On x86_64-linux-gnu, I get:

Starting program: /home/manuel/test1/226953M/build/gcc/f951
~/test1/src/gcc/testsuite/gfortran.dg/pr65045.f90
/home/manuel/test1/src/gcc/testsuite/gfortran.dg/pr65045.f90:7:6: Error: Symbol
‘i’ at (1) is not appropriate for an expression
/home/manuel/test1/src/gcc/testsuite/gfortran.dg/pr65045.f90:9:59:

else  ! { dg-error "Unexpected ELSE statement" }
   1
Error: Unexpected ELSE statement at (1)
/home/manuel/test1/src/gcc/testsuite/gfortran.dg/pr65045.f90:10:6: Error: ‘i’
at (1) is not a variable
/home/manuel/test1/src/gcc/testsuite/gfortran.dg/pr65045.f90:11:6:

end if! { dg-error "Expecting END BLOCK statement" }
  1
Error: Expecting END BLOCK statement at (1)
/home/manuel/test1/src/gcc/testsuite/gfortran.dg/pr65045.f90:13:8:

 print*,i ! { dg-error "not appropriate for an expression" }
1
Error: Symbol ‘i’ at (1) is not appropriate for an expression


However, there is a bug indeed. The first caret line is not printed because
there was a previous buffered error at the same location that later got
cleared. However, clearing it does not reset context->last_location, which is
checked in gfc_diagnostic_starter.

This patch seems to fix it:

Index: error.c
===
--- error.c (revision 227391)
+++ error.c (working copy)
@@ -755,10 +755,13 @@ gfc_clear_pp_buffer (output_buffer *this
   pretty_printer *pp = global_dc->printer;
   output_buffer *tmp_buffer = pp->buffer;
   pp->buffer = this_buffer;
   pp_clear_output_area (pp);
   pp->buffer = tmp_buffer;
+  /* We need to reset last_location, otherwise we may skip caret lines
+ when we actually give a diagnostic.  */
+  global_dc->last_location = UNKNOWN_LOCATION;
 }



However, it is not a very nice solution. It may happen that we give an error at
location X, then we buffer some errors, then we clear them, so we reset last
location to UNKNOWN, then we give an error at location X and we will get a
duplicated caret line. Thus, it would be better if last_location was only
updated when we actually flush the output text. However, that would require
some deeper changes, probably in diagnostic.c and in fortran/error.c to handle
both buffered and not buffered output_buffers. On the other hand, I'm not sure
the previous Fortran diagnostic machinery cared about duplicated caret lines,
thus perhaps the above is enough.

BTW, you should use %qs instead of '%s' in order to get quoting localized to
the user's language and color highlighting.

[Bug fortran/67429] [5/6 Regression] Missing part of error messages.

[manu at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67429

--- Comment #10 from Manuel López-Ibáñez  ---
(In reply to Paul Thomas from comment #7)
> The patch for PR65045 is the simplest manifestation that I have found. I am
> very grateful to Dominique for posting this PR because the problem has been
> driving me crazy. I assumed that it was my patches that were wrong. However,
> Dominique proved by going through different revisions that the December 2014
> patches to error.c are the culprits.

(In reply to Dominique d'Humieres from comment #0)

> Error: Expecting END BLOCK statement at (1)
> (null):0: confused by earlier errors, bailing out

Even if *I* cannot reproduce this, such an error should be easy to debug: Just
put a break in diagnostic_report_diagnostic on the line that gives this message
and go up to find out what triggered this code to run. My guess is that you are
somehow triggering an ICE while reporting an ICE. It would be interesting to
understand where that strange location comes from (I guess it is
UNKNOWN_LOCATION?). And also to harden that code to not try to print a null
s.file pointer!

[Bug fortran/67429] [5/6 Regression] Missing part of error messages.

[sgk at troutmask dot apl.washington.edu]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67429

--- Comment #3 from Steve Kargl  ---
On Tue, Sep 01, 2015 at 08:05:33PM +, dominiq at lps dot ens.fr wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67429
> 
> --- Comment #2 from Dominique d'Humieres  ---
> > So, why isn't this a duplicate of PR65045?
> 
> Because PR65045 is about an ICE and this PR is about missing text in the error
> messages.
> 

It's the same piece of code causing the problem.  One
needs to go to PR65045 to get the code, so this PR
is superfluous.  When the ICE is fixed, then
error message should be addressed.

[Bug fortran/67429] [5/6 Regression] Missing part of error messages.

[dominiq at lps dot ens.fr]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67429

--- Comment #4 from Dominique d'Humieres  ---
> It's the same piece of code causing the problem.  One
> needs to go to PR65045 to get the code, so this PR
> is superfluous.  When the ICE is fixed, then
> error message should be addressed.

The piece of code is used as an example exhibiting a different problem than
PR65045.
Please read comment 0.

[Bug fortran/67429] [5/6 Regression] Missing part of error messages.

[pault at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67429

Paul Thomas  changed:

   What|Removed |Added

 CC||pault at gcc dot gnu.org

--- Comment #6 from Paul Thomas  ---
Created attachment 36283
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=36283=edit
Patch for PR65045 that displays the problem

[Bug fortran/67429] [5/6 Regression] Missing part of error messages.

[dominiq at lps dot ens.fr]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67429

Dominique d'Humieres  changed:

   What|Removed |Added

 Status|UNCONFIRMED |NEW
   Last reconfirmed||2015-09-01
 Ever confirmed|0   |1

--- Comment #8 from Dominique d'Humieres  ---
> Created attachment 36283 [details]
> Patch for PR65045 that displays the problem

Paul,

Your patch for gcc/fortran/decl.c does not apply on trunk:

 if (state == COMP_BLOCK)

is now

 while (state == COMP_BLOCK)

[Bug fortran/67429] [5/6 Regression] Missing part of error messages.

[sgk at troutmask dot apl.washington.edu]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67429

--- Comment #5 from Steve Kargl  ---
>
> Please read comment 0.
> 

I read comment #0.

THERE IS NO CODE THERE.

THERE IS NO CODE ATTACHED TO THIS PR.

One needs to go to PR65045 to get the code
that is causing the error message.  Ergo,
this is a duplicate of PR65045.

When PR65045 is fixed.  The issue with the
error message should be addressed then.

[Bug fortran/67429] [5/6 Regression] Missing part of error messages.

[pault at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67429

--- Comment #7 from Paul Thomas  ---
(In reply to Steve Kargl from comment #5)
> >
> > Please read comment 0.
> > 
> 
> I read comment #0.
> 
> THERE IS NO CODE THERE.
> 
> THERE IS NO CODE ATTACHED TO THIS PR.
> 
> One needs to go to PR65045 to get the code
> that is causing the error message.  Ergo,
> this is a duplicate of PR65045.
> 
> When PR65045 is fixed.  The issue with the
> error message should be addressed then.

Steve,

This is a problem that has surfaced with the change to the gcc error handling.
Rainer Orth has encountered it on Solaris and it caused me to hold back on my
patch for pointer function assignment.

The patch for PR65045 is the simplest manifestation that I have found. I am
very grateful to Dominique for posting this PR because the problem has been
driving me crazy. I assumed that it was my patches that were wrong. However,
Dominique proved by going through different revisions that the December 2014
patches to error.c are the culprits.

Go easy on us both - it's getting a bit late here :-)

Cheers

Paul

[Bug fortran/67429] [5/6 Regression] Missing part of error messages.

[dominiq at lps dot ens.fr]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67429

--- Comment #2 from Dominique d'Humieres  ---
> So, why isn't this a duplicate of PR65045?

Because PR65045 is about an ICE and this PR is about missing text in the error
messages.