[cfe-dev] adding FileSkipped() to PPCallbacks

[Chris Lattner]

On Apr 16, 2010, at 2:09 PM, Zhanyong Wan (λx.x x) wrote:

> Thanks for the review, Chris.  Please see the new patch.  I kept the
> signature of FileSkipped() unchanged to be consistent with
> FileChanged().

Keeping the interfaces consistent is nice, but is it really worth it?  This will punish other PPCallbacks that don't care about this, like -MM mode and friends.  SourceMgr.createFileID isn't hyper-expensive, but it is good to avoid work that no other client at all needs.  Sinking this work into the implementation that cares makes sense to me.

-Chris

> 
> On Fri, Apr 16, 2010 at 1:34 PM, Chris Lattner <clattner at apple.com> wrote:
>> 
>> On Apr 16, 2010, at 11:48 AM, Zhanyong Wan (λx.x x) wrote:
>> 
>>> Hi Doug,
>>> 
>>> Per our IRC chat yesterday, I'm extending PPCallbacks with a new
>>> method FileSkipped() to notify a listener when a #include is skipped
>>> due to header guard optimization.  Please see the attached patch and
>>> let me know if this is the right direction.  (I verified that it works
>>> for my use case.)  Also, how should I add a test for this?
>> 
>> The .h file change look fine to me, but it isn't good to sink the check below SourceMgr.createFileID.  Skipping headers is a fairly hot path, so we don't want it to do additional work when ppcallbacks aren't being used.
>> 
>> Please change the callback to take the FileEntry or some other information that doesn't require sinking the check below the potentially expensive work.
>> 
>> -Chris
> 
> 
> 
> -- 
> Zhanyong
> <file_skipped2.patch>