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