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

[pierre gousseau via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL275261: [PCH] Fix timestamp check on windows hosts. 
(authored by pgousseau).

Changed prior to commit:
  http://reviews.llvm.org/D20867?vs=59887=63794#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D20867

Files:
  cfe/trunk/lib/Serialization/ASTReader.cpp

Index: cfe/trunk/lib/Serialization/ASTReader.cpp
===
--- cfe/trunk/lib/Serialization/ASTReader.cpp
+++ cfe/trunk/lib/Serialization/ASTReader.cpp
@@ -2010,17 +2010,8 @@
   // For an overridden file, there is nothing to validate.
   if (!Overridden && //
   (StoredSize != File->getSize() ||
-#if defined(LLVM_ON_WIN32)
-   false
-#else
-   // In our regression testing, the Windows file system seems to
-   // have inconsistent modification times that sometimes
-   // erroneously trigger this error-handling path.
-   //
-   // FIXME: This probably also breaks HeaderFileInfo lookups on Windows.
(StoredTime && StoredTime != File->getModificationTime() &&
 !DisableValidation)
-#endif
)) {
 if (Complain) {
   // Build a list of the PCH imports that got us here (in reverse).


Index: cfe/trunk/lib/Serialization/ASTReader.cpp
===
--- cfe/trunk/lib/Serialization/ASTReader.cpp
+++ cfe/trunk/lib/Serialization/ASTReader.cpp
@@ -2010,17 +2010,8 @@
   // For an overridden file, there is nothing to validate.
   if (!Overridden && //
   (StoredSize != File->getSize() ||
-#if defined(LLVM_ON_WIN32)
-   false
-#else
-   // In our regression testing, the Windows file system seems to
-   // have inconsistent modification times that sometimes
-   // erroneously trigger this error-handling path.
-   //
-   // FIXME: This probably also breaks HeaderFileInfo lookups on Windows.
(StoredTime && StoredTime != File->getModificationTime() &&
 !DisableValidation)
-#endif
)) {
 if (Complain) {
   // Build a list of the PCH imports that got us here (in reverse).
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[pierre gousseau via cfe-commits]

pgousseau added a comment.

In http://reviews.llvm.org/D20867#482465, @rsmith wrote:

> You have two independent functional changes in this patch: one adds a flag to 
> control the emission of timestamps into PCH files, and the other re-enables 
> timestamp checking on Win32. Please separate them out into distinct patches 
> to be committed separately.
>
> Both parts of this LGTM.


Splitting the patch sounds good, thanks Richard and Bruno for reviewing, will  
push soon.


http://reviews.llvm.org/D20867



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Richard Smith via cfe-commits]

rsmith added a comment.

You have two independent functional changes in this patch: one adds a flag to 
control the emission of timestamps into PCH files, and the other re-enables 
timestamp checking on Win32. Please separate them out into distinct patches to 
be committed separately.

Both parts of this LGTM.


http://reviews.llvm.org/D20867



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[pierre gousseau via cfe-commits]

pgousseau added a comment.

Ping!


http://reviews.llvm.org/D20867



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[pierre gousseau via cfe-commits]

pgousseau added a comment.

Ping!


http://reviews.llvm.org/D20867



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[pierre gousseau via cfe-commits]

pgousseau added a comment.

Ping!


http://reviews.llvm.org/D20867



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[pierre gousseau via cfe-commits]

pgousseau added a comment.

Ping!


http://reviews.llvm.org/D20867



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[pierre gousseau via cfe-commits]

pgousseau added a comment.

> > Are you asking for the 2 seconds tolerance to be part of this patch?

> 

> >  Or, as it seems the main problem here is the lack of a reproducible, are 
> > you ok with the idea of committing this patch first, to see if the 
> > inconsistent time stamps is still an issue?

> 

> 

> Yes, let's commit a patch to remove the #if first, and see if anything 
> actually breaks (and if so, what). There's no need to add an option for this 
> we have no uses for it.


Sounds good, does it mean it is ok to commit?
Sorry, regarding the option, not sure if you are saying: "if we have no uses 
for it" or "as we have no uses for it"?
The idea behind the option is to allow distributed builds system, that do not 
preserve time stamps, to use PCH files.


http://reviews.llvm.org/D20867



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Richard Smith via cfe-commits]

On 7 Jun 2016 7:52 a.m., "pierre gousseau" 
wrote:
>
> pgousseau added a comment.
>
> > > 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.
>
>
> I am not sure how to reproduce this build scenario, would you be able to
provide some more stepped details?
> I have tried emitting and including a PCH on a networked FAT32 drive, but
no false warnings observed so far.
>
> Are you asking for the 2 seconds tolerance to be part of this patch?
> Or, as it seems the main problem here is the lack of a reproducible, are
you ok with the idea of committing this patch first, to see if the
inconsistent time stamps is still an issue?

Yes, let's commit a patch to remove the #if first, and see if anything
actually breaks (and if so, what). There's no need to add an option for
this we have no uses for it.

> http://reviews.llvm.org/D20867
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [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  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

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

[pierre gousseau via cfe-commits]

pgousseau added a comment.

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


I am not sure how to reproduce this build scenario, would you be able to 
provide some more stepped details?
I have tried emitting and including a PCH on a networked FAT32 drive, but no 
false warnings observed so far.

Are you asking for the 2 seconds tolerance to be part of this patch?
Or, as it seems the main problem here is the lack of a reproducible, are you ok 
with the idea of committing this patch first, to see if the inconsistent time 
stamps is still an issue?


http://reviews.llvm.org/D20867



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[pierre gousseau via cfe-commits]

pgousseau updated this revision to Diff 59887.
pgousseau added a comment.

Following Bruno's comment:

- Remove call to sleep using touch -m -a -t


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/pragma-once2-pch.h
  test/PCH/Inputs/pragma-once2.h
  test/PCH/include-timestamp.cpp

Index: test/PCH/include-timestamp.cpp
===
--- /dev/null
+++ test/PCH/include-timestamp.cpp
@@ -0,0 +1,27 @@
+// Test that the timestamp is not included in the produced pch file with
+// -fno-pch-timestamp.
+
+// Check timestamp is included by default.
+// RUN: %clang_cc1 -x c++-header -emit-pch -o %t %S/Inputs/pragma-once2-pch.h
+// RUN: touch -m -a -t 201008011501 %S/Inputs/pragma-once2.h
+// RUN: not %clang_cc1 -include-pch %t %s 2>&1 | FileCheck -check-prefix=CHECK-TIMESTAMP %s
+
+// Check bitcode output as well.
+// RUN: llvm-bcanalyzer -dump %t | FileCheck -check-prefix=CHECK-BITCODE-TIMESTAMP-ON %s
+
+// Check timestamp inclusion is disabled by -fno-pch-timestamp.
+// RUN: %clang_cc1 -x c++-header -emit-pch -o %t %S/Inputs/pragma-once2-pch.h -fno-pch-timestamp
+// RUN: touch -m -a -t 201008011502 %S/Inputs/pragma-once2.h
+// RUN: %clang_cc1 -include-pch %t %s 2>&1
+
+// Check bitcode output as well.
+// RUN: llvm-bcanalyzer -dump %t | FileCheck -check-prefix=CHECK-BITCODE-TIMESTAMP-OFF %s
+
+#include "Inputs/pragma-once2.h"
+
+void g() { f(); }
+
+// CHECK-BITCODE-TIMESTAMP-ON: getSize() ||
-#if defined(LLVM_ON_WIN32)
-   false
-#else
-   // In our regression testing, the Windows file system seems to
-   // have inconsistent modification times that sometimes
-   // erroneously trigger this error-handling path.
-   //
-   // FIXME: This probably also breaks HeaderFileInfo lookups on Windows.
(StoredTime && StoredTime != File->getModificationTime() &&
 !DisableValidation)
-#endif
)) {
 if (Complain) {
   // Build a list of the PCH imports that got us here (in reverse).
Index: lib/Frontend/FrontendActions.cpp
===
--- lib/Frontend/FrontendActions.cpp
+++ lib/Frontend/FrontendActions.cpp
@@ -92,7 +92,10 @@
   std::vector Consumers;
   Consumers.push_back(llvm::make_unique(
 CI.getPreprocessor(), OutputFile, nullptr, Sysroot,
-Buffer, CI.getFrontendOpts().ModuleFileExtensions));
+Buffer, CI.getFrontendOpts().ModuleFileExtensions,
+/*AllowASTWithErrors*/false,
+/*IncludeTimestamps*/
+  +CI.getFrontendOpts().IncludeTimestamps));
   Consumers.push_back(CI.getPCHContainerWriter().CreatePCHContainerGenerator(
   CI, InFile, OutputFile, OS, Buffer));
 
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1144,6 +1144,7 @@
   Opts.ModuleFiles = Args.getAllArgValues(OPT_fmodule_file);
   Opts.ModulesEmbedFiles = Args.getAllArgValues(OPT_fmodules_embed_file_EQ);
   Opts.ModulesEmbedAllFiles = Args.hasArg(OPT_fmodules_embed_all_files);
+  Opts.IncludeTimestamps = !Args.hasArg(OPT_fno_pch_timestamp);
 
   Opts.CodeCompleteOpts.IncludeMacros
 = Args.hasArg(OPT_code_completion_macros);
Index: include/clang/Frontend/FrontendOptions.h
===
--- include/clang/Frontend/FrontendOptions.h
+++ include/clang/Frontend/FrontendOptions.h
@@ -153,6 +153,8 @@
///< implicit module build.
   unsigned ModulesEmbedAllFiles : 1;   ///< Whether we should embed all used
///< files into the PCM file.
+  unsigned IncludeTimestamps : 1;  ///< Whether timestamps should be
+   ///< written to the produced PCH file.
 
   CodeCompleteOptions CodeCompleteOpts;
 
@@ -277,8 +279,8 @@
 SkipFunctionBodies(false), UseGlobalModuleIndex(true),
 GenerateGlobalModuleIndex(true), ASTDumpDecls(false), ASTDumpLookups(false),
 BuildingImplicitModule(false), ModulesEmbedAllFiles(false),
-ARCMTAction(ARCMT_None), ObjCMTAction(ObjCMT_None),
-ProgramAction(frontend::ParseSyntaxOnly)
+IncludeTimestamps(true), ARCMTAction(ARCMT_None),
+ObjCMTAction(ObjCMT_None), ProgramAction(frontend::ParseSyntaxOnly)
   {}
 
   /// getInputKindForExtension - Return the appropriate input kind for a file
Index: include/clang/Driver/CC1Options.td
===
--- include/clang/Driver/CC1Options.td
+++ include/clang/Driver/CC1Options.td
@@ -507,6 

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

[Richard Smith via cfe-commits]

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

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

[pierre gousseau via cfe-commits]

pgousseau added inline comments.


Comment at: test/PCH/include-timestamp.cpp:8
@@ +7,3 @@
+// RUN: sleep 1
+// RUN: not %clang_cc1 -include-pch %t %s 2>&1 | FileCheck 
-check-prefix=CHECK-TIMESTAMP %s
+

Without the sleep the test fails for me, as it seems the call to touch does not 
have time to take effect. It might be something specific to gnuwin32 tools on 
Windows I am not sure.


http://reviews.llvm.org/D20867



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[pierre gousseau via cfe-commits]

pgousseau updated this revision to Diff 59355.
pgousseau added a comment.

Following Bruno's comments:

- Update input files' name to match http://reviews.llvm.org/D20243 review.
- Add llvm-bcanalyzer checks


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/pragma-once2-pch.h
  test/PCH/Inputs/pragma-once2.h
  test/PCH/include-timestamp.cpp

Index: test/PCH/include-timestamp.cpp
===
--- /dev/null
+++ test/PCH/include-timestamp.cpp
@@ -0,0 +1,29 @@
+// Test that the timestamp is not included in the produced pch file with
+// -fno-pch-timestamp.
+
+// Check timestamp is included by default.
+// RUN: %clang_cc1 -x c++-header -emit-pch -o %t %S/Inputs/pragma-once2-pch.h
+// RUN: touch %S/Inputs/pragma-once2.h
+// RUN: sleep 1
+// RUN: not %clang_cc1 -include-pch %t %s 2>&1 | FileCheck -check-prefix=CHECK-TIMESTAMP %s
+
+// Check bitcode output as well.
+// RUN: llvm-bcanalyzer -dump %t | FileCheck -check-prefix=CHECK-BITCODE-TIMESTAMP-ON %s
+
+// Check timestamp inclusion is disabled by -fno-pch-timestamp.
+// RUN: %clang_cc1 -x c++-header -emit-pch -o %t %S/Inputs/pragma-once2-pch.h -fno-pch-timestamp
+// RUN: touch %S/Inputs/pragma-once2.h
+// RUN: sleep 1
+// RUN: %clang_cc1 -include-pch %t %s 2>&1
+
+// Check bitcode output as well.
+// RUN: llvm-bcanalyzer -dump %t | FileCheck -check-prefix=CHECK-BITCODE-TIMESTAMP-OFF %s
+
+#include "Inputs/pragma-once2.h"
+
+void g() { f(); }
+
+// CHECK-BITCODE-TIMESTAMP-ON: getSize() ||
-#if defined(LLVM_ON_WIN32)
-   false
-#else
-   // In our regression testing, the Windows file system seems to
-   // have inconsistent modification times that sometimes
-   // erroneously trigger this error-handling path.
-   //
-   // FIXME: This probably also breaks HeaderFileInfo lookups on Windows.
(StoredTime && StoredTime != File->getModificationTime() &&
 !DisableValidation)
-#endif
)) {
 if (Complain) {
   // Build a list of the PCH imports that got us here (in reverse).
Index: lib/Frontend/FrontendActions.cpp
===
--- lib/Frontend/FrontendActions.cpp
+++ lib/Frontend/FrontendActions.cpp
@@ -92,7 +92,10 @@
   std::vector Consumers;
   Consumers.push_back(llvm::make_unique(
 CI.getPreprocessor(), OutputFile, nullptr, Sysroot,
-Buffer, CI.getFrontendOpts().ModuleFileExtensions));
+Buffer, CI.getFrontendOpts().ModuleFileExtensions,
+/*AllowASTWithErrors*/false,
+/*IncludeTimestamps*/
+  +CI.getFrontendOpts().IncludeTimestamps));
   Consumers.push_back(CI.getPCHContainerWriter().CreatePCHContainerGenerator(
   CI, InFile, OutputFile, OS, Buffer));
 
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1144,6 +1144,7 @@
   Opts.ModuleFiles = Args.getAllArgValues(OPT_fmodule_file);
   Opts.ModulesEmbedFiles = Args.getAllArgValues(OPT_fmodules_embed_file_EQ);
   Opts.ModulesEmbedAllFiles = Args.hasArg(OPT_fmodules_embed_all_files);
