MaskRay added inline comments.
================ Comment at: clang/lib/Frontend/CompilerInvocation.cpp:4244 + if (const Arg *A = Args.getLastArg(OPT_frandomize_layout_seed_file_EQ)) { + std::ifstream SeedFile(A->getValue(0)); ---------------- void wrote: > MaskRay wrote: > > Why is -frandomize-layout-seed-file= needed? Can't the user use something > > like -frandomize-layout-seed=$(<file) ? Or backquotes for POSIX sh > > compatibility? > > > > The impl uses the very uncommon header <fstream>. > That seems a bit clunky to me. If you don't like it, I can just remove the > option entirely. Wish you would have mentioned these concerns earlier...like > in the several weeks this has been in review. > > The `fstream` header is used in other places. If there's a better > alternative, please suggest one. > I was a subscriber only vaguely aware of this patch and mostly absent in the past 2 weeks on trips (which meant I spent really little time on reading patches) :) I just hope that every option added is useful. A thing that is not so necessarily can be delayed until it is actually needed. Just noticed that there is test coverage gap that the cc1 options are completely untested. There are unit tests, but no lit test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121556/new/ https://reviews.llvm.org/D121556 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits