| Message ID | 5477ACB3.2000409@net-b.de |
|---|---|
| State | New |
| Headers | show |
On 27 November 2014 at 23:58, Tobias Burnus <burnus@net-b.de> wrote: > Manuel López-Ibáñez wrote: >> Sure, but I would like to test specifically triggering and discarding >> the gfc_warning that I converted (or another single one that you >> suggest), if this were possible. > > > Hmm, theoretically, it would be possible. However, my feeling is that there > is no such case. What would be needed is something which is ambiguous, the > compiler assumes first the wrong kind of statement, fails, and retries is > with a different kind of statement. > > That's simple. However, either one has already aborted (gfc_error or > NO_MATCH) *before* reaching a gfc_warning - or the code is valid and, hence, > the buffered warning is printed. Then, I don't understand the point of gfc_warning. If gfc_warning is never reached, then we could have used gfc_warning_now. If the warning is printed, then gfc_warning was reached, thus we could have used gfc_warning_now. The only difference is that gfc_warning_now will print the warning as soon as gfc_warning is reached, but since there is at most one buffered warning and there can not be a previous error, then this should not make a difference. What am I missing here? >> Why does it ICE? At worst it should give a wrong location, but the >> computation of the offset is fairly similar to what Fortran already >> does. Could you give me a patch+testcase that ICEs? > > However, the change > > - gfc_warning_now_1 ("Line truncated at %L", &gfc_current_locus); > + gfc_warning_now ("Line truncated at %L", &gfc_current_locus); > > still triggers an ICE with "gfortran -Wall test.f90" and with "test.f" (two > locations in scanner.c; see attached test cases and attache patch). The > backtrace for test.f90 is: The problem seems to be that linemap_position_for_loc_and_offset cannot handle an offset larger than the column_hint. Fortran hardcodes that to 120. There are two bugs there: (1) we should not crash, at worst we should use offset=0 and (2) we should update the column_hint to the largest column seen. I'm testing a patch that seems to fix the problem. Cheers, Manuel.
Manuel López-Ibáñez wrote: > On 27 November 2014 at 23:58, Tobias Burnus <burnus@net-b.de> wrote: >> Hmm, theoretically, it would be possible. However, my feeling is that there >> is no such case. What would be needed is something which is ambiguous, the >> compiler assumes first the wrong kind of statement, fails, and retries is >> with a different kind of statement. >> >> That's simple. However, either one has already aborted (gfc_error or >> NO_MATCH) *before* reaching a gfc_warning - or the code is valid and, hence, >> the buffered warning is printed. > Then, I don't understand the point of gfc_warning. I think the idea is that a warning could be printed in the wrong code path (assumption of which kind of statement or expression one has) – and then one re-tries a different code path, see that it fits and drops the buffered message. Grepping through the 132 gfc_warning, I have the impression that this cannot happen, i.e. that the message is always shown. However, in order to be sure, I had to check all code paths and had to think about ways they become unreachable – with the chance that I miss something. > If gfc_warning is never reached, then we could have used gfc_warning_now. > If the warning is printed, then gfc_warning was reached, thus we could > have used gfc_warning_now. I concur; the question is only whether my impression is correct. You could try to map gfc_warning to gfc_warning_now_1 and see whether something in the testsuite breaks. > The only difference is that gfc_warning_now will print the warning as > soon as gfc_warning is reached, but since there is at most one > buffered warning and there can not be a previous error, then this > should not make a difference. What am I missing here? (a) That, conceptionally, warning buffering makes sense, if you buffering errors for rewinding - even if it only occurs for error and not for warnings. (b) That I might have missed some code path. (c) That one might add such a code path in future. Except for (b), I do not see a reason not to map gfc_warning to gfc_warning_now. If one later needs buffering, one can still take gfc_error as model for implementing it as it surely needs buffering. FX, Steve et al.: Comments or additional facts? >>> Why does it ICE? At worst it should give a wrong location, but the >>> computation of the offset is fairly similar to what Fortran already >>> does. Could you give me a patch+testcase that ICEs? >> However, the change >> >> - gfc_warning_now_1 ("Line truncated at %L", &gfc_current_locus); >> + gfc_warning_now ("Line truncated at %L", &gfc_current_locus); >> >> still triggers an ICE with "gfortran -Wall test.f90" and with "test.f" (two >> locations in scanner.c; see attached test cases and attache patch). The >> backtrace for test.f90 is: > The problem seems to be that linemap_position_for_loc_and_offset > cannot handle an offset larger than the column_hint. Fortran hardcodes > that to 120. There are two bugs there: (1) we should not crash, at > worst we should use offset=0 and (2) we should update the column_hint > to the largest column seen. I'm testing a patch that seems to fix the > problem. Fortran (old code) chops off items from the beginning of the line if the column number is high (and also trimps the output line to the right). Thus, instead of print 'aa.................' ....................(1) is just show aaaaaaa' (1) I don't know how C/C++ handles this. If one doesn't chop off, the output can become extremely long – especially with "topformflat 0". And then the "^" is also not helpful as one has to count the numbe of lines ... Tobias
diff --git a/gcc/fortran/scanner.c b/gcc/fortran/scanner.c index 884fe70..5b42777 100644 --- a/gcc/fortran/scanner.c +++ b/gcc/fortran/scanner.c @@ -777,6 +777,6 @@ skip_free_comments (void) else - gfc_warning_now_1 ("!$OMP at %C starts a commented " - "line as it neither is followed " - "by a space nor is a " - "continuation line"); + gfc_warning_now ("!$OMP at %C starts a commented " + "line as it neither is followed " + "by a space nor is a " + "continuation line"); } @@ -1058,3 +1058,3 @@ restart: gfc_current_locus.nextc = gfc_current_locus.lb->line + maxlen; - gfc_warning_now_1 ("Line truncated at %L", &gfc_current_locus); + gfc_warning_now ("Line truncated at %L", &gfc_current_locus); gfc_current_locus.nextc = current_nextc; @@ -1196,3 +1196,3 @@ restart: gfc_current_locus.lb->truncated = 0; - gfc_warning_now_1 ("Line truncated at %L", &gfc_current_locus); + gfc_warning_now ("Line truncated at %L", &gfc_current_locus); } @@ -1390,3 +1390,3 @@ gfc_gobble_whitespace (void) linenum = cur_linenum; - gfc_warning_now_1 ("Nonconforming tab character at %C"); + gfc_warning_now (OPT_Wtabs, "Nonconforming tab character at %C"); }