[PATCH] Teach Clang how to use response files when calling other tools

Rafael Auler rafaelauler at gmail.com
Mon Sep 1 22:23:56 PDT 2014


Sean,

Thanks for the fast reply. I answered some of your comments below. The remaining comments were fixed in the next patch (will upload it now).

Best regards,
Rafael Auler

================
Comment at: lib/Driver/Job.cpp:205
@@ +204,3 @@
+    ResponseFlag = Creator.getResponseFileFlag();
+    ResponseFlag += ResponseFile.data();
+    if (RespFileSup == Tool::RF_FileList)
----------------
silvas wrote:
> I don't think this is correct. ResponseFile.data() isn't guaranteed to be 0 terminated (and you don't want to embed that hidden assumption). Maybe use .append(begin, end)?
Great observation. I'll change ResponseFile back to a const char * because, otherwise, I would need to copy it to a null-terminated string storage, since I may need to directly store it in the argv vector (which only contains null-terminated strings).

================
Comment at: lib/Driver/Job.cpp:216-232
@@ -104,16 +215,19 @@
+
+  for (size_t i = 0, e = Args.size(); i < e; ++i) {
+    const char *const Arg = Args[i];
 
     if (CrashReport) {
       if (int Skip = skipArgs(Arg)) {
         i += Skip - 1;
         continue;
       }
     }
 
     OS << ' ';
     PrintArg(OS, Arg, Quote);
 
     if (CrashReport && quoteNextArg(Arg) && i + 1 < e) {
       OS << ' ';
-      PrintArg(OS, Arguments[++i], true);
+      PrintArg(OS, Args[++i], true);
     }
   }
----------------
silvas wrote:
> This loop scares me. Could you clean this up in a separate patch? Or at least add comment here for now.
Sure, will work on it in a separate patch.

================
Comment at: lib/Driver/Tools.h:531-537
@@ -441,1 +530,9 @@
+
+    ResponseFileSupport getResponseFilesSupport() const override {
+      return RF_Full;
+    }
+    llvm::sys::fs::WindowsEncodingMethod
+    getResponseFileEncoding() const override {
+      return llvm::sys::fs::WEM_CurrentCodePage;
+    }
   };
----------------
silvas wrote:
> This seems highly repetitive. Is there a way to reduce it?
> 
> Also, I'm not sure that it makes sense to have a method returning WindowsEncodingMethod for a netbsd tool (same goes for most tools that have no reason to run on windows).
WindowsEncodingMethod, when used for UNIX tools, covers the case where these tools run in MinGW environments (in a cross-compilation setup).

http://reviews.llvm.org/D4897






More information about the cfe-commits mailing list