aganea 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;
+
----------------
hans wrote:
> 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.
Reid said "pack" and "field" and my Pavlovian reflex made me think "bitfields".
Yes, plain bools are just fine.
================
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();
----------------
hans wrote:
> 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?
"burying", as in
https://github.com/llvm/llvm-project/blob/master/llvm/include/llvm/Support/BuryPointer.h
Changed the comment and the function to `enableFree()`.
================
Comment at: clang/lib/Driver/Job.cpp:325
+ llvm::erase_if(Arguments, RemoveDisableFree);
+}
+
----------------
hans wrote:
> 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.
That was my original intent, but the `-disable-free` flag is added by
`Clang::ConstructJob()` and at that point we don't know yet how many jobs we
will have, nor their nature.
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
