<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Aug 11, 2015 at 11:49 AM, Manuel Klimek <span dir="ltr"><<a href="mailto:klimek@google.com" target="_blank">klimek@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Back in the day I'm pretty sure you agreed with the general concept :)</div></blockquote><div><br></div><div><div>Yeah, I don't think I fully understood the reason for having a cc1 -module-file-deps flag at the time.</div></div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><a href="http://reviews.llvm.org/D8378" target="_blank">http://reviews.llvm.org/D8378</a><br></div><div>I remember that we had problems with absolute paths ending up in the .d file (which should not happen if none of the paths provided are absolute), but I don't remember why we went for not writing them to the .d file instead of fixing the underlying problem.</div></div></blockquote><div><br></div><div>Here's the situation from before your change:<br></div><div><br></div><div> * Names of (implicit) module files were not written to the .pcm file; they're an implementation detail of the compilation process and not an actual input, so this is correct behavior (the input files used to *build* the implicit modules do get written to the .pcm file, which again seems right)<br></div><div> * As a special case, when creating a .d file for a .pch file, implicit module files *were* written to the .d file. This is unfortunate but necessary, because rebuilding a .pcm file in the module cache does not give a bit-for-bit identical result (sometimes just the signature changes, sometimes the .pcm is built with a different configuration in a way that the module cache hash doesn't detect, and very occasionally there's a hash collision).</div><div><br></div><div>The -module-file-deps flag was an implementation detail of the .pch building process, as a way of the driver telling the frontend to include .pcm files in the produced .d file. The only effect of the flag added here was to turn off this bugfix for PCH files; I don't think this is a useful flag.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div class="h5"><div class="gmail_quote"><div dir="ltr">On Tue, Aug 11, 2015 at 8:41 PM Richard Smith <<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span>On Thu, Mar 19, 2015 at 5:00 AM, Manuel Klimek <span dir="ltr"><<a href="mailto:klimek@google.com" target="_blank">klimek@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: klimek<br>
Date: Thu Mar 19 07:00:22 2015<br>
New Revision: 232721<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=232721&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=232721&view=rev</a><br>
Log:<br>
Add option to switch off putting header modules into the dependency file.<br></blockquote><div><br></div></span><div>Why? It is not correct to use a .pch file after the corresponding .pcm file has been rebuilt. And why do you want a switch to turn on adding implicit module files to the .d file? (I think the right behavior here is that explicit modules are always listed, and implicit modules are never listed except when building a .pch file.)</div><div><div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Modified:<br>
cfe/trunk/include/clang/Driver/Options.td<br>
cfe/trunk/lib/Driver/Tools.cpp<br>
cfe/trunk/test/Driver/pch-deps.c<br>
<br>
Modified: cfe/trunk/include/clang/Driver/Options.td<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=232721&r1=232720&r2=232721&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=232721&r1=232720&r2=232721&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Driver/Options.td (original)<br>
+++ cfe/trunk/include/clang/Driver/Options.td Thu Mar 19 07:00:22 2015<br>
@@ -772,6 +772,10 @@ def fno_modules_strict_decluse : Flag <[<br>
Flags<[DriverOption]>;<br>
def fimplicit_modules : Flag <["-"], "fimplicit-modules">, Group<f_Group>,<br>
Flags<[DriverOption]>;<br>
+def fmodule_file_deps : Flag <["-"], "fmodule-file-deps">, Group<f_Group>,<br>
+ Flags<[DriverOption]>;<br>
+def fno_module_file_deps : Flag <["-"], "fno-module-file-deps">, Group<f_Group>,<br>
+ Flags<[DriverOption]>;<br>
def fno_ms_extensions : Flag<["-"], "fno-ms-extensions">, Group<f_Group>;<br>
def fno_ms_compatibility : Flag<["-"], "fno-ms-compatibility">, Group<f_Group>;<br>
def fno_delayed_template_parsing : Flag<["-"], "fno-delayed-template-parsing">, Group<f_Group>;<br>
<br>
Modified: cfe/trunk/lib/Driver/Tools.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=232721&r1=232720&r2=232721&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=232721&r1=232720&r2=232721&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Driver/Tools.cpp (original)<br>
+++ cfe/trunk/lib/Driver/Tools.cpp Thu Mar 19 07:00:22 2015<br>
@@ -324,8 +324,9 @@ void Clang::AddPreprocessingOptions(Comp<br>
if (A->getOption().matches(options::OPT_M) ||<br>
A->getOption().matches(options::OPT_MD))<br>
CmdArgs.push_back("-sys-header-deps");<br>
-<br>
- if (isa<PrecompileJobAction>(JA))<br>
+ if ((isa<PrecompileJobAction>(JA) &&<br>
+ !Args.hasArg(options::OPT_fno_module_file_deps)) ||<br>
+ Args.hasArg(options::OPT_fmodule_file_deps))<br>
CmdArgs.push_back("-module-file-deps");<br>
}<br>
<br>
<br>
Modified: cfe/trunk/test/Driver/pch-deps.c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/pch-deps.c?rev=232721&r1=232720&r2=232721&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/pch-deps.c?rev=232721&r1=232720&r2=232721&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/Driver/pch-deps.c (original)<br>
+++ cfe/trunk/test/Driver/pch-deps.c Thu Mar 19 07:00:22 2015<br>
@@ -8,3 +8,14 @@<br>
// RUN: FileCheck %s -check-prefix=CHECK-NOPCH -input-file=%t<br>
// CHECK-NOPCH: -dependency-file<br>
// CHECK-NOPCH-NOT: -module-file-deps<br>
+<br>
+// RUN: %clang -x c-header %s -o %t.pch -MMD -MT dependencies -MF %t.d \<br>
+// RUN: -fno-module-file-deps -### 2> %t<br>
+// RUN: FileCheck %s -check-prefix=CHECK-EXPLICIT -input-file=%t<br>
+// CHECK-EXPLICIT: -dependency-file<br>
+// CHECK-EXPLICIT-NOT: -module-file-deps<br>
+<br>
+// RUN: %clang -x c++ %s -o %t.o -MMD -MT dependencies -MF %t.d -fmodule-file-deps -### 2> %t<br>
+// RUN: FileCheck %s -check-prefix=CHECK-EXPLICIT-NOPCH -input-file=%t<br>
+// CHECK-EXPLICIT-NOPCH: -dependency-file<br>
+// CHECK-EXPLICIT-NOPCH: -module-file-deps<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div></div></div><br></div></div>
</div><br></div>
</blockquote></div></div></div></div>
</blockquote></div><br></div></div>