+  Opts.IncludeTimestamps = !Args.hasArg(OPT_fno_pch_timestamp);
 
   Opts.CodeCompleteOpts.IncludeMacros
 = Args.hasArg(OPT_code_completion_macros);
Index: include/clang/Frontend/FrontendOptions.h
===
--- include/clang/Frontend/FrontendOptions.h
+++ include/clang/Frontend/FrontendOptions.h
@@ -153,6 +153,8 @@
///< implicit module build.
   unsigned ModulesEmbedAllFiles : 1;   ///< Whether we should embed all used
///< files into the PCM file.
+  unsigned IncludeTimestamps : 1;  ///< Whether timestamps should be
+   ///< written to the produced PCH file.
 
   CodeCompleteOptions CodeCompleteOpts;
 
@@ -277,8 +279,8 @@
 SkipFunctionBodies(false), UseGlobalModuleIndex(true),
 GenerateGlobalModuleIndex(true), ASTDumpDecls(false), ASTDumpLookups(false),
 BuildingImplicitModule(false), ModulesEmbedAllFiles(false),
-ARCMTAction(ARCMT_None), ObjCMTAction(ObjCMT_None),
-ProgramAction(frontend::ParseSyntaxOnly)
+IncludeTimestamps(true), ARCMTAction(ARCMT_None),
+ObjCMTAction(ObjCMT_None), ProgramAction(frontend::ParseSyntaxOnly)
   {}
 
   /// getInputKindForExtension - Return the appropriate input kind for a file
