[cfe-dev] RFC: '--verify-pch /path/to/pch/file' to verify that a PCH file can be used

Wed Nov 13 12:27:01 PST 2013

On Nov 13, 2013, at 12:04 PM, Nico Weber <thakis at chromium.org> wrote:

> On Wed, Nov 13, 2013 at 9:17 AM, Argyrios Kyrtzidis <kyrtzidis at apple.com> wrote:
> 
> On Nov 12, 2013, at 10:00 PM, Nico Weber <thakis at chromium.org> wrote:
> 
>> On Tue, Nov 12, 2013 at 4:02 PM, Argyrios Kyrtzidis <kyrtzidis at apple.com> wrote:
>> 
>> On Nov 12, 2013, at 3:33 PM, Nico Weber <thakis at chromium.org> wrote:
>> 
>>> Wouldn't that mean that the build system had to launch clang for every pch file in noop builds?
>> 
>> Not necessarily, It would definitely have to launch it if there is going to be a clang invocation using the PCH file.
>> 
>>> And don't you want to force a full rebuild on compiler version changes anyhow?
>>> 
>> 
>> Again, not necessarily, incremental version changes are not supposed to change the semantics of your code.
>> 
>> Sure, it's not 100% necessary, but it's a good idea and should always be done in practice (say, if you track performance over time, you don't want half of your .o files built with an older compiler and half with a newer), right?
> 
> I did a quick check and it looks like ninja (on my llvm build) doesn't rebuild if the compiler changes; and xcodebuild doesn't either.
> 
> Ninja does what you tell it to do. You can certainly make gch files depend on the compiler if you want. You are in a position to change xcodebuild to do this too, I believe :-)
>  
> 
>> 
>> I can think of two things causing a PCH file to be invalid:
>> 
>> 1. It's invalid due to one of its dependency inputs (the compiler binary that generated it, the .h files it includes) changing. Build systems rebuild files whose dependency inputs change, and the compiler probably always writes valid PCH files (if it doesn't, that's just a bug in the compiler, and if the compiler has such a bug, regenerating the PCH file again will probably just write an invalid PCH file again).
>> 
>> 2. It's invalid due to interaction with the translation unit it's included from. This probably can't happen (maybe the PCH uses some module include that's not compatible with some other module used by the translation unit?), but if it does it can't be diagnosed by a standalone diagnostic pass (since that doesn't check the PCH file in interaction with other PCH files).
>> 
>> Did I miss a scenario, or are you worried about scenario 1 and your build system just doesn't track all dependencies for PCH files?
> 
> Yes, the build system does not track all dependencies for PCH files.
> Apart from the compiler version, neither ninja (at least in my llvm build) nor xcodebuild track system headers.
> 
> Ninja's philosophy that it's up to the generator to think of things like this. In Chromium, we considered giving gch files a dependency on the compiler, but ended up just deleting the build directory on clang updates as that was easier to do and was Good Enough for us.

Sure, flexibility is paramount. Noone is going to be forced to use this new flag, do you have specific concerns that make you opposed to this flag getting in ?

>  
> There is also a PCH dependency that is not even reported by clang. If you import a modularized header in modules-mode (e.g "#import <Foundation/Foundation.h>") clang should still report the headers as dependencies but a module file will also be created in the module cache that the PCH will point to. Now if you delete the module cache then the PCH is unusable.
> 
> Is that an issue for general dependency tracking too? Should the module cache be reported in the -MD output?

I prefer not. The module cache should not be leaking out, the whole mechanism is designed to be transparent, the dependencies reported should be the header files and the module.map if one exists (there's a bug related to modules and dependencies list but this is another issue..)

>  
> 
> The last one is a somewhat corner case, but it illustrates that delegating to clang is a good "catch-all" to check if the PCH needs rebuilding or not.
> 
> -Argyrios
> 
>>  
>> And the version is but one of multiple requirements for PCH verification.
>> 
>> 
>>  
>> 
>>> 
>>> On Tue, Nov 12, 2013 at 2:57 PM, Argyrios Kyrtzidis <kyrtzidis at apple.com> wrote:
>>> Hi all,
>>> 
>>> Clang may reject using a PCH file for various reasons, e.g. because it was created from a different clang version, because an include file was modified, etc.
>>> 
>>> I propose that we add an option like '--verify-pch /path/to/pch/file' in clang that when invoked will:
>>>   -Load the given PCH file
>>>   -Verify it (check compiler version, stat includes, etc.)
>>>   -Return 0 on success or non-zero and a diagnostic error on failure.
>>> 
>>> This can be used by a build system, if it so chooses, as part of its build dependencies calculation in order to verify that a PCH file is usable, and rebuild it if it is not.
>>> Clang is the "ultimate authority" on what constitutes a usable PCH file, so it makes sense to delegate to it.
>>> 
>>> Apart from making the experience better when the PCH file becomes stale in ways the build system does not detect (e.g. due to different compiler version), another advantage of this approach is to allow us to reduce the amount of PCH verification that clang does by only doing it once at the beginning of the build process.
>>> Some time ago we stopped stat'ing the system headers during PCH verification, since we considered it not worth the cost to stat them for every clang invocation. If the build system checked once at the beginning and then informed clang on subsequent invocations that it doesn't need to re-verify the PCH again, we could bring back verification of system headers.
>>> 
>>> Let me know what you think.
>>> 
>>> -Argyrios
>>> _______________________________________________
>>> cfe-dev mailing list
>>> cfe-dev at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>>> 
>>> _______________________________________________
>>> cfe-dev mailing list
>>> cfe-dev at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>> 
>> 
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20131113/b8b5175a/attachment.html>