Skip to content

Provide ref-transparent ops for TextSize #8

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

Merged
merged 1 commit into from
Mar 13, 2020

Conversation

CAD97
Copy link
Contributor

@CAD97 CAD97 commented Mar 11, 2020

Counter-proposal to #7. Basically the same set of impls, but set up slightly differently.

@matklad
Copy link
Member

matklad commented Mar 12, 2020

Do you know the reason why stdlib doesn't do this and spells all impls manually?

@CAD97
Copy link
Contributor Author

CAD97 commented Mar 12, 2020

Relevant snippets from std:

https://github.com/rust-lang/rust/blob/master/src/libcore/internal_macros.rs, https://github.com/rust-lang/rust/blob/303d8aff6092709edd4dbd35b1c88e9aa40bf6d8/src/libcore/ops/arith.rs#L101

// implements binary operators "&T op U", "T op &U", "&T op &U"
// based on "T op U" where T and U are expected to be `Copy`able
macro_rules! forward_ref_binop {
    (impl $imp:ident, $method:ident for $t:ty, $u:ty) => {
        forward_ref_binop!(impl $imp, $method for $t, $u,
                #[stable(feature = "rust1", since = "1.0.0")]);
    };
    (impl $imp:ident, $method:ident for $t:ty, $u:ty, #[$attr:meta]) => {
        #[$attr]
        impl<'a> $imp<$u> for &'a $t {
            type Output = <$t as $imp<$u>>::Output;

            #[inline]
            fn $method(self, other: $u) -> <$t as $imp<$u>>::Output {
                $imp::$method(*self, other)
            }
        }

        #[$attr]
        impl $imp<&$u> for $t {
            type Output = <$t as $imp<$u>>::Output;

            #[inline]
            fn $method(self, other: &$u) -> <$t as $imp<$u>>::Output {
                $imp::$method(self, *other)
            }
        }

        #[$attr]
        impl $imp<&$u> for &$t {
            type Output = <$t as $imp<$u>>::Output;

            #[inline]
            fn $method(self, other: &$u) -> <$t as $imp<$u>>::Output {
                $imp::$method(*self, *other)
            }
        }
    }
}

// implements "T op= &U", based on "T op= U"
// where U is expected to be `Copy`able
macro_rules! forward_ref_op_assign {
    (impl $imp:ident, $method:ident for $t:ty, $u:ty) => {
        forward_ref_op_assign!(impl $imp, $method for $t, $u,
                #[stable(feature = "op_assign_builtins_by_ref", since = "1.22.0")]);
    };
    (impl $imp:ident, $method:ident for $t:ty, $u:ty, #[$attr:meta]) => {
        #[$attr]
        impl $imp<&$u> for $t {
            #[inline]
            fn $method(&mut self, other: &$u) {
                $imp::$method(self, *other);
            }
        }
    }
}

macro_rules! add_impl {
    ($($t:ty)*) => ($(
        #[stable(feature = "rust1", since = "1.0.0")]
        impl Add for $t {
            type Output = $t;

            #[inline]
            #[rustc_inherit_overflow_checks]
            fn add(self, other: $t) -> $t { self + other }
        }

        forward_ref_binop! { impl Add, add for $t, $t }
    )*)
}

add_impl! { usize u8 u16 u32 u64 u128 isize i8 i16 i32 i64 i128 f32 f64 }

blame tracking shows that these impls date back to rust-lang/rust#21227 at the beginning of 2015 (1.0.0-alpha.2), to re-add behavior lost when changing the ops traits to take by-move rather than by-ref. At least if I'm reading history correctly.

The main reason std might not want to make the impls more generic is touchy type inference. I'm going to open an irlo thread about it.

The key thing for acting like std is not in exactly matching the std impls, though, but covering all of the std impls. I think the generic impls are better for us than just mirroring std's macro copied impls.

@CAD97
Copy link
Contributor Author

CAD97 commented Mar 13, 2020

@matklad
Copy link
Member

matklad commented Mar 13, 2020

I think it might make sense to optimistically merge this before irlo discussion is resolved. r=me after rebase

@CAD97
Copy link
Contributor Author

CAD97 commented Mar 13, 2020

bors: r=matklad

@bors
Copy link
Contributor

bors bot commented Mar 13, 2020

Build succeeded

@bors bors bot merged commit 3d5a1ba into rust-analyzer:master Mar 13, 2020
@CAD97 CAD97 deleted the cad97-ops branch March 13, 2020 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants