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

[Nico Weber]

On Wed, Nov 13, 2013 at 12:27 PM, Argyrios Kyrtzidis <kyrtzidis at apple.com>wrote:

>
> 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 ?
>

No, sorry if I gave this impression! It was just a general "this design
sounds like your build system will be slower than necessary" comment. (And
it sounds to me that just regular timestamp-based dependency tracking is
enough to express what you need.)


>
>
>
>> 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,
>

It sounds like you described how this does already leak out when used with
PCH files? Maybe I misunderstood your comment.


> 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/056b92ee/attachment.html>