[PATCH] PR 17421: Implemented -save-temps={obj|cwd} option

Reid Kleckner rnk at google.com
Fri Jan 30 13:53:50 PST 2015


Looks good to me.


================
Comment at: include/clang/Driver/Driver.h:72
@@ +71,3 @@
+      // Diag - Forwarding function for diagnostics.
+      DiagnosticBuilder
+      Diag(unsigned DiagID) const {
----------------
Any reason for this whitespace change?

================
Comment at: lib/Driver/Driver.cpp:372
@@ +371,3 @@
+                    .Case("obj", SaveTempsObj)
+                    .Default(SaveTempsCwd); // Should we complain here instead?
+  }
----------------
Not worth it, IMO.

================
Comment at: lib/Driver/Driver.cpp:1792
@@ +1791,3 @@
+  if (!AtTopLevel && isSaveTempsObj() && JA.getType() != types::TY_PCH) {
+    SmallString<256> Result;
+    if (isSaveTempsObj() && C.getArgs().hasArg(options::OPT_o)) {
----------------
Looks unused

================
Comment at: lib/Driver/Driver.cpp:1793
@@ +1792,3 @@
+    SmallString<256> Result;
+    if (isSaveTempsObj() && C.getArgs().hasArg(options::OPT_o)) {
+      Arg *FinalOutput = C.getArgs().getLastArg(options::OPT_o);
----------------
No need to check isSaveTempsObj(), we already know it, right?

http://reviews.llvm.org/D7304

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






More information about the cfe-commits mailing list