[PATCH] D123636: [randstruct] Add test for "-frandomize-layout-seed-file" flag

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.


================
Comment at: clang/unittests/AST/RandstructTest.cpp:40
 
-static std::unique_ptr<ASTUnit> makeAST(const std::string &SourceCode) {
+static std::string Seed = "1234567890abcdef";
+
----------------
`constexpr llvm::StringLiteral Seed = ...` or `constexpr char Seed[]`


================
Comment at: clang/unittests/AST/RandstructTest.cpp:42
+
+static auto makeAST = [](const std::string &SourceCode) {
   std::vector<std::string> Args = getCommandLineArgsForTesting(Lang_C99);
----------------
Change this to a normal function

```
static std::pair<std::unique_ptr<ASTUnit>, std::unique_ptr<ASTUnit>>
makeAST(llvm::StringRef SourceCode) {
```


================
Comment at: clang/unittests/AST/RandstructTest.cpp:72
+
+  return std::tuple<ASTUnit *, ASTUnit *>(AST.release(), ASTFileSeed.release());
+};
----------------
aaron.ballman wrote:
> void wrote:
> > aaron.ballman wrote:
> > > Why not keep these as unique pointers and move them into the tuple? Then the callers don't have to call delete manually.
> > The d'tors for the `unique_ptr`s is called if I place them in the tuple. I think it's because I can't do something like this:
> > 
> > ```
> > const unique_ptr<ASTUnit> std::tie(AST, ASTFileSeed) = makeAST(...);
> > ```
> > 
> > in the test functions. When I assign it as a non-initializer, it apparently calls the d'tor. So, kinda stumped on what to do.
> > 
> > And the `EXPECT_FALSE` above is used because the `ASSERT_FALSE` adds an extra return point, which messes with the lambda.
> Okay, thank you for the explanations!
I think std::unique_ptr should and can be used here. See comment above.


================
Comment at: clang/unittests/AST/RandstructTest.cpp:118
+
+  std::vector<std::string> LHSFields = getFieldNamesFromRecord(LHSRD);
+  std::vector<std::string> RHSFields = getFieldNamesFromRecord(RHSRD);
----------------
`return getFieldNamesFromRecord(LHSRD) == getFieldNamesFromRecord(RHSRD);`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123636/new/

https://reviews.llvm.org/D123636