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

Make error types usable in more contexts #1297

Closed
Tracked by #671
joshlf opened this issue May 18, 2024 · 0 comments · Fixed by #1686
Closed
Tracked by #671

Make error types usable in more contexts #1297

joshlf opened this issue May 18, 2024 · 0 comments · Fixed by #1686

Comments

@joshlf
Copy link
Member

joshlf commented May 18, 2024

Per #1288 (comment):

Error types like SizeError do not implement std::error::Error, so the functions like read_from_bytes() that now return a Result can't be used with anyhow::Context, while the previous version that returned Option could. Since Debug and Display are already implemented, I think this should just be a matter of adding impl std::error::Error for SizeError {}, but I didn't actually try this out. It may still not be possible to call .context() due to other constraints (Send + Sync). This is easy enough to work around (assuming the caller doesn't really care about the underlying error), and we didn't use this pattern in many places, so it's not really a big deal for us.

Implementing std::error::Error is implemented in #1298.

Supporting Send + Sync may be harder, since the contained Src type may not be Send or Sync. We may need to provide a way to discard the Src type, and we will need to consider how discoverable this mechanism is for people who are encountering a trait-not-satisfied error. I could see a few options:

  • Provide an Error<Src> -> Error<()> conversion
  • Provide a new type alias type Foo = Error<()> so that type signatures are cleaner
  • Provide an entirely new type which contains similar information to the original error, but without the source
  • Provide an entirely new type which contains the name of the source type, but doesn't contain an instance of the source type (just like we do for Dst right now)
@joshlf joshlf mentioned this issue May 18, 2024
85 tasks
joshlf added a commit that referenced this issue May 18, 2024
While we're here, also relax `Dispaly for AlignmentError<Src, Dst>` to
permit `Dst: ?Sized` in exchange for `Dst: KnownLayout`. This is an
important relaxation since our APIs permit performing conversions into
unsized destination types with runtime alignment checking.

Makes progress on #1297
joshlf added a commit that referenced this issue May 18, 2024
While we're here, also relax `Dispaly for AlignmentError<Src, Dst>` to
permit `Dst: ?Sized` in exchange for `Dst: KnownLayout`. This is an
important relaxation since our APIs permit performing conversions into
unsized destination types with runtime alignment checking.

Makes progress on #1297
joshlf added a commit that referenced this issue May 18, 2024
While we're here, also relax `Dispaly for AlignmentError<Src, Dst>` to
permit `Dst: ?Sized` in exchange for `Dst: KnownLayout`. This is an
important relaxation since our APIs permit performing conversions into
unsized destination types with runtime alignment checking.

Makes progress on #1297
joshlf added a commit that referenced this issue May 18, 2024
While we're here, also relax `Dispaly for AlignmentError<Src, Dst>` to
permit `Dst: ?Sized` in exchange for `Dst: KnownLayout`. This is an
important relaxation since our APIs permit performing conversions into
unsized destination types with runtime alignment checking.

Makes progress on #1297
joshlf added a commit that referenced this issue May 18, 2024
While we're here, also relax `Dispaly for AlignmentError<Src, Dst>` to
permit `Dst: ?Sized` in exchange for `Dst: KnownLayout`. This is an
important relaxation since our APIs permit performing conversions into
unsized destination types with runtime alignment checking.

Also make all errors `Send + Sync` regardless of `Dst`, which only
exists at the type level, but is never instantiated.

Makes progress on #1297
joshlf added a commit that referenced this issue May 18, 2024
While we're here, also relax `Dispaly for AlignmentError<Src, Dst>` to
permit `Dst: ?Sized` in exchange for `Dst: KnownLayout`. This is an
important relaxation since our APIs permit performing conversions into
unsized destination types with runtime alignment checking.

Also make all errors `Send + Sync` regardless of `Dst`, which only
exists at the type level, but is never instantiated.

Makes progress on #1297
joshlf added a commit that referenced this issue May 18, 2024
While we're here, also relax `Dispaly for AlignmentError<Src, Dst>` to
permit `Dst: ?Sized` in exchange for `Dst: KnownLayout`. This is an
important relaxation since our APIs permit performing conversions into
unsized destination types with runtime alignment checking.

Also make all errors `Send + Sync` regardless of `Dst`, which only
exists at the type level, but is never instantiated.

Makes progress on #1297
joshlf added a commit that referenced this issue May 18, 2024
While we're here, also relax `Dispaly for AlignmentError<Src, Dst>` to
permit `Dst: ?Sized` in exchange for `Dst: KnownLayout`. This is an
important relaxation since our APIs permit performing conversions into
unsized destination types with runtime alignment checking.

Also make all errors `Send + Sync` regardless of `Dst`, which only
exists at the type level, but is never instantiated.

Makes progress on #1297
joshlf added a commit that referenced this issue May 18, 2024
While we're here, also relax `Dispaly for AlignmentError<Src, Dst>` to
permit `Dst: ?Sized` in exchange for `Dst: KnownLayout`. This is an
important relaxation since our APIs permit performing conversions into
unsized destination types with runtime alignment checking.

Also make all errors `Send + Sync` regardless of `Dst`, which only
exists at the type level, but is never instantiated.

Makes progress on #1297
joshlf added a commit that referenced this issue May 18, 2024
While we're here, also relax `Dispaly for AlignmentError<Src, Dst>` to
permit `Dst: ?Sized` in exchange for `Dst: KnownLayout`. This is an
important relaxation since our APIs permit performing conversions into
unsized destination types with runtime alignment checking.

Also make all errors `Send + Sync` regardless of `Dst`, which only
exists at the type level, but is never instantiated.

Makes progress on #1297
jswrenn pushed a commit that referenced this issue May 18, 2024
While we're here, also relax `Dispaly for AlignmentError<Src, Dst>` to
permit `Dst: ?Sized` in exchange for `Dst: KnownLayout`. This is an
important relaxation since our APIs permit performing conversions into
unsized destination types with runtime alignment checking.

Also make all errors `Send + Sync` regardless of `Dst`, which only
exists at the type level, but is never instantiated.

Makes progress on #1297
@joshlf joshlf added the blocking-next-release This issue should be resolved before we release on crates.io label May 30, 2024
joshlf added a commit that referenced this issue Jun 24, 2024
While we're here, also relax `Dispaly for AlignmentError<Src, Dst>` to
permit `Dst: ?Sized` in exchange for `Dst: KnownLayout`. This is an
important relaxation since our APIs permit performing conversions into
unsized destination types with runtime alignment checking.

Also make all errors `Send + Sync` regardless of `Dst`, which only
exists at the type level, but is never instantiated.

Finally, make `Display`'s verbosity conditional on `debug_assertions`.

Makes progress on #1297

Co-authored-by: John Wrenn <jswrenn@amazon.com>
joshlf added a commit that referenced this issue Jun 24, 2024
While we're here, also relax `Dispaly for AlignmentError<Src, Dst>` to
permit `Dst: ?Sized` in exchange for `Dst: KnownLayout`. This is an
important relaxation since our APIs permit performing conversions into
unsized destination types with runtime alignment checking.

Also make all errors `Send + Sync` regardless of `Dst`, which only
exists at the type level, but is never instantiated.

Finally, make `Display`'s verbosity conditional on `debug_assertions`.

Makes progress on #1297

Co-authored-by: John Wrenn <jswrenn@amazon.com>
joshlf added a commit that referenced this issue Jun 24, 2024
While we're here, also relax `Dispaly for AlignmentError<Src, Dst>` to
permit `Dst: ?Sized` in exchange for `Dst: KnownLayout`. This is an
important relaxation since our APIs permit performing conversions into
unsized destination types with runtime alignment checking.

Also make all errors `Send + Sync` regardless of `Dst`, which only
exists at the type level, but is never instantiated.

Finally, make `Display`'s verbosity conditional on `debug_assertions`.

Makes progress on #1297

Co-authored-by: John Wrenn <jswrenn@amazon.com>
joshlf added a commit that referenced this issue Jun 24, 2024
While we're here, also relax `Dispaly for AlignmentError<Src, Dst>` to
permit `Dst: ?Sized` in exchange for `Dst: KnownLayout`. This is an
important relaxation since our APIs permit performing conversions into
unsized destination types with runtime alignment checking.

Also make all errors `Send + Sync` regardless of `Dst`, which only
exists at the type level, but is never instantiated.

Finally, make `Display`'s verbosity conditional on `debug_assertions`.

Makes progress on #1297

Co-authored-by: John Wrenn <jswrenn@amazon.com>
github-merge-queue bot pushed a commit that referenced this issue Jun 26, 2024
While we're here, also relax `Dispaly for AlignmentError<Src, Dst>` to
permit `Dst: ?Sized` in exchange for `Dst: KnownLayout`. This is an
important relaxation since our APIs permit performing conversions into
unsized destination types with runtime alignment checking.

Also make all errors `Send + Sync` regardless of `Dst`, which only
exists at the type level, but is never instantiated.

Finally, make `Display`'s verbosity conditional on `debug_assertions`.

Makes progress on #1297

Co-authored-by: John Wrenn <jswrenn@amazon.com>
@joshlf joshlf added blocking-next-release-publicization and removed blocking-next-release This issue should be resolved before we release on crates.io labels Jul 1, 2024
jswrenn added a commit that referenced this issue Sep 19, 2024
github-merge-queue bot pushed a commit that referenced this issue Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant