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)); ---------------- aaron.ballman wrote: > MaskRay wrote: > > 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. > > I just hope that every option added is useful. A thing that is not so > > necessarily can be delayed until it is actually needed. > > I think this option is useful. Windows' cmd.exe doesn't make it particularly > trivial to pipe contents to an argument in a command line, but also, IDEs > don't always make it obvious how you would pipe the seed content into a file > either. I don't see an issue with using `fstream` either; we use it when > necessary. Ah, LG! Just need some tests for the two -f -cc1 options. 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