void 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));
----------------
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.
================
Comment at: clang/lib/Sema/SemaDecl.cpp:17852
+ !Record->isRandomized()) {
+ SmallVector<Decl *, 32> OrigFieldOrdering(Record->fields());
+ SmallVector<Decl *, 32> NewFieldOrdering;
----------------
MaskRay wrote:
> 32 may be too large. Consider the default inlined number of elements (fit the
> vector in 64 bytes).
Linux has many structures with far more fields. In fact, it's probably not
useful to randomize structures with a small number of fields.
================
Comment at: clang/unittests/AST/RandstructTest.cpp:41
+
+std::unique_ptr<ASTUnit> makeAST(const std::string &SourceCode,
+ bool ExpectErr = false) {
----------------
MaskRay wrote:
> Use static. See
> https://llvm.org/docs/CodingStandards.html#anonymous-namespaces
Nah.
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
