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

Consider compiler-erroring instead of panicking in DST APIs #1149

Closed
Tracked by #671
jswrenn opened this issue Apr 29, 2024 · 1 comment · Fixed by #1154
Closed
Tracked by #671

Consider compiler-erroring instead of panicking in DST APIs #1149

jswrenn opened this issue Apr 29, 2024 · 1 comment · Fixed by #1154

Comments

@jswrenn
Copy link
Collaborator

jswrenn commented Apr 29, 2024

This issue goes partway towards addressing #325.

For example, Ref::from_prefix_with_trailing_elements documents a panic if Self is a ZST. However, the ZST-iness (pronounced "zestiness") of a type relevant here is always a static property and can be reported via postmonomorphization error.

The advantages of this are several-fold:

  • it allows us to maintain the consistent policy that our APIs do not panic (with the exception of panics in user-supplied TryFromBytes validators)
  • it is forwards compatible with us relaxing the compile error to a panic in future non-breaking releases
  • it may have optimizer advantages

Postmonomorphization errors currently have a bit of a 'weirdness' factor (and an unwieldy name), but I suspect that will change. The recent stabilization notes for inline const exprs cite ZST-checking as a motivating case of what the feature will make possible.

@jswrenn jswrenn mentioned this issue Apr 29, 2024
73 tasks
@joshlf
Copy link
Member

joshlf commented Apr 29, 2024

Note that this partially addresses #325. I think we should leave #325 open and continue the main discussion there, but it's good to have this as a separate issue so we can block #671 on it.

jswrenn added a commit that referenced this issue May 1, 2024
Presently, we deny ZSTy DSTs in our APIs via panicking at runtime. However, the
ZSTiness of a DST is statically detectable and can be denied instead at compile
time. This PR replaces our ZSTy DST panics with compile-time assertions. Doing
gives us the freedom later provide meaningful runtime semantics in such cases.

Partially addresses #325
Closes #1149
jswrenn added a commit that referenced this issue May 1, 2024
Presently, we deny ZSTy DSTs in our APIs via panicking at runtime. However, the
ZSTiness of a DST is statically detectable and can be denied instead at compile
time. This PR replaces our ZSTy DST panics with compile-time assertions. Doing
gives us the freedom later provide meaningful runtime semantics in such cases.

Partially addresses #325
Closes #1149
jswrenn added a commit that referenced this issue May 1, 2024
Presently, we deny ZSTy DSTs in our APIs via panicking at runtime. However, the
ZSTiness of a DST is statically detectable and can be denied instead at compile
time. This PR replaces our ZSTy DST panics with compile-time assertions. Doing
gives us the freedom later provide meaningful runtime semantics in such cases.

Partially addresses #325
Closes #1149
joshlf pushed a commit that referenced this issue May 1, 2024
Presently, we deny ZSTy DSTs in our APIs via panicking at runtime. However, the
ZSTiness of a DST is statically detectable and can be denied instead at compile
time. This PR replaces our ZSTy DST panics with compile-time assertions. Doing
gives us the freedom later provide meaningful runtime semantics in such cases.

Partially addresses #325
Closes #1149
github-merge-queue bot pushed a commit that referenced this issue May 1, 2024
Presently, we deny ZSTy DSTs in our APIs via panicking at runtime. However, the
ZSTiness of a DST is statically detectable and can be denied instead at compile
time. This PR replaces our ZSTy DST panics with compile-time assertions. Doing
gives us the freedom later provide meaningful runtime semantics in such cases.

Partially addresses #325
Closes #1149
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants