[cfe-commits] r161650 - in /cfe/trunk: include/clang/Frontend/VerifyDiagnosticConsumer.h lib/ARCMigrate/ARCMT.cpp lib/Frontend/VerifyDiagnosticConsumer.cpp test/ARCMT/verify.m test/ASTMerge/function.c test/Frontend/verify.c test/Frontend/verify2.c test/Frontend/veri

[Jordan Rose]

On Aug 16, 2012, at 11:00 , Andy Gibbs <andyg1001 at hotmail.co.uk> wrote:

> On Thursday, August 16, 2012 6:28 PM, Jordan Rose wrote:
>> Hm. Now I'm having second thoughts about this:
>> 
>>> fatal_error:
>>>      llvm::report_fatal_error(Twine("presence of -verify directives cannot be "
>>>                                     "verified during post-processing check of ",
>>>                               StringRef(FE ? FE->getName() : "(unknown)")));
>> 
>> This happens if we don't have a SourceManager or Preprocessor available.
>> But what if we really /don't/ have a Preprocessor available?
> 
> The best, of course, is that by the time we get to the post-processing
> check, there are *no* unparsed files.  Thinking about it again, what could
> be done is this:
> 
> (this is a hybrid between your suggestion and my previous approach)
> 
> Store two lists:
> 
> Map<FileID, FileEntry*> Parsed
> Map<FileID, Pair<FileEntry*, bool FoundDirectives>> Unparsed
> 
> ---When we enter a file from the preprocessor---
> remove from unparsed set
> add to parsed set
> 
> ---When we get a diagnostic---
> if (is a macro expansion)
> ignore [[big culprit this one!]]

Why not look at where the macro is instantiated?


> if (not in parsed set)
> if (is modules header)
>   add to parsed set
> else
>   run through findDirectives
>   add to unparsed set, setting FoundDirectives flag appropriately
> 
> ---When you check at the end---
> if (unparsed set not empty)
> build set of parsed FileEntry pointers
> iterate through unparsed set:
>   ignore all files with aliases
>   fatal error if FoundDirective flag set
> 
> 
> This way, SourceManager and Preprocessor instances are not needed at all
> at the end check.  SourceManager *is* needed for checking diagnostics
> lists, but this is another thing.  This is the big advantage of this
> approach: the end check is really freed of its constraints, and
> becomes a very fast loop (all the data is already gathered).
> 
> The downside, but a small one perhaps?, is that findDirectives may be
> called more times than necessary.  Analysing the situation: in most
> cases, a file will be in the parsed set before it produces a diagnostic.
> Those that aren't are in most cases actually macro expansions and should
> therefore be ignored.  Those that fall outside that are PCH, modules and
> -ast-merge tests.  Of these, its the PCH and -ast-merge tests that will
> call findDirectives on their included files, and therefore be handled
> less efficiently (potentially) this way.

Once an ID is in Unparsed, you've already run through findDirectives. You could get a few funny issues with aliases here, but I think that's going to be fairly rare (unparsed and aliased and diagnostics from different aliases). So you can just add a check before 'run through findDirectives' to see if it's already been marked as unparsed (and therefore is being externally parsed).


> Now, for findDirectives, we can (I think) get rid of the Preprocessor
> requirement since the only thing it is really needed for is the LangOpts
> structure for the Lexer -- the rest is freely available elsewhere.  (And
> LangOpts can be picked up from BeginSourceFile, so even this shouldn't be
> a problem.)
> 
> This means we really start to be quite free from the Preprocessor.
> 
> What do you think to this approach?

I think it works pretty well, but I'd like to see it in code. :-) Apparently my reservations always appear on second reading.

Thanks for the continued iterations,
Jordan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120816/12ec9fba/attachment.html>