Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cope with types that have deleted move-constructors #489

Closed
adetaylor opened this issue May 14, 2021 · 3 comments
Closed

Cope with types that have deleted move-constructors #489

adetaylor opened this issue May 14, 2021 · 3 comments
Labels
bug Something isn't working cpp-feature C++ feature not yet supported

Comments

@adetaylor
Copy link
Collaborator

This is a problem encountered on an internal codebase, so I can't give details here, but recording that this bug exists for my own records. Once it's minimized I will post a test case here.

@adetaylor
Copy link
Collaborator Author

adetaylor commented May 16, 2021

Minimization command for the record

~/autocxx/target/release/autocxx-reduce -I . -h column_list.h -d 'generate!("spanner::ReadOptions")' -o min.h -p "move-insertable" --compile -d 'block!("proto2:Message")' -c=-std=c++17 `cat customopts-min.txt` --gen-cmd $(pwd)/custom_autocxx_gen -k --clang $(pwd)/custom_clang -- --n 64

@adetaylor
Copy link
Collaborator Author

Problems here include:
gen0.cc:1383:15: error: call to deleted constructor of '::xyz' ::new (out) ::xyz(::std::move(v->back())); called from this cxx-generated code:

void cxxbridge1$std$vector$xyz$pop_back(::std::vector<::xyz> *v, ::xyz *out) noexcept {
  ::new (out) ::xyz(::std::move(v->back()));
  v->pop_back();
}

and
error: static_assert failed due to requirement '__is_cpp17_move_insertable<std::allocator<xyz>, void>::value' "The specified type does not meet the requirements of Cpp17MoveInsertable"

from:

void cxxbridge1$std$vector$xyz$push_back(::std::vector<::xyz> *v, ::xyz *value) noexcept {
  v->push_back(::std::move(*value));
  ::rust::destroy(value);
}

The problem here stems from types which have:

  • Deleted move constructors (e.g. using DISALLOW_COPY_AND_ASSIGN)
  • Deleted copy constructors (which may be because a field has a deleted copy constructor)

This seems to be OK when we impl UniquePtr<T> but causes problems when we impl CxxVector<T> in the cxx::bridge.

I think for now we should cease to generate impl CxxVector<T> unless there's a generate_vec!(..) directive.

In the future we might be able to use the wisdom that we'll need to learn in areas around #458 and #258 to determine if there are move constructors, and if so, reinstate auto vector generation.

@adetaylor adetaylor changed the title Problem building generated C++ code: "move-insertable" Cope with types that have deleted move-constructors May 16, 2021
@adetaylor adetaylor added cpp-feature C++ feature not yet supported and removed bug Something isn't working labels May 16, 2021
@martinboehme
Copy link
Collaborator

You may be able to query whether the move or copy constructor are deleted using the code introduced as part of #426 -- but this may not work if they are implicitly deleted (I don't recall), so it may be best to just avoid generating impl CxxVector<T>.

@adetaylor adetaylor added the bug Something isn't working label Jan 8, 2022
adetaylor added a commit that referenced this issue Jan 8, 2022
This works but it turns out we need to do the same for copy constructors.
It's also a bit messy thus far.

Work towards #489.
adetaylor added a commit that referenced this issue Jan 8, 2022
This works but it turns out we need to do the same for copy constructors.
It's also a bit messy thus far.

Work towards #489.
adetaylor added a commit that referenced this issue Jan 9, 2022
This works but it turns out we need to do the same for copy constructors.
It's also a bit messy thus far.

Work towards #489.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working cpp-feature C++ feature not yet supported
Projects
None yet
Development

No branches or pull requests

2 participants