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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits