void added inline comments.
================ Comment at: clang/unittests/AST/RandstructTest.cpp:72 + + return std::tuple<ASTUnit *, ASTUnit *>(AST.release(), ASTFileSeed.release()); +}; ---------------- MaskRay wrote: > void wrote: > > MaskRay wrote: > > > 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. > > Yeah, it's just that it doesn't work. I actually tried it a lot of > > different ways before, of course, and it just doesn't work. Sorry about > > that. > > > > ``` > > /usr/local/google/home/morbo/llvm/llvm-project/clang/unittests/AST/RandstructTest.cpp:73:10: > > error: no matching constructor for initialization of > > 'std::pair<std::unique_ptr<ASTUnit>, std::unique_ptr<ASTUnit>>' > > return std::pair<std::unique_ptr<ASTUnit>, std::unique_ptr<ASTUnit>>(AST, > > ASTFileSeed); > > ^ > > ~~~~~~~~~~~~~~~~ > > /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_pair.h:266:17: > > note: candidate template ignored: requirement '_PCC<true, > > std::unique_ptr<clang::ASTUnit, std::default_delete<clang::ASTUnit>>, > > std::unique_ptr<clang::ASTUnit, > > std::default_delete<clang::ASTUnit>>>::_ConstructiblePair()' was not > > satisfied [with _U1 = std::unique_ptr<clang::ASTUnit>, _U2 = > > std::unique_ptr<clang::ASTUnit>] > > constexpr pair(const _T1& __a, const _T2& __b) > > ^ > > /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_pair.h:276:26: > > note: candidate template ignored: requirement '_PCC<true, > > std::unique_ptr<clang::ASTUnit, std::default_delete<clang::ASTUnit>>, > > std::unique_ptr<clang::ASTUnit, > > std::default_delete<clang::ASTUnit>>>::_ConstructiblePair()' was not > > satisfied [with _U1 = std::unique_ptr<clang::ASTUnit>, _U2 = > > std::unique_ptr<clang::ASTUnit>] > > explicit constexpr pair(const _T1& __a, const _T2& __b) > > ^ > > /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_pair.h:322:18: > > note: candidate template ignored: requirement '_PCC<true, > > std::unique_ptr<clang::ASTUnit, std::default_delete<clang::ASTUnit>>, > > std::unique_ptr<clang::ASTUnit, > > std::default_delete<clang::ASTUnit>>>::_MoveCopyPair()' was not satisfied > > [with _U1 = std::unique_ptr<clang::ASTUnit> &] > > constexpr pair(_U1&& __x, const _T2& __y) > > ^ > > /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_pair.h:329:27: > > note: candidate template ignored: requirement '_PCC<true, > > std::unique_ptr<clang::ASTUnit, std::default_delete<clang::ASTUnit>>, > > std::unique_ptr<clang::ASTUnit, > > std::default_delete<clang::ASTUnit>>>::_MoveCopyPair()' was not satisfied > > [with _U1 = std::unique_ptr<clang::ASTUnit> &] > > explicit constexpr pair(_U1&& __x, const _T2& __y) > > ^ > > /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_pair.h:336:18: > > note: candidate template ignored: requirement '_PCC<true, > > std::unique_ptr<clang::ASTUnit, std::default_delete<clang::ASTUnit>>, > > std::unique_ptr<clang::ASTUnit, > > std::default_delete<clang::ASTUnit>>>::_CopyMovePair()' was not satisfied > > [with _U2 = std::unique_ptr<clang::ASTUnit> &] > > constexpr pair(const _T1& __x, _U2&& __y) > > ^ > > /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_pair.h:343:17: > > note: candidate template ignored: requirement '_PCC<true, > > std::unique_ptr<clang::ASTUnit, std::default_delete<clang::ASTUnit>>, > > std::unique_ptr<clang::ASTUnit, > > std::default_delete<clang::ASTUnit>>>::_CopyMovePair()' was not satisfied > > [with _U2 = std::unique_ptr<clang::ASTUnit> &] > > explicit pair(const _T1& __x, _U2&& __y) > > ^ > > /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_pair.h:352:12: > > note: candidate template ignored: requirement '_PCC<true, > > std::unique_ptr<clang::ASTUnit, std::default_delete<clang::ASTUnit>>, > > std::unique_ptr<clang::ASTUnit, > > std::default_delete<clang::ASTUnit>>>::_MoveConstructiblePair()' was not > > satisfied [with _U1 = std::unique_ptr<clang::ASTUnit> &, _U2 = > > std::unique_ptr<clang::ASTUnit> &] > > constexpr pair(_U1&& __x, _U2&& __y) > > ^ > > /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_pair.h:361:21: > > note: candidate template ignored: requirement '_PCC<true, > > std::unique_ptr<clang::ASTUnit, std::default_delete<clang::ASTUnit>>, > > std::unique_ptr<clang::ASTUnit, > > std::default_delete<clang::ASTUnit>>>::_MoveConstructiblePair()' was not > > satisfied [with _U1 = std::unique_ptr<clang::ASTUnit> &, _U2 = > > std::unique_ptr<clang::ASTUnit> &] > > explicit constexpr pair(_U1&& __x, _U2&& __y) > > ^ > > /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_pair.h:300:19: > > note: candidate constructor template not viable: requires single argument > > '__p', but 2 arguments were provided > > constexpr pair(const pair<_U1, _U2>& __p) > > ^ > > /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_pair.h:309:21: > > note: candidate constructor template not viable: requires single argument > > '__p', but 2 arguments were provided > > explicit constexpr pair(const pair<_U1, _U2>& __p) > > ^ > > /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_pair.h:314:17: > > note: candidate constructor not viable: requires 1 argument, but 2 were > > provided > > constexpr pair(const pair&) = default; ///< Copy constructor > > ^ > > /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_pair.h:315:17: > > note: candidate constructor not viable: requires 1 argument, but 2 were > > provided > > constexpr pair(pair&&) = default; ///< Move constructor > > ^ > > /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_pair.h:371:12: > > note: candidate constructor template not viable: requires single argument > > '__p', but 2 arguments were provided > > constexpr pair(pair<_U1, _U2>&& __p) > > ^ > > /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_pair.h:381:21: > > note: candidate constructor template not viable: requires single argument > > '__p', but 2 arguments were provided > > explicit constexpr pair(pair<_U1, _U2>&& __p) > > ^ > > /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_pair.h:387:9: > > note: candidate constructor template not viable: requires 3 arguments, but > > 2 were provided > > pair(piecewise_construct_t, tuple<_Args1...>, tuple<_Args2...>); > > ^ > > /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_pair.h:232:26: > > note: candidate constructor template not viable: requires 0 arguments, but > > 2 were provided > > _GLIBCXX_CONSTEXPR pair() > > ^ > > /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_pair.h:245:26: > > note: candidate constructor template not viable: requires 0 arguments, but > > 2 were provided > > explicit constexpr pair() > > ^ > > /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_pair.h:452:9: > > note: candidate constructor template not viable: requires 4 arguments, but > > 2 were provided > > pair(tuple<_Args1...>&, tuple<_Args2...>&, > > ^ > > 1 error generated. > > ``` > I think it will work if you change makeAST a function and use > > `return {std::move(...), std::move(...)};` > > I have tested it locally. I went a different path. PTAL. 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