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 <rich...@metafoo.co.uk> wrote: > On Wed, Jun 1, 2016 at 8:33 AM, pierre gousseau via cfe-commits < > cfe-commits@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@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> >> >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits