can DropBehavior have an associated constant instead of a function

jonhoo / left-right

A lock-free, read-optimized, concurrency primitive.

Apache License 2.0

1.94k stars 94 forks source link

can DropBehavior have an associated constant instead of a function #85

Closed Eh2406 closed 3 years ago

Eh2406 commented 3 years ago

pub trait DropBehavior {
    const DO_DROP: bool;
}

That feels less complicated than a method that returns a constant.

jonhoo commented 3 years ago

Oh, interesting, I didn't even know that traits could have associated constants now. Absolutely! Want to submit a PR? I don't think it'll make much difference in practice, but it is maybe a little nicer. It is a breaking change to left-right, but I think that's fine since it's so new and still pre-1.0.

One thing I do worry about with DropBehavior is that the compiler gets smart enough that it recognizes that the Drop implementation for Aliased<_, NoDrop> is a no-op, and therefore decides to lay it out differently in memory. I don't think that's something it does, but it is something that making it a constant will make it more likely optimization. But that said, if that optimization is possible, it would make what we're doing unsound regardless of whether it's an associated const or an fn, so the change is likely a good one anyway.

Eh2406 commented 3 years ago

Want to submit a PR?

It seems fairly straight forward, so I think I'd prefer to leave it open for someone with less experience to pick up. I am open to mentoring or pair programming or answering questions for anyone that does want to give it a try.