[cfe-dev] Proposed minor refactoring of the diag subsystem

[Nico Weber]

Hi,

the TextDiagPrinter in libDriver causes a crash if one forgets to (or  
doesn't know that one has to) call setHeaderSearch() on it. A simple  
fix is to check if headersearch is valid in  
TextDiagnostics::isInSystemHeader(). However, this really is a hack: A  
DiagClient without a HeaderSearch is always a rogue object, since it  
doesn't implement isInSystemHeader() correctly (case in point: Running  
clang with `-html-diags=html` produces a different set of warnings as  
running clang without that option.

So I propose a HeaderSearch object should always be mandatory for a  
DiagClient (which means HeaderSearch, HeaderMap and DirectoryLookup  
move from Lex to Basic). Then, isInSystemHeader() can be implemented  
in the base DiagClient. FormatDiagnostic() should also be there; most  
clients need this anyways.

In return, DiagClient should be optional for Diagnostic. If the client  
is 0, then no diagnostics are reported (the ErrorOccurred flag is of  
cause still set). That this is the "right thing" is already hinted at  
by the change Ted landed today that change DiagClient from a reference  
to a pointer. This also makes constructing a Preprocessor object easier.

The attached patch implements the proposed changes. This removes some  
code duplication from PathDiagnostic, and with this `-html-diags`  
outputs the same warnings as command-line clang. If clients _want_ to  
see warnings in system headers, they can still override  
isInSystemHeader() to always return false.

Comments? :-)

Nico

"Why change one line when you can change a thousand?" :-P


-------------- next part --------------
A non-text attachment was scrubbed...
Name: diag_refactor.patch
Type: application/octet-stream
Size: 68137 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20080807/07bcf63e/attachment.obj>
-------------- next part --------------