erichkeane added a comment. In D125402#3553855 <https://reviews.llvm.org/D125402#3553855>, @erichkeane wrote:
> In D125402#3553841 <https://reviews.llvm.org/D125402#3553841>, @aaron.ballman > wrote: > >> In D125402#3553825 <https://reviews.llvm.org/D125402#3553825>, @erichkeane >> wrote: >> >>> In D125402#3553802 <https://reviews.llvm.org/D125402#3553802>, >>> @aaron.ballman wrote: >>> >>>> In D125402#3517865 <https://reviews.llvm.org/D125402#3517865>, @nlee wrote: >>>> >>>>> How about adding CXXRecordDecl::hasMoveConstructor() to ensure the type >>>>> is movable? I ran a test with OpenCV(c++11), and it returned some useful >>>>> warnings: >>>> >>>> I don't think that will be quite correct -- IIRC, that would still return >>>> true if the move constructor was deleted. `hasSimpleMoveConstructor()` and >>>> `hasSimpleMoveAssignment()` might be a better approach. >>> >>> The 'Simple' version might not be quite right... That is implemented as: >>> >>> bool hasSimpleMoveConstructor() const { >>> return !hasUserDeclaredMoveConstructor() && hasMoveConstructor() && >>> !data().DefaultedMoveConstructorIsDeleted; >>> } >>> >>> >>> So this would still warn about user-defined move constructors. >> >> Good catch! >> >>> HOWEVER, I might suggest `hasMoveConstructor() && >>> !defaultedMoveConstructorIsDeleted()` for the ctor test. There is similar >>> storage for the 'DefaultedMoveAssignmentIsDeleted`, but it isn't exposed, >>> so you might need to add a function to expose it in DeclCXX.h. >> >> I think that might still not be correct because there could be a >> user-provided move constructor that's explicitly deleted. (So it has a move >> constructor, but the defaulted one is not deleted because it's user >> provided.) > > I don't think that is possible? I think 'defaultedMoveConstructor' includes > the user typing `StructName(StructName&&) = delete;`. > > Note that out of line deleted implementations are prohibited: > https://godbolt.org/z/MTK4jGa57 So we don't have to worry about that. I might be wrong about this above: https://clang.llvm.org/doxygen/classclang_1_1CXXRecordDecl.html#a8ddbc71ffd71a6f7d580fc7a19452f30 Comment says: " /// Set that we attempted to declare an implicit move /// constructor, but overload resolution failed so we deleted it." So perhaps more work to figure out non-deleted move ctor, it might require a lookup. So if we do that, we need a bunch of tests to validate all these conditions I believe (manually deleted, inherited deleted, member-cauesd-deleted, and 'defaulted' versions of the last 2, user provided but works, etc). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125402/new/ https://reviews.llvm.org/D125402 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
