-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Require TAITs to be mentioned in the signatures of functions that register hidden types for them #112652
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @lcnr (or someone else) soon. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
|
This comment has been minimized.
This comment has been minimized.
9f5c836
to
ca8ff0e
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
abbc6da
to
a078624
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
cb6836b
to
efeee50
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure if this is ready yet and it might be nice to move the cleanup into a separate PR which can be approved separately.
My understanding was that we want to only allow defining uses for things in signatures, but make other uses in the same module ambiguous. This is not what's implemented in 9523613a1dcadfcd1105da1ee86c18879640976b afaict. I would expect us to implement this by keep the current logic for trait solving and checking in type_of
whether any defining use was outside of the "currently stable" defining uses, if not error and suggest moving the function out of the defining module or whatever.
@rustbot author |
…-in-OpaqueTypeCollector, r=oli-obk Don't substitute a GAT that has mismatched generics in `OpaqueTypeCollector` Fixes rust-lang#111828 I didn't put up minimized UI tests for rust-lang#112510 or rust-lang#112873 because they'd minimize to literally the same code, but with different substs on the trait/impl. I don't think that warrants duplicate tests given the nature of the fix. r? `@oli-obk` ---- Side-note: I checked, and this isn't fixed by rust-lang#112652 -- I think we discussed whether or not that PR fixed it either intentionally or by accident. The code here isn't really touched by that PR either as far as I can tell? Also, sorry, did some other drive-bys. Hope it doesn't make rebasing rust-lang#112652 too difficult 😅
☔ The latest upstream changes (presumably #112914) made this pull request unmergeable. Please resolve the merge conflicts. |
78f41ed
to
b8345ac
Compare
This comment has been minimized.
This comment has been minimized.
b8345ac
to
5bdadf5
Compare
if !self.tcx.opaque_types_defined_by(item_def_id).contains(&self.def_id) { | ||
self.tcx.sess.emit_err(TaitForwardCompat { | ||
span: hidden_type.span, | ||
item_span: self | ||
.tcx | ||
.def_ident_span(item_def_id) | ||
.unwrap_or_else(|| self.tcx.def_span(item_def_id)), | ||
}); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the main change of this PR. We double check that any item that produces hidden types also has the corresponding opaque type in its signature
@rustbot ready |
☔ The latest upstream changes (presumably #113429) made this pull request unmergeable. Please resolve the merge conflicts. |
…ister hidden types for them
…ntermediate `Vec` first
@bors r=compiler-errors |
…er-errors Require TAITs to be mentioned in the signatures of functions that register hidden types for them r? `@lcnr` `@compiler-errors` This implements the lang team decision from [the TAIT design meeting](https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/design.20meeting.202023-05-31.20TAITs/near/362518164).
☀️ Test successful - checks-actions |
Finished benchmarking commit (d4096e0): comparison URL. Overall result: ❌ regressions - no action needed@rustbot label: -perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 655.621s -> 656.508s (0.14%) |
r? @lcnr @compiler-errors
This implements the lang team decision from the TAIT design meeting.