Skip to content
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

Changing ?Sized related trait bounds is not detected #532

Open
weiznich opened this issue Sep 8, 2023 · 1 comment
Open

Changing ?Sized related trait bounds is not detected #532

weiznich opened this issue Sep 8, 2023 · 1 comment
Labels
C-bug Category: doesn't meet expectations

Comments

@weiznich
Copy link

weiznich commented Sep 8, 2023

Steps to reproduce the bug with the above code

git clone https://github.com/diesel-rs/diesel
cd diesel/diesel
cargo semver-checks --baseline-rev v2.0.4 --only-explicit-features --features postgres --features sqlite --features mysql --features extras --features with-deprecated --manifest-path .

Actual Behaviour

No breaking change is reported:

     Cloning v2.0.4
     Parsing diesel v2.1.1 (current)
     Parsing diesel v2.0.4 (baseline)
    Checking diesel v2.0.4 -> v2.1.1 (minor change)
   Completed [   0.150s] 39 checks; 39 passed, 6 unnecessary

Expected Behaviour

Cargo semver-checks should report a major breaking change. A trait function added a ?Sized bound to it's where clause.

Documentation:

Relevant function signature:
2.0.4:

    fn push_bound_value<T, U>(
        &mut self,
        bind: &'a U,
        metadata_lookup: &mut DB::MetadataLookup,
    ) -> QueryResult<()>
    where
        DB: Backend + HasSqlType<T>,
        U: ToSql<T, DB> + 'a;

2.1.1:

    fn push_bound_value<T, U>(
        &mut self,
        bind: &'a U,
        metadata_lookup: &mut DB::MetadataLookup,
    ) -> QueryResult<()>
    where
        DB: Backend + HasSqlType<T>,
        U: ToSql<T, DB> + ?Sized + 'a;

Note the added ?Sized bound in the newer version. From caller side this is a non-breaking change as it only extends the set of allowed values. For existing implementations outside of diesel of the given trait this is a breaking change as it requires updating the function signature. It also might break an implementation that assumes that U is Sized.

Generated System Information

Software version

cargo-semver-checks 0.22.1

Operating system

Linux 6.4.7-200.fc38.x86_64

Command-line

/home/weiznich/.cargo/bin/cargo-semver-checks semver-checks --bugreport 

cargo version

> cargo -V 
cargo 1.68.0 (115f34552 2023-02-26)

Compile time information

  • Profile: release
  • Target triple: x86_64-unknown-linux-gnu
  • Family: unix
  • OS: linux
  • Architecture: x86_64
  • Pointer width: 64
  • Endian: little
  • CPU features: fxsr,sse,sse2
  • Host: x86_64-unknown-linux-gnu

Build Configuration

No response

Additional Context

No response

@weiznich weiznich added the C-bug Category: doesn't meet expectations label Sep 8, 2023
@obi1kenobi
Copy link
Owner

Thanks, this is awesome! Especially appreciate the details on how this might be breaking and for whom, which will be super helpful writing the diagnostic message this should show.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: doesn't meet expectations
Projects
None yet
Development

No branches or pull requests

2 participants