On Mon, 2015-10-12 at 17:36 +0100, Manuel López-Ibáñez wrote: > On 12 October 2015 at 16:44, David Malcolm <dmalc...@redhat.com> wrote: > > v4 of the patch does the conversion of Fortran, and eliminates the > > adaptation layer. No partial transitions here! > > > > Manu: I hope this addresses your concerns. > > Yes, it looks great. I don't understand how this > > - and for two locations that do not fit in the same locus line: > - > - [name]:[locus]: Error: (1) > - [name]:[locus2]: Error: Some error at (1) and (2) > + [locus of primary range]: Error: Some error at (1) and (2) > > > passes the Fortran regression testsuite since the testcases normally > try to match the two locus separately, but I guess you figured out a > way to make it work and I must admit I did not have the time to read > the patch in deep detail.
The way it works is that the patch kit emulates the behavior of the old printer for the -fno-diagnostics-show-caret case. Consider this two-locus error, where the loci are on different lines. With the patch it prints: associate_5.f03:33:6: ASSOCIATE (y => x) ! { dg-error "variable definition context" } 2 y = 5 ! { dg-error "variable definition context" } 1 Error: Associate-name ‘y’ can not appear in a variable definition context (assignment) at (1) because its target at (2) can not, either ...using the new implementation of diagnostic-show-locus. With -fno-diagnostics-show-caret, it prints: associate_5.f03:33:6: Error: (1) associate_5.f03:32:20: Error: Associate-name ‘y’ can not appear in a variable definition context (assignment) at (1) because its target at (2) can not, either where the latter is the same behavior as before the patch. The testsuite passes since it's faithfully emulating the old -fno-diagnostics-show-caret behavior. > But it is a bit strange that you also > deleted this part: > > - With -fdiagnostic-show-caret (the default) and for valid locations, > - it prints for one location: > + With -fdiagnostic-show-caret (the default) it prints: > > - [locus]: > + [locus of primary range]: > > some code > 1 > Error: Some error at (1) > > - for two locations that fit in the same locus line: > + With -fno-diagnostic-show-caret or if the primary range is not > + valid, it prints: > > - [locus]: > - > - some code and some more code > - 1 2 > - Error: Some error at (1) and (2) > - > - and for two locations that do not fit in the same locus line: > - > - [locus]: > - > - some code > - 1 > - [locus2]: > - > - some other code > - 2 > - Error: Some error at (1) and (2) > - > > which should work the same before and after your patch. But this isn't what the new printer prints, for the -fdiagnostic-show-caret case. It doesn't print multiple "[locusN]:" lines; these are only printed for the no-d-show-caret case. > Independently > of whether the actual logic moved into some new mechanism in the new > rich locations world, this seems like useful info to keep in > fortran/error.c. Perhaps it's easiest to approach this from the POV of what the comment *should* say. For reference, the comment for gfc_diagnostic_starter reads like this after the patch: /* This function prints the locus (file:line:column), the diagnostic kind (Error, Warning) and (optionally) the relevant lines of code with annotation lines with '1' and/or '2' below them. With -fdiagnostic-show-caret (the default) it prints: [locus of primary range]: some code 1 Error: Some error at (1) With -fno-diagnostic-show-caret or if the primary range is not valid, it prints: [locus of primary range]: Error: Some error at (1) and (2) */ Does this look OK? Thanks Dave