derive(SmartPointer): rewrite bounds in where and generic bounds #127681

dingxiangfei2009 · 2024-07-13T12:17:02Z

Due to the Unsize bounds, we need to commute the bounds on the pointee type to the new self type.

rustbot · 2024-07-13T12:17:10Z

rustbot has assigned @compiler-errors.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

dingxiangfei2009 · 2024-07-13T13:03:11Z

r? @davidtwco

davidtwco

One nit, r=me other than that

compiler/rustc_builtin_macros/src/deriving/smart_ptr.rs

Darksonn · 2024-07-23T08:15:04Z

What needs to happen here? Can we merge it now so that the fix will be available in the same version of rustc that initially adds #[derive(SmartPointer)]?

Darksonn · 2024-07-24T09:27:29Z

Ping. Not sure if it's too late to get the fix in?

tests/ui/deriving/smart-pointer-bounds-issue-127647.rs

compiler/rustc_builtin_macros/src/deriving/smart_ptr.rs

compiler-errors · 2024-07-29T17:50:52Z

compiler/rustc_builtin_macros/src/deriving/smart_ptr.rs

+    for (params, orig_params) in impl_generics.params[pointee_param_idx + 1..]
+        .iter_mut()
+        .zip(&generics.params[pointee_param_idx + 1..])


Why do we only care about pointee_param_idx + 1 ..? Is there some restriction about the placement of #[pointee] that this is implicitly relying on?

It has to do with those generic types that come after the annotated #[pointee] type parameter that may contain bounds "involving" the #[pointee] type, such as the X in this example.

#[derive(core::marker::SmartPointer)] #[repr(transparent)] pub struct Ptr6<'a, #[pointee] T: ?Sized, X: MyTrait<T>> { data: &'a mut T, x: core::marker::PhantomData<X>, }

The macro needs to write down a new bound X: MyTrait<T> from X: MyTrait<__S> as well.

But bounds can reference the #[pointee] parameter before the parameter is declared:

trait Foo<S>{} struct Test<S: Foo<T>, #[pointee] T> { /* ... */ }

I always thought this was not possible. I will update this part.

Applied. pointee_param_idx is skipped because that bound will be written separately.

compiler/rustc_builtin_macros/src/deriving/smart_ptr.rs

tests/ui/deriving/smart-pointer-bounds-issue-127647.rs

compiler-errors · 2024-07-29T18:47:27Z

Please remove the random test files. Also there's still a question I have about why that [i + 1...] exists, that you still have not answered.

compiler/rustc_builtin_macros/src/deriving/smart_ptr.rs

Rollup of 8 pull requests Successful merges: - rust-lang#125048 (PinCoerceUnsized trait into core) - rust-lang#127681 (derive(SmartPointer): rewrite bounds in where and generic bounds) - rust-lang#127830 (When an archive fails to build, print the path) - rust-lang#128147 (migrate fmt-write-bloat to rmake) - rust-lang#128356 (Migrate `cross-lang-lto-clang` and `cross-lang-lto-pgo-smoketest` `run-make` tests to rmake) - rust-lang#128387 (More detailed note to deprecate ONCE_INIT) - rust-lang#128388 (Match LLVM ABI in `extern "C"` functions for `f128` on Windows) - rust-lang#128412 (Remove `crate_level_only` from `ELIDED_LIFETIMES_IN_PATHS`) r? `@ghost` `@rustbot` modify labels: rollup

… r=compiler-errors derive(SmartPointer): rewrite bounds in where and generic bounds Fix rust-lang#127647 Due to the `Unsize` bounds, we need to commute the bounds on the pointee type to the new self type. cc ``@Darksonn``

…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#127681 (derive(SmartPointer): rewrite bounds in where and generic bounds) - rust-lang#127830 (When an archive fails to build, print the path) - rust-lang#128356 (Migrate `cross-lang-lto-clang` and `cross-lang-lto-pgo-smoketest` `run-make` tests to rmake) - rust-lang#128387 (More detailed note to deprecate ONCE_INIT) - rust-lang#128388 (Match LLVM ABI in `extern "C"` functions for `f128` on Windows) - rust-lang#128412 (Remove `crate_level_only` from `ELIDED_LIFETIMES_IN_PATHS`) r? `@ghost` `@rustbot` modify labels: rollup

… r=compiler-errors derive(SmartPointer): rewrite bounds in where and generic bounds Fix rust-lang#127647 Due to the `Unsize` bounds, we need to commute the bounds on the pointee type to the new self type. cc ```@Darksonn```

Darksonn · 2024-07-31T14:54:36Z

compiler/rustc_builtin_macros/src/deriving/smart_ptr.rs

