Skip to content

Make is_complete static assertion unconditional for unique_ptr<T> bindings. #1536

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,14 @@ cc = "1.0.83"
cxxbridge-flags = { version = "=1.0.160", path = "flags", default-features = false }

[dev-dependencies]
cc = "1.0.83"
cxx-build = { version = "=1.0.160", path = "gen/build" }
cxx-gen = { version = "0.7", path = "gen/lib" }
cxx-test-suite = { version = "0", path = "tests/ffi" }
proc-macro2 = "1.0.95"
quote = "1.0.40"
rustversion = "1.0.13"
tempdir = "0.3.7"
trybuild = { version = "1.0.81", features = ["diff"] }

# Disallow incompatible cxxbridge-cmd version appearing in the same lockfile.
Expand Down
31 changes: 31 additions & 0 deletions build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ fn main() {
println!("cargo:rustc-cfg=error_in_core");
}
}

persist_target_triple();
}

struct RustVersion {
Expand All @@ -73,3 +75,32 @@ fn rustc_version() -> Option<RustVersion> {
let minor = pieces.next()?.parse().ok()?;
Some(RustVersion { version, minor })
}

/// `tests/cpp_ui_tests.rs` needs to know the target triple when invoking a
/// C/C++ compiler through the `cc` crate. The function below facilitates this
/// by capturing the value of the `TARGET` environment variable seen during
/// `build.rs` execution, and writing this value to a file that the
/// `cpp_ui_tests` can pick up using `include_str!`.
///
/// An alternative approach would be to drive `cpp_ui_tests` from `build.rs`
/// during build time. This seems less desirable than the current approach,
/// which benefits from being a set of regular test cases (which can be
/// filtered, have their stderr captured, etc.). FWIW the `tests/ui` tests also
/// invoke build tools (e.g. `rustc`) at test time, rather than build time, so
/// this seems okay.
///
/// This function ignores errors, because we don't want to avoid disrupting
/// production builds (even if failure to generate `target_triple.txt` may
/// disrupt test builds).
fn persist_target_triple() {
let Some(out_dir) = env::var_os("OUT_DIR") else {
return;
};
let Ok(target) = env::var("TARGET") else {
return;
};
println!("cargo:rerun-if-env-changed=TARGET");

let out_dir = Path::new(&out_dir);
let _ = std::fs::write(out_dir.join("target_triple.txt"), target);
}
41 changes: 12 additions & 29 deletions gen/src/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1707,25 +1707,12 @@ fn write_unique_ptr_common(out: &mut OutFile, ty: UniquePtr) {
UniquePtr::CxxVector(_) => false,
};

let conditional_delete = match ty {
UniquePtr::Ident(ident) => {
!out.types.structs.contains_key(ident) && !out.types.enums.contains_key(ident)
}
UniquePtr::CxxVector(_) => false,
};

if conditional_delete {
out.builtin.is_complete = true;
let definition = match ty {
UniquePtr::Ident(ty) => &out.types.resolve(ty).name.cxx,
UniquePtr::CxxVector(_) => unreachable!(),
};
writeln!(
out,
"static_assert(::rust::detail::is_complete<{}>::value, \"definition of {} is required\");",
inner, definition,
);
}
out.builtin.is_complete = true;
writeln!(
out,
"static_assert(::rust::detail::is_complete<{}>::value, \"definition of `{}` is required\");",
inner, inner,
);
writeln!(
out,
"static_assert(sizeof(::std::unique_ptr<{}>) == sizeof(void *), \"\");",
Expand Down Expand Up @@ -1797,16 +1784,12 @@ fn write_unique_ptr_common(out: &mut OutFile, ty: UniquePtr) {
"void cxxbridge1$unique_ptr${}$drop(::std::unique_ptr<{}> *ptr) noexcept {{",
instance, inner,
);
if conditional_delete {
out.builtin.deleter_if = true;
writeln!(
out,
" ::rust::deleter_if<::rust::detail::is_complete<{}>::value>{{}}(ptr);",
inner,
);
} else {
writeln!(out, " ptr->~unique_ptr();");
}
out.builtin.deleter_if = true;
writeln!(
out,
" ::rust::deleter_if<::rust::detail::is_complete<{}>::value>{{}}(ptr);",
inner,
);
writeln!(out, "}}");
}

Expand Down
48 changes: 48 additions & 0 deletions testing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# Testing

This document tries to provide an outline of different kinds of tests
used by the `cxx` project.

## Errors from proc macro

In some situations, we want to verify that the `#[cxx::bridge]` macro reports
expected error messages when invoked by `rustc`.

Such verification is handled
by test cases underneath `tests/ui` directory and driven by
`tests/compiletest.rs`. The test cases consist of a pair of files:

* `foo.rs` is the input
* `foo.stderr` is the expected output

## Errors from C++ compiler

In some situations, we want to verify that
the C++ code
generated by a successful invocation of the `cxxbridge-cmd` command
results in expected error messages
when compiled by a C++ compiler.
(Errors from unsuccessful invocations of the `cxxbridge-cmd` command
should have test coverage provided by the `tests/ui` test suite.)

Such verification is covered by `tests/cpp_ui_tests.rs`.

## End-to-end functionality

End-to-end functional tests are structured as follows:

* The code under test is contained underneath `tests/ffi` directory which
contains:
- Rust code under test - the `cxx-test-suite` crate
(`lib.rs` and `module.rs`) with:
- A few `#[cxx::bridge]` declarations
- Rust types under test (e.g. `struct R`)
- Rust functions and methods under test (e.g. `r_return_primitive`)
- C/C++ code under test (`tests.h` and `tests.cc`)
- C++ types under test (e.g. `class C`)
- C++ functions and methods under test (e.g. `c_return_primitive`)
* The testcases can be found in:
- Rust calling into C++: `tests/test.rs`
- C++ calling into Rust: `tests/ffi/test.cc`.
The tests are transitively, manually invoked from the
`cxx_run_test` function in `tests/ffi/test.cc`
23 changes: 23 additions & 0 deletions tests/cpp_ui_tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
mod cpp_ui_tests_harness;
use cpp_ui_tests_harness::Test;

use quote::quote;

/// This is a regression test for `static_assert(::rust::is_complete...)`
/// which we started to emit in <https://github.com/dtolnay/cxx/commit/534627667>
#[test]
fn test_unique_ptr_of_incomplete_foward_declared_pointee() {
let test = Test::new(quote! {
#[cxx::bridge]
mod ffi {
unsafe extern "C++" {
include!("include.h");
type ForwardDeclaredType;
}
impl UniquePtr<ForwardDeclaredType> {}
}
});
test.write_file("include.h", "class ForwardDeclaredType;");
let err_msg = test.compile().expect_single_error();
assert!(err_msg.contains("definition of `::ForwardDeclaredType` is required"));
}
Loading
Loading