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? Do we really want to
complain about this? A buffer that doesn't need to be preprocessed probably
won't contain -verify directives. Or the directives could be added in some
other way. Do we want to /require/ that a preprocessor be available just to
check the diagnostics we have?
Perhaps the compromise here is to do the checking only after we've checked the
current set of diagnostics against the current set of directives. This misses
the case where someone put an expected-warning in an unparsed file that refers
to a diagnostic in a parsed file, but that seems pretty rare.
To put it another way, if all diagnostics are accounted for in a PCH file,
maybe I don't care that the PCH file has its own expectations. I can see how
that would be arguable, though.
(We'd miss a few intended "expected but not seen", but never miss "seen but not
expected".)
Jordan
On Aug 15, 2012, at 22:28 , Andy Gibbs <[email protected]> wrote:
> On Thursday, August 16, 2012 4:05 AM, Jordan Rose wrote:
>> This looks pretty good to me! I initially thought we could pull all the
>> SourceManager stuff under NDEBUG, but it does make sense to have it used
>> for the actual emitting of errors at the end, rather than pulling it
>> from the preprocessor.
>
> I did consider keeping the map between FileID and FileEntry* and also
> marking module header files as parsed at the point they are seen so that
> SrcManager also dropped out in the NDEBUG block in CheckDiagnostics, but
> I wasn't convinced the additional complexity and "baggage" was warranted
> so I kept with the simple approach for now.
>
>> I'll look at this again tomorrow morning (time zone PDT) and commit it
>> then if it all looks good.
>
> Great, thanks! :o)
>
> Plus, it means I can also get in this bit that slipped the net:
>
> --- tools/clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
> +++ tools/clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
> @@ -37,10 +37,11 @@
> }
>
> VerifyDiagnosticConsumer::~VerifyDiagnosticConsumer() {
> assert(!ActiveSourceFiles && "Incomplete parsing of source files!");
> assert(!CurrentPreprocessor && "CurrentPreprocessor should be invalid!");
> + SrcManager = 0;
> CheckDiagnostics();
> Diags.takeClient();
> if (OwnsPrimaryClient)
> delete PrimaryClient;
> }
>
>
> SrcManager needs to be cleared before CheckDiagnostics because it will
> certainly be pointing to some invalid location by this stage.
>
> Thanks again,
>
> Andy
>
>
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
