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

Fangrui Song via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 14 14:23:22 PDT 2022


MaskRay added inline comments.


================
Comment at: clang/unittests/AST/RandstructTest.cpp:72
+
+  return std::tuple<ASTUnit *, ASTUnit *>(AST.release(), ASTFileSeed.release());
+};
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123636



More information about the cfe-commits mailing list