hans added inline comments.
================
Comment at: clang/include/clang/Driver/Job.h:90
+ /// Whether the command will be executed in this process or not.
+ bool InProcess : 1;
+
----------------
I think Reid just meant put the bool fields after each other to minimize
padding. Using bitfields is taking it one step further. I think in LLVM we
generally use "unsigned" for bitfield types, but I'm not sure using bitfields
at all is worth it here.
================
Comment at: clang/include/clang/Driver/Job.h:134
- /// Set whether to print the input filenames when executing.
- void setPrintInputFilenames(bool P) { PrintInputFilenames = P; }
+ /// Prevent burying pointers, and ensure we free everything after execution.
+ void forceCleanUp();
----------------
I hadn't heard the term "burying pointers" before, and the name is also pretty
broad: "clean up" could refer to a lot more than just freeing memory.
Maybe "enableFree()" or something would be a better name?
================
Comment at: clang/lib/Driver/Driver.cpp:3758
+ JobList &Jobs = C.getJobs();
+ if (!Jobs.empty()) {
+ Command &Last = *Jobs.getJobs().back();
----------------
Is this if-statement really necessary? If you just do the for-loop directly, it
will not execute if Jobs is empty anyway.
Also, with an old-school for-loop it's easier to not iterate over the last
element. How about something like:
```
for (size_t i = 0, e = C.getJobs().size(); i + 1 < e; i++) {
auto &Job = C.getJobs().getJobs()[i];
if (Job.InProcess)
Job.forceCleanup();
}
```
================
Comment at: clang/lib/Driver/Job.cpp:325
+ llvm::erase_if(Arguments, RemoveDisableFree);
+}
+
----------------
I wish it were possible to avoid adding the -disable-free flag to
in-process-cc1 jobs in the first place, but I guess maybe that's not practical.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74447/new/
https://reviews.llvm.org/D74447
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
