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

Nico Weber nicolasweber at gmx.de
Sat Aug 9 16:56:55 PDT 2008


Hi Chris,

> I like some parts of this patch, but others are more questionable.   
> The nice thing about libbasic right now is that it is C-indepedent.   
> The components in it are reusable for other languages, e.g. if you  
> did a Java or Ada frontend.  HeaderSearch is quite C specific, so  
> I'm somewhat uncomfortable with moving it into basic.

I don't _quite_ buy that, as SourceLocation can contain a MacroID,  
FileIDInfo has an IncludeLoc, etc -- there's C-dependence in Basic  
everywhere ;-)

>> * Remove isInSystemHeader() from DiagClient
>
> isInSystemHeader really is a hack and it's been on my todo list to  
> fix for quite awhile.  However, I think it is best to turn it into a  
> property managed by SourceManager on a per-file-id basis.  I think  
> that when the preprocessor ends up calling  
> "SourceManager::create*FileID", that it should pass in the system- 
> headerness into SourceManager.  Then we'd have methods on  
> SourceManager that tell you if a SourceLocation is in a system  
> header or not.
>
> Does this seem reasonable?

That's a great idea. I've done that.

>> This fixes the following problems:
>>
>> * -html-diags (and probably others) does now output the same set of  
>> warnings as console clang does
>> * nothing crashes if one forgets to call setHeaderSearch() on  
>> TextDiagnostic
>> * some code duplication is removed
>
> I think that making SourceManager handle system-headerness (and  
> having the PP pass this info down from headersearch when it is  
> entering #includes, etc) would solve this problem in a less invasive  
> way.  Do you think this approach would work?

Seems to work fine; attached.

Nico

-------------- next part --------------
A non-text attachment was scrubbed...
Name: diag_refactor_v3.patch
Type: application/octet-stream
Size: 22672 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20080810/6e8d3822/attachment.obj>
-------------- next part --------------



More information about the cfe-dev mailing list