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

Merge #16

Merged
merged 27 commits into from
Aug 30, 2024
Merged

Merge #16

merged 27 commits into from
Aug 30, 2024

Conversation

kuecks
Copy link

@kuecks kuecks commented Aug 30, 2024

No description provided.

dtolnay and others added 27 commits July 8, 2024 20:38
Required by recent versions of the cc crate.

    error: package `cc v1.0.106` cannot be built because it requires rustc 1.67 or newer, while the currently active rustc version is 1.63.0
    warning: this could be rewritten as `let...else`
      --> gen/build/src/cargo.rs:54:13
       |
    54 | /             let k = match k.to_str() {
    55 | |                 Some(k) => k,
    56 | |                 None => continue,
    57 | |             };
       | |______________^ help: consider writing: `let Some(k) = k.to_str() else { continue };`
       |
       = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_let_else
       = note: `-W clippy::manual-let-else` implied by `-W clippy::pedantic`
       = help: to override `-W clippy::pedantic` add `#[allow(clippy::manual_let_else)]`

    warning: this could be rewritten as `let...else`
      --> gen/build/src/cargo.rs:58:13
       |
    58 | /             let v = match v.into_string() {
    59 | |                 Ok(v) => v,
    60 | |                 Err(_) => continue,
    61 | |             };
       | |______________^ help: consider writing: `let Ok(v) = v.into_string() else { continue };`
       |
       = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_let_else

    warning: this could be rewritten as `let...else`
       --> gen/build/src/lib.rs:437:5
        |
    437 | /     let mut entries = match fs::read_dir(src) {
    438 | |         Ok(entries) => entries,
    439 | |         Err(_) => return,
    440 | |     };
        | |______^ help: consider writing: `let Ok(mut entries) = fs::read_dir(src) else { return };`
        |
        = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_let_else

    warning: this could be rewritten as `let...else`
       --> gen/build/src/out.rs:160:5
        |
    160 | /     let relative_path = match abstractly_relativize_symlink(original, link) {
    161 | |         Some(relative_path) => relative_path,
    162 | |         None => return original.to_path_buf(),
    163 | |     };
        | |______^ help: consider writing: `let Some(relative_path) = abstractly_relativize_symlink(original, link) else { return original.to_path_buf() };`
        |
        = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_let_else

    warning: this could be rewritten as `let...else`
       --> syntax/check.rs:558:9
        |
    558 | /         let resolve = match cx.types.try_resolve(&receiver.ty) {
    559 | |             Some(resolve) => resolve,
    560 | |             None => return,
    561 | |         };
        | |__________^ help: consider writing: `let Some(resolve) = cx.types.try_resolve(&receiver.ty) else { return };`
        |
        = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_let_else
        = note: `-W clippy::manual-let-else` implied by `-W clippy::pedantic`
        = help: to override `-W clippy::pedantic` add `#[allow(clippy::manual_let_else)]`

    warning: this could be rewritten as `let...else`
       --> syntax/parse.rs:439:5
        |
    439 | /     let name = match &abi.name {
    440 | |         Some(name) => name,
    441 | |         None => {
    442 | |             return Err(Error::new_spanned(
    ...   |
    446 | |         }
    447 | |     };
        | |______^
        |
        = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_let_else
    help: consider writing
        |
    439 ~     let Some(name) = &abi.name else {
    440 +             return Err(Error::new_spanned(
    441 +                 abi,
    442 +                 "ABI name is required, extern \"C++\" or extern \"Rust\"",
    443 +             ));
    444 +         };
        |

    warning: this could be rewritten as `let...else`
        --> syntax/parse.rs:1332:5
         |
    1332 | /     let len_expr = if let Expr::Lit(lit) = &ty.len {
    1333 | |         lit
    1334 | |     } else {
    1335 | |         let msg = "unsupported expression, array length must be an integer literal";
    1336 | |         return Err(Error::new_spanned(&ty.len, msg));
    1337 | |     };
         | |______^
         |
         = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_let_else
    help: consider writing
         |
    1332 ~     let Expr::Lit(len_expr) = &ty.len else {
    1333 +         let msg = "unsupported expression, array length must be an integer literal";
    1334 +         return Err(Error::new_spanned(&ty.len, msg));
    1335 +     };
         |

    warning: this could be rewritten as `let...else`
      --> syntax/report.rs:24:9
       |
    24 | /         let mut all_errors = match iter.next() {
    25 | |             Some(err) => err,
    26 | |             None => return Ok(()),
    27 | |         };
       | |__________^ help: consider writing: `let Some(mut all_errors) = iter.next() else { return Ok(()) };`
       |
       = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_let_else

    warning: this could be rewritten as `let...else`
       --> syntax/types.rs:174:13
        |
    174 | /             let impl_key = match ty.impl_key() {
    175 | |                 Some(impl_key) => impl_key,
    176 | |                 None => continue,
    177 | |             };
        | |______________^ help: consider writing: `let Some(impl_key) = ty.impl_key() else { continue };`
        |
        = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_let_else
After this commit, it is possible to explicitly construct `rust::Slice`
from a reference to any continguous C++ container (any container that
exposes `data` and `size` accessors).

The new constructor results in a slightly more ergonomic code, by
removing the need to explicit extract and pass `c.data()` and `c.size()`
at a callsite of a `rust::Slice` constructor.  The callsites using the
new constructor are also more obviously correct, because they doesn't
require double-checking that the passed `data` and `size` match.

The implementation of the new constructor mimics `std::span` from C++20,
but for C++11 compatibility reimplements `std::size` /
`std::ranges::size` in a pedestrian way (same for `std::data` /
`std::ranges::data`).
Ergonomics: allow constructing `rust::Slice` from any C++ container.
This commit implements forwarding of `Read` trait implementation from
`UniquePtr<T>` to the pointee type.  This is quite similar to how
`Box<T>` also forwards - see
https://doc.rust-lang.org/std/boxed/struct.Box.html#impl-Read-for-Box%3CR%3E

Just as with `Box<T>`, the `impl` cannot be provided in the crate
introducing `T`, because this violates the orphan rule.  This means that
before this commit a wrapper newtype would be required to work around
the orphan rule - e.g.:

    ```
    struct UniquePtrOfReadTrait(cxx::UniquePtr<ffi::ReadTrait>);
    impl Read for UniquePtrOfReadTrait { … }
    ```

After this commit, one can provide an `impl` that works more directly
with the C++ type `T` (the FFI will typically require passing `self:
Pin<&mut ffi::ReadTrait>`):

    ```
    impl<'a> Read for Pin<&'a mut ffi::ReadTrait> { … }
    ```

For a more specific motivating example, please see:
https://docs.google.com/document/d/1EPn1Ss-hfOC6Ki_B5CC6GA_UFnY3TmoDLCP2HjP7bms
…ait-impls

`impl<T> Read for UniquePtr<T> where ... Pin<&a mut T> : Read`.
Old compilers didn't used to consider `()` acceptable for FFI.

    error: `extern` block uses type `()`, which is not FFI-safe
      --> demo/src/main.rs:20:14
       |
    20 |           type BlobstoreClient;
       |  ______________^
    21 | |
    22 | |         fn new_blobstore_client() -> UniquePtr<BlobstoreClient>;
    23 | |         fn put(&self, parts: &mut MultiBuf) -> u64;
       | |________________^ not FFI-safe
       |
       = help: consider using a struct instead
       = note: tuples have unspecified layout
    note: the lint level is defined here
      --> demo/src/main.rs:1:1
       |
    1  | #[cxx::bridge(namespace = "org::blobstore")]
       | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
       = note: this error originates in the attribute macro `cxx::bridge` (in Nightly builds, run with -Z macro-backtrace for more info)
Drop "readonly" LLVM attribute from references to opaque C++ types
@dtolnay dtolnay merged commit ada41e6 into main Aug 30, 2024
28 checks passed
@dtolnay dtolnay deleted the merge branch August 30, 2024 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants