bruno added a comment.

Maybe add some docs to explain the new flags?


================
Comment at: lib/Driver/Tools.cpp:6102
@@ +6101,3 @@
+        StatsFile.assign(Output.getFilename());
+        llvm::sys::path::remove_filename(StatsFile);
+      }
----------------
MatzeB wrote:
> bruno wrote:
> > Why removing StatsFile here? IIUC, at this point StatsFile is still the 
> > same as the output (if it's a file).
> a) It behaves like -save-temps=obj (`clang -save-temps=obj foo.c -o bar` will 
> give you foo.i, foo.o, ...;
> b) This makes sense when you have multiple (`clang -save-stats=obj foo.c 
> bar.c -o baz`) inputs for which you need multiple .stats filenames but you 
> only have 1 output name 
> c) It magically works for `-o -` as well
> 
Ok, I see now, I misread `remove_filename` as `remove`

================
Comment at: test/Driver/save-stats.c:1
@@ +1,2 @@
+// RUN: %clang -target x86_64-apple-darwin -save-stats %s -### 2>&1 | 
FileCheck %s
+// RUN: %clang -target x86_64-apple-darwin -save-stats=cwd %s -### 2>&1 | 
FileCheck %s
----------------
Is -save-stats == -save-stats=cwd? It doesn't seem so by looking at 
lib/Driver/Tools.cpp. Need test for the diag::err_drv_invalid_value as well.

================
Comment at: test/Driver/save-stats.c:12
@@ +11,3 @@
+// RUN: %clang -target x86_64-apple-darwin -save-stats=obj -c -o 
obj/dir/save-stats.o %s -### 2>&1 | FileCheck %s -check-prefix=CHECK-OBJ
+// CHECK-OBJ: "-stats-file=obj/dir{{/|\\\\}}save-stats.stats"
+// CHECK-OBJ: "-o" "obj/dir{{/|\\\\}}save-stats.o"
----------------
aprantl wrote:
> This is up to taste, but just accepting {{.}} as the path separator would be 
> sufficient IMO and might be more readable.
+1 to Adrian's suggestion


Repository:
  rL LLVM

https://reviews.llvm.org/D24820



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to