[PATCH] D19551: Handle Windows drive letters and ".." for --reproduce.

Davide Italiano via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 26 13:20:04 PDT 2016


davide added inline comments.

================
Comment at: ELF/Driver.cpp:258
@@ -243,6 +257,3 @@
 
-static void dumpLinkerInvocation(ArrayRef<const char *> Args) {
-  std::error_code EC = sys::fs::create_directories(Config->Reproduce,
-    false /* IgnoreExisting */);
-  if (EC) {
-    error(EC, "--reproduce: can't create directory");
+static void logCommandline(ArrayRef<const char *> Args) {
+  if (std::error_code EC = sys::fs::create_directories(
----------------
ruiu wrote:
> davide wrote:
> > I prefer dumpLinkerInvocation still.
> To me this name is more plain and therefore good. It just logs command line -- I can tell it would obviously have no side effect (except writing a log somewhere) just by looking at the name. But dumpLinkerInvocation sounds something more complex.
As you wish. Discussing this is probably not worth more time.

================
Comment at: ELF/Driver.cpp:272
@@ -257,3 +271,3 @@
   OS << Args[0];
-  for (unsigned I = 1, E = Args.size(); I < E; ++I)
+  for (size_t I = 1, E = Args.size(); I < E; ++I)
     OS << " " << Args[I];
----------------
ruiu wrote:
> davide wrote:
> > We use unsigned everywhere else in the code base, try to be consistent or change all the other occurrences.
> I don't think we do. I actually use size_t where it means the size of an array. This is also technically accurate (even though it is unrealistic that we have more than 2^32 arguments).
I think these should be changed then:

Writer.cpp:  for (unsigned I = 0; I < OutputSections.size(); ++I) {
OutputSections.cpp:    for (unsigned I = 0, N = Offsets.size(); I != N; ++I) {

I don't care which one we use just try to use the same everywhere. Feel free to change in another patch.


http://reviews.llvm.org/D19551





More information about the llvm-commits mailing list