Relax error bound

[SarguelUnda]

This relax the bound on logos Error type for more interoperability with error library like anyhow. I could not see where or if those bounds were mandatory. We still need the default trait for constructor but it is ok to use a manual implementation over a wrapper type like

struct MyAppLexError(anyhow::Error)

It would be harder/misleading to do so for Clone and PartialEq hence the pull request.

[jeertmans]

Hi @SarguelUnda, thanks for your PR!

As you can see, your changes require a increase of the minimal supported Ruest version (MSRV): do you have any idea what is causing this? I guess you can fiund the explaination in the changelog of Rust 1.81? Or maybe is was already broken before, as it seems to be triggered by a dependency we have.

[SarguelUnda]

Hello,

from running the command locally it seems to be the home deps from the afl package from the fuzz local crate that trigger this error.

cargo msrv verify -- cargo check --workspace --features debug:

Compatibility Check #1: Rust 1.74.0
  [FAIL]   Is incompatible

  ╭───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮
  │ error: package `home v0.5.11` cannot be built because it requires rustc 1.81 or newer, while the currently active rustc version is 1.74.0 │
  │ Either upgrade to rustc 1.81 or newer, or use                                                                                             │
  │ cargo update [email protected] --precise ver                                                                                                    │
  │ where `ver` is the latest version of `home` supporting rustc 1.74.0                                                                       │
  │                                                                                                                                           │
  ╰───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯

cargo tree --workspace

logos-fuzz v0.15.0
├── afl v0.15.13
│   ├── libc v0.2.169
│   ├── rustc_version v0.4.1 (*)
│   └── xdg v2.5.2
│   [build-dependencies]
│   └── home v0.5.11
│       └── windows-sys v0.59.0
│           └── windows-targets v0.52.6 (*)
├── arbitrary v1.4.1
└── logos-codegen v0.15.0 (*)

[sokorototo]

This somewhat tracks, as an error type really only needs Debug and Display

[codspeed-hq[bot]]

CodSpeed Performance Report

Merging #459 will not alter performance

_{Comparing SarguelUnda:relax_error_bounds (21fea5a) with master (192d07d)}

Summary

✅ 6 untouched benchmarks

[jeertmans]

Hi @SarguelUnda, sorry for the delay in the review!

CodSpeed, CodeCov, and library tests are now finally all fixes and running as expected on the master branch. I just synced your branch with the master one, so we restart the review process where we stopped. Are you still available to work on this?

[codecov-commenter]

Codecov Report

:x: Patch coverage is 0% with 1 line in your changes missing coverage. Please review. :white_check_mark: Project coverage is 49.31%. Comparing base (192d07d) to head (21fea5a). :warning: Report is 122 commits behind head on master.

Files with missing lines	Patch %	Lines
src/lib.rs	0.00%	1 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #459      +/-   ##
==========================================
+ Coverage   49.26%   49.31%   +0.04%     
==========================================
  Files          33       33              
  Lines        2054     2054              
==========================================
+ Hits         1012     1013       +1     
+ Misses       1042     1041       -1     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[interacsion]

@jeertmans Would it be possible to also relax the Default bound here, since #445 was merged? I find that this requirement is quite frustrating when supplying a custom constructor

[interacsion]

I made it work in https://github.com/interacsion/logos/commit/f5aea0cf522dd99a133735f364fb4d9daafcd434, would like me to open a separate PR for it?

[SarguelUnda]

Hi @SarguelUnda, sorry for the delay in the review!

CodSpeed, CodeCov, and library tests are now finally all fixes and running as expected on the master branch. I just synced your branch with the master one, so we restart the review process where we stopped. Are you still available to work on this?

Hello,

Sorry for the late reply, I'm available to work on this PR if still needed

[jeertmans]

Just checking: is it by any chance compatible with #491? @robot-rover

[jeertmans]

@jeertmans Would it be possible to also relax the Default bound here, since #445 was merged? I find that this requirement is quite frustrating when supplying a custom constructor

Probably, but then we need a way to indicate Logos how to create the default error variant.

[robot-rover]

Just checking: is it by any chance compatible with #491? @robot-rover

Just checked; when I apply the two lines of change all the tests still pass, so it seems yes.

[jeertmans]

Thanks for checking @robot-rover!