[cfe-dev] Proposed minor refactoring of the diag subsystem
kremenek at apple.com
Sat Aug 9 11:17:02 PDT 2008
On Aug 8, 2008, at 11:54 AM, Nico Weber wrote:
> On 08.08.2008, at 01:35, Ted Kremenek wrote:
>> HeaderSearch is tied to the Preprocessor, and hence belongs in
>> libLex, not libBasic.
> Why? HeaderSearch does not use anything from the Preprocessor. To
> me, it's not more Lex-y than IdentifierInfo, which is in Lex, too.
HeaderSearch implements functionality of the Preprocessor. It
provides the logic to help resolve what a #include and #import does.
It doesn't use the Preprocessor, but it is a piece of the preprocessor
component. The whole notion of a system header, etc., is all tied to
the preprocessor. That's why I don't believe HeaderSearch belongs in
>> If isInPrivateHeader() is not meaningful for all DiagClients, it
>> should be removed from DiagClient. Perhaps Diag should have two
>> methods, ReportWithFile(), that takes a HeaderSearch, and Report().
>> Then Lex, Parser, and Sema could call this method, and other
>> clients could call the ordinary Report() function.
I don't see the advantage of having Report methods that are overloaded
in this manner. This would also place a burden on clients of
Diagnositcs to use either method, which seems to shift some of the
burden of responsibility from DiagnosticClient to the clients of
Diagnostics. The clients of Diagnostic should *not* decide the policy
of how diagnostics are rendered; that's the task of the
DiagnosticClient. Clients should typically only care about
SourceLocations; that's the beauty of the current interface. Specific
instances of DiagnosticClient can then decide how diagnostics are
presented by using information from HeaderSearch, etc.
>> With this change, DiagClient doesn't need to know about
>> HeaderSearch at all (but Diagnotic must know about it, hence
>> HeaderSearch would have to stay in Basic).
I agree that methods such as TextDiagnosticClient::isInSystemHeader()
should be refactored for use by multiple DiagnosticClients, but I'm
not convinced (yet) that putting it into Diagnostics is the answer.
They current layering allows the Diagnostics class to be useful even
in the complete absence of preprocessor information.
More information about the cfe-dev