Index: include/clang/Driver/CC1Options.td
===
--- include/clang/Driver/CC1Options.td

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

[Bruno Cardoso Lopes via cfe-commits]

bruno added inline comments.


Comment at: test/PCH/include-timestamp.cpp:5
@@ +4,3 @@
+// Check timestamp is included by default.
+// RUN: %clang_cc1 -x c++-header -emit-pch -o %t 
%S/Inputs/include-timestamp-pch.h
+// RUN: touch %S/Inputs/include-timestamp.h

Can you also check the timestamp by looking at llvm-bcanalyzer output (assuming 
the tool outputs such info)?


Comment at: test/PCH/include-timestamp.cpp:7
@@ +6,3 @@
+// RUN: touch %S/Inputs/include-timestamp.h
+// RUN: sleep 1
+// RUN: not %clang_cc1 -include-pch %t %s 2>&1 | FileCheck 
-check-prefix=CHECK-TIMESTAMP %s

Why use `sleep 1` here?


Comment at: test/PCH/include-timestamp.cpp:16
@@ +15,3 @@
+
+#include "Inputs/include-timestamp.h"
+

It make sense for the context of this patch, but if 
http://reviews.llvm.org/D20243 gets in, you should re-use the same headers 
you're adding there.


http://reviews.llvm.org/D20867



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits