Re: [PATCH] D20243: [PCH] Disable inclusion of timestamps when generating pch files on windows.

2016-05-24 Thread pierre gousseau via cfe-commits
pgousseau added a comment. In http://reviews.llvm.org/D20243#437192, @thakis wrote: > Hm, the ASTReader code this works around is over 6 years old (r100866). Maybe > we could try enabling the access time check instead? Yes the issue described in ASTReader lacks a reproducible so it makes

Re: [PATCH] D20243: [PCH] Disable inclusion of timestamps when generating pch files on windows.

2016-05-24 Thread pierre gousseau via cfe-commits
pgousseau updated this revision to Diff 58207. pgousseau added a comment. Moving REQUIRE line higher following Bruno's comments. http://reviews.llvm.org/D20243 Files: lib/Frontend/FrontendActions.cpp lib/Serialization/ASTReader.cpp test/PCH/Inputs/pragma-once2-pch.h

Re: [PATCH] D20243: [PCH] Disable inclusion of timestamps when generating pch files on windows.

2016-05-23 Thread Nico Weber via cfe-commits
thakis added a comment. Hm, the ASTReader code this works around is over 6 years old (r100866). Maybe we could try enabling the access time check instead? http://reviews.llvm.org/D20243 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D20243: [PCH] Disable inclusion of timestamps when generating pch files on windows.

2016-05-23 Thread Bruno Cardoso Lopes via cfe-commits
bruno added a subscriber: bruno. bruno added a comment. Hi Pierre, Comment at: test/PCH/pragma-once-timestamp.cpp:17 @@ +16,3 @@ + +// REQUIRES: system-windows + Can you move this to the beginning of the file? It makes it easier to spot that this is windows

Re: [PATCH] D20243: [PCH] Disable inclusion of timestamps when generating pch files on windows.

2016-05-19 Thread pierre gousseau via cfe-commits
pgousseau added a comment. In http://reviews.llvm.org/D20243#433615, @thakis wrote: > Did you see http://reviews.llvm.org/D19815 ? Does that help? Warren might > have opinions on this. Yes thanks, I agree with Warren, this is a separate issue. In the test I am adding I avoid the issue that

Re: [PATCH] D20243: [PCH] Disable inclusion of timestamps when generating pch files on windows.

2016-05-18 Thread Warren Ristow via cfe-commits
wristow added a comment. In http://reviews.llvm.org/D20243#433615, @thakis wrote: > Did you see http://reviews.llvm.org/D19815 ? Does that help? Warren might > have opinions on this. Yes, these are definitely related. Fixing that other problem does //not //also fix the issue here, however

Re: [PATCH] D20243: [PCH] Disable inclusion of timestamps when generating pch files on windows.

2016-05-18 Thread Nico Weber via cfe-commits
thakis added a subscriber: thakis. thakis added a comment. Did you see http://reviews.llvm.org/D19815 ? Does that help? Warren might have opinions on this. http://reviews.llvm.org/D20243 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D20243: [PCH] Disable inclusion of timestamps when generating pch files on windows.

2016-05-18 Thread Cameron via cfe-commits
cameron314 added a subscriber: cameron314. cameron314 added a comment. I've seen rare cases where parses using the PCH files were yielding completely invalid data, almost as if they were corrupted. I wonder now if this could be the cause. (We're on Windows too.) http://reviews.llvm.org/D20243

Re: [PATCH] D20243: [PCH] Disable inclusion of timestamps when generating pch files on windows.

2016-05-18 Thread Paul Robinson via cfe-commits
probinson added a comment. This is fine as far as I'm concerned, but I think somebody more familiar with the area ought to chime in. http://reviews.llvm.org/D20243 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D20243: [PCH] Disable inclusion of timestamps when generating pch files on windows.

2016-05-16 Thread pierre gousseau via cfe-commits
pgousseau updated this revision to Diff 57324. pgousseau added a comment. Add newlines and change test's name following Paul's comments. http://reviews.llvm.org/D20243 Files: lib/Frontend/FrontendActions.cpp lib/Serialization/ASTReader.cpp test/PCH/Inputs/pragma-once2-pch.h

Re: [PATCH] D20243: [PCH] Disable inclusion of timestamps when generating pch files on windows.

2016-05-16 Thread pierre gousseau via cfe-commits
pgousseau added a comment. In http://reviews.llvm.org/D20243#429811, @probinson wrote: > Please make sure the test files have newlines at the end. > "touch-pragma-once.cpp" should probably be named something like > "pragma-once-timestamp.cpp". Will fix thanks!

Re: [PATCH] D20243: [PCH] Disable inclusion of timestamps when generating pch files on windows.

2016-05-13 Thread Paul Robinson via cfe-commits
probinson added a subscriber: probinson. probinson added a comment. Please make sure the test files have newlines at the end. "touch-pragma-once.cpp" should probably be named something like "pragma-once-timestamp.cpp". http://reviews.llvm.org/D20243

[PATCH] D20243: [PCH] Disable inclusion of timestamps when generating pch files on windows.

2016-05-13 Thread pierre gousseau via cfe-commits
pgousseau created this revision. pgousseau added a reviewer: rsmith. pgousseau added a subscriber: cfe-commits. On Linux, if a header file included in the pch is modified then a fatal error is emitted, which is reasonable. On Windows the check is ifdefed out, allowing the compilation to continue