[cfe-dev] adding FileSkipped() to PPCallbacks

[Chris Lattner]

On Apr 19, 2010, at 12:38 PM, Zhanyong Wan (λx.x x) wrote:

> Please take a look at the new patch, which doesn't call createFileID()
> for a skipped file.  Thanks!

Looks good, committed in r101813, thanks!

-Chris

> 
> On Fri, Apr 16, 2010 at 4:41 PM, Chris Lattner <clattner at apple.com> wrote:
>> 
>> On Apr 16, 2010, at 2:38 PM, Zhanyong Wan (λx.x x) wrote:
>> 
>>> On Fri, Apr 16, 2010 at 2:14 PM, Chris Lattner <clattner at apple.com> wrote:
>>>> 
>>>> 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.
>>> 
>>> If createFileID() is called here, the file being skipped has already
>>> been processed earlier, which is a much more expensive operation.  So
>>> do we really want to sacrifice the cleanness of the API for it?
>> 
>> I'm pushing back because the preprocessor is a particularly performance sensitive part of the compiler, and this adds no value for any current clients.  I think the cost should be sunk into your client, even if it makes its implementation a little less elegant.
>> 
>> -Chris
> 
> 
> 
> -- 
> Zhanyong
> <file_skipped3.patch>