[PATCH] D20867: [PCH] Fix timestamp check on windows hosts.

[Pierre Gousseau via cfe-commits]

It seems Phabricator did not picked up Richard's message so I have manually
copied and replied to it via Phabricator's web interface.

Cheers,

Pierre

On 6 June 2016 at 21:53, Richard Smith <richard at metafoo.co.uk> wrote:

> On Wed, Jun 1, 2016 at 8:33 AM, pierre gousseau via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
>> pgousseau created this revision.
>> pgousseau added reviewers: rsmith, thakis.
>> pgousseau added subscribers: cfe-commits, wristow, probinson, gbedwell,
>> bruno, cameron314.
>>
>> On Linux, if the timestamp of a header file, included in the pch, is
>> modified, then including the pch without regenerating it causes a fatal
>> error, which is reasonable.
>> On Windows the check is ifdefed out, allowing the compilation to continue
>> in a broken state.
>> The root of the broken state is that, if timestamps dont match, the
>> preprocessor will reparse a header without discarding the pch data.
>> This leads to "#pragma once" header to be included twice.
>> The reason behind the ifdefing of the check lacks documentation, and was
>> done 6 years ago.
>>
>
> It's documented in the comment you deleted:
>
>        // In our regression testing, the Windows file system seems to
>        // have inconsistent modification times that sometimes
>        // erroneously trigger this error-handling path.
>
> We should confirm whether this is in fact still the case. Maybe this is
> caused by building on a networked file system, where a locally-changed file
> might have a different mtime locally and remotely (the local mtime may be
> precise where the remote one has been rounded to a multiple of 2 seconds by
> an underlying FAT filesystem)? If it's something like that, we could
> perhaps work around this by rounding the mtime to a multiple of 2 seconds
> ourselves.
>
>
>> This change tentatively removes the ifdefing and adds a cc1 option to
>> disable the inclusion of timestamps in pch files, giving some flexibility
>> to build systems such as distributed builds.
>>
>> This change is a follow up to the discussion started in
>> http://reviews.llvm.org/D20243
>>
>> http://reviews.llvm.org/D20867
>>
>> Files:
>>   include/clang/Driver/CC1Options.td
>>   include/clang/Frontend/FrontendOptions.h
>>   lib/Frontend/CompilerInvocation.cpp
>>   lib/Frontend/FrontendActions.cpp
>>   lib/Serialization/ASTReader.cpp
>>   test/PCH/Inputs/include-timestamp-pch.h
>>   test/PCH/Inputs/include-timestamp.h
>>   test/PCH/include-timestamp.cpp
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160607/51096c9f/attachment-0001.html>