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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
