[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