JelteF / derive_more

Some more derive(Trait) options
MIT License
1.73k stars 123 forks source link

Bugfix/double reference #354

Closed yfcai closed 5 months ago

yfcai commented 6 months ago

Resolves #328

Part of #328

Synopsis

This is an alternative solution to the double reference problem reported in #328.

Solution

Instead of dereferencing macro-generated bindings such as _0 at the use sites, do the the following:

  1. In binding generation, generate let _0 = self.0 instead of let _0 = &self.0 in case the field self.0 is already a reference type.

  2. In body generation, add & before #ident in the call derive_more::core::fmt::#trait_ident::fmt(#ident, __derive_more_f), because the function fmt dereferences.

  3. In bounds generation, strip & in the bounded type for traits other than Pointer because the fmt function of those traits dereferences exhaustively.

Open questions

  1. All previous test function names are assert. The tests added in this PR have distinct names so that they can be executed alone via cargo test <substring-of-test-function-name>. Is there a reason for the naming convention assert? If so I'd rename the new tests.

  2. Are the improvements to tests in #328 important? I can incorporate them if needed.

  3. Are there any breaking changes? I'm not 100% sure.

Checklist

yfcai commented 5 months ago

The approach in this PR does not handle non-reference types that implement Pointer.

Another approach is defining a newtype for bindings _0, _1 etc that transparently delegate Pointer and other Display traits to the underlying type. While implementing Deref makes the newtype a transparent reference for method and function calls, it does not work for operators including user implementations of traits in std::ops.

In conclusion, the approach in #328 is better.