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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
