r179677 - Extended VerifyDiagnosticConsumer to also verify sourcefile for diagnostic.

[Andy Gibbs]

From: Jordan Rose 
Sent: Wednesday, April 17, 2013 7:14 PM
To: Andy Gibbs 
Cc: cfe-commits at cs.uiuc.edu 
Subject: Re: r179677 - Extended VerifyDiagnosticConsumer to also verify sourcefile for diagnostic.




On Apr 17, 2013, at 1:06 , Andy Gibbs <andyg1001 at hotmail.co.uk> wrote:


  +        // Lookup file via Preprocessor, like a #include.
  +        const DirectoryLookup *CurDir;
  +        const FileEntry *FE = PP->LookupFile(Filename, false, NULL, CurDir,
  +                                             NULL, NULL, 0);



This seems a bit awkward...what if the file is included on the command line with -include? (Some of our tests do this.)


I guess it works. I was just expecting something like a last-path-component match, using, say, a search over fileinfo_begin()/_end() in SourceManager.

The problem is choosing a solution that works with modules, which is always the tricky case.  I chose this route because the file may not have already been loaded (which is why there is also a check to see whether the file needs to be registered).  Using the Preprocessor LookupFile method works quite well because it enables use of the #include search paths, plus most test-cases will use #include for the included files so there is some symmetry here.  See what you think after you've used it for a bit; if it needs further perfecting, I can do so!


  +        if (PH.Next(Line) && Line > 0)
  +          ExpectedLoc = SM.translateFileLineCol(FE, Line, 1);



This should probably be asserted or checked; it doesn't make sense to specify another file without a line number.

It is checked.  You get an error message if the line number is omitted or invalid.  :o)


I'll look into the 79/80 issue too. Thanks for doing this!
Jordan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130417/704350c2/attachment.html>