[PATCH] Support generating Jom-style depfiles.

Sean Silva chisophugis at gmail.com
Fri Apr 24 14:37:02 PDT 2015


Mimicking the existing precedent from another compiler for the option name makes sense (actually wasn't this at least in one of SCE's GCC derivatives?, so the sources ought to be out there in open source somewhere...). The other alternative I can think of is `-Msyntax={make,nmake}`, but I'm fine with `-MV`.

Also, since this feature is not in GCC, it requires documentation in docs/UsersManual.rst


================
Comment at: lib/Frontend/DependencyFile.cpp:153
@@ -152,2 +152,3 @@
   bool IncludeModuleFiles;
+  bool UseDoubleQuotes;
 private:
----------------
Maybe a comment explaining why you would want double quotes? Alternatively, maybe instead of "bool" it could be `enum DepfileSyntax { Make, NMake }` or something like that.

================
Comment at: lib/Frontend/DependencyFile.cpp:298
@@ +297,3 @@
+                          bool UseDoubleQuotes) {
+  if (UseDoubleQuotes) {
+    // Add quotes only if needed.
----------------
Is there a "spec" you can link to explaining the syntax? IIRC the microsoft docs for nmake have something explaining the quoting scheme.

================
Comment at: lib/Frontend/DependencyFile.cpp:300
@@ +299,3 @@
+    // Add quotes only if needed.
+    if (Filename.find_first_of(" #$") != StringRef::npos)
+      OS << '\"' << Filename << '\"';
----------------
I guess `#include <foo"bar.h>` is "don't do that"? I'm fine with that; I think the standard actually gives us a lot of freedom in what we can accept here; as long as we don't break use cases we care about it's all "implementation defined". We currently accept this though with no warning. If NMake doesn't have a way to quote this then maybe we should consider a "unadvisable include name" warning.

http://reviews.llvm.org/D9260

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the cfe-commits mailing list