pubgrub-rs / pubgrub

PubGrub version solving algorithm implemented in Rust
https://pubgrub-rs.github.io/pubgrub/pubgrub/
Mozilla Public License 2.0
337 stars 30 forks source link

Avoid loss of data in Range.simplify #172

Closed konstin closed 3 months ago

konstin commented 6 months ago

Upstream port of https://github.com/zanieb/pubgrub/pull/12

Eh2406 commented 6 months ago

The existing simplify returns the smallest representation where all the versions get the same result. This feels like an important property to keep available. On the other hand the new simplify is probably better for error messages, and for some (not yet determined) reason works in contexts where the original doesn't.

Perhaps we should have a second function available:


pub fn bike_shed<'v, I>(&self, versions: I) -> Self
    where
        I: Iterator<Item = &'v V> + 'v,
        V: 'v,
    {
        // Do not simplify singletons
        if self.is_singleton() {
            return self.clone();
        }

        let simp = self.simplify(versions);

        if simp == Empty() {
            self.cone()
        } else {
            simp
        }
    }

@mpizenberg thoughts?

mpizenberg commented 6 months ago

for some (not yet determined) reason works in contexts where the original doesn't.

I might not have the enough context for this.

Personally, I'd not use the same simplify if one is for optimization and the other is for quality of errors. The issue is that the original simplify is lossy. So you can't really apply the alternative simplify if the data has already been simplified with loss of information.