+                .struct_span_err(
+                    pointee_ty_ident.span,
+                    format!(
+                        "`SmartPointer` is meaningless because `{}` is not `?Sized`",


I would probably phrase this as

"`derive(SmartPointer)` requires the use of `{}: ?Sized`"

Or "derive(SmartPointer) requires {} is marked ?Sized".

I elected

`derive(SmartPointer)` requires {} to be marked `?Sized`

Darksonn · 2024-07-31T14:55:15Z

tests/ui/deriving/deriving-smart-pointer-neg.rs

+#[derive(SmartPointer)]
+#[repr(transparent)]
+struct NoMaybeSized<'a, #[pointee] T> {
+    //~^ ERROR: SmartPointer` is meaningless because `T` is not `?Sized
+    ptr: &'a T,
+}


Should we have a test that ?std::marker::Sized succeeds?

Oops I somehow pushed this commit. 😓 I intend to open another PR.

It is working. To verify it, the test has updated with sensible ways that Sized marker can be used in.

…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#126454 (bump-stage0: use IndexMap for determinism) - rust-lang#127681 (derive(SmartPointer): rewrite bounds in where and generic bounds) - rust-lang#127830 (When an archive fails to build, print the path) - rust-lang#128151 (Structured suggestion for `extern crate foo` when `foo` isn't resolved in import) - rust-lang#128387 (More detailed note to deprecate ONCE_INIT) - rust-lang#128388 (Match LLVM ABI in `extern "C"` functions for `f128` on Windows) - rust-lang#128402 (Attribute checking simplifications) - rust-lang#128412 (Remove `crate_level_only` from `ELIDED_LIFETIMES_IN_PATHS`) - rust-lang#128430 (Use a separate pattern type for `rustc_pattern_analysis` diagnostics ) r? `@ghost` `@rustbot` modify labels: rollup

compiler-errors · 2024-07-31T17:04:37Z

@bors r+

bors · 2024-07-31T17:04:39Z

📌 Commit 1679c27 has been approved by compiler-errors

It is now in the queue for this repository.

matthiaskrgr · 2024-07-31T17:48:28Z

ah this was force pushed after I added it to the rollup, heh

Rollup merge of rust-lang#127681 - dingxiangfei2009:smart-ptr-bounds, r=compiler-errors derive(SmartPointer): rewrite bounds in where and generic bounds Fix rust-lang#127647 Due to the `Unsize` bounds, we need to commute the bounds on the pointee type to the new self type. cc ```@Darksonn```

compiler-errors · 2024-07-31T17:51:29Z

Ok, thanks for the heads up @matthiaskrgr. @dingxiangfei2009, please open a new PR with the delta between 5d78d7d and the commit here.

compiler-errors · 2024-07-31T17:54:31Z

@bors r-

compiler-errors · 2024-07-31T17:59:35Z

Actually I just did it myself in #128451.

…-maybe-sized, r=compiler-errors derive(SmartPointer): require pointee to be maybe sized cc `@Darksonn` So `#[pointee]` has to be `?Sized` in order for deriving `SmartPointer` to be meaningful. cc `@compiler-errors` for suggestions in rust-lang#127681

…-maybe-sized, r=compiler-errors derive(SmartPointer): require pointee to be maybe sized cc ``@Darksonn`` So `#[pointee]` has to be `?Sized` in order for deriving `SmartPointer` to be meaningful. cc ``@compiler-errors`` for suggestions in rust-lang#127681

Rollup merge of rust-lang#128452 - dingxiangfei2009:smart-ptr-require-maybe-sized, r=compiler-errors derive(SmartPointer): require pointee to be maybe sized cc ``@Darksonn`` So `#[pointee]` has to be `?Sized` in order for deriving `SmartPointer` to be meaningful. cc ``@compiler-errors`` for suggestions in rust-lang#127681

Darksonn · 2024-08-06T08:03:31Z

@rustbot label F-derive_smart_pointer

rustbot assigned compiler-errors Jul 13, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 13, 2024

dingxiangfei2009 mentioned this pull request Jul 13, 2024

Trait bounds not properly propagated in #[derive(SmartPointer)] #127647

Closed

rustbot assigned davidtwco and unassigned compiler-errors Jul 13, 2024

dingxiangfei2009 force-pushed the smart-ptr-bounds branch 3 times, most recently from dd508f4 to ba5b4eb Compare July 13, 2024 20:04

davidtwco approved these changes Jul 16, 2024

View reviewed changes

compiler/rustc_builtin_macros/src/deriving/smart_ptr.rs Outdated Show resolved Hide resolved

Darksonn approved these changes Jul 16, 2024

View reviewed changes