Re: Fix 69650, bogus line numbers from libcpp

[Bernd Schmidt <bschmidt at redhat dot com>]

On 03/21/2016 09:15 PM, David Malcolm wrote:
> On Mon, 2016-03-14 at 14:20 +0100, Bernd Schmidt wrote:
> > On 03/11/2016 11:09 PM, David Malcolm wrote:
> > > > +	  cpp_error (pfile, CPP_DL_ERROR,
> > > > +		     "file \"%s\" left but not entered",
> > > > new_file);
> > >                                                            ^^^^^^^^
> > > Although it looks like you're preserving the existing behavior from
> > > when this was in linemap_add, shouldn't this be
> > >     ORDINARY_MAP_FILE_NAME (from)
> > > rather than new_file?  (i.e. shouldn't it report the name of the
> > > file
> > > being *left*, rather than the one being entered?)
>
> I realize now that I was wrong here: "new_file" refers to the filename
> given in the linemarker directive, which, depending on the (optional)
> flag could be a directive to enter or leave a linemap.
>
> Sorry about that; you may want to disregard my earlier worries.

No, I think you were mostly on point: new_file is the one in the #line 
directive, which AFAICT is the file being reentered. so the wording is 
in fact misleading. Including a file called "t.h" from "v.c" produces 
this pattern:

# 1 "t.h" 1
int t;
# 2 "v.c" 2

> I was thinking of something like the attached, which makes things more
> explicit; I felt the first dg-error in your patch was a little too
> concise.

No problem, but I do think we want to change the wording of the message 
to something like "file %s unexpectedly reentered".

> I'm very familiar with the code for tracking ranges and issuing
> diagnostics, but less familiar with other parts of libcpp, so I'm not
> sure I'm fully qualified to approve the patch.  That said, the patch
> looks sane, mostly...  The remaining thing I have a concern about
> relates to the other question I had, which I don't think you addressed:
> is it possible to construct a testcase that triggers the logic in the
> non-MAIN_FILE_P clause?

Are you thinking of anything more complex than the following?

# 1 "v.c"
# 1 "t.h" 1
# 1 "t2.h" 1
int t;
# 2 "t.h" 2
# 2 "v.c" 2

Change any of the filenames of the "^#.*2$" lines and you'll see error 
messages. For example, changing "t.h" to "x.h" in the second to last line:

In file included from t.h:1:0,
                 from v.c:1:
t2.h:2:13: error: file "x.h" left but not entered
t2.h:3:12: error: file "v.c" left but not entered

Of course any such error is likely to have a large number of follow-on 
errors. Not sure how to avoid this given that the input file most likely 
has a completely messed up structure.

> (How much existing test coverage do we have for line-markers?  I found
> about 15 existing testcases that use them in a crude search with grep,
> but these are all apparently only incidentally as part of testing other
> functionality; is it worth me adding some more general test coverage
> for this?)

It might be worthwhile but I'm not planning to for this issue.


Bernd