-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Gate the type length limit check behind a nightly flag #127670
Conversation
imo this is the right choice - there is more fallout than I think we anticipated with re-adding the type length limit. r=me, but maybe we want to wait a day or so for anyone in the team to dissent |
@bors r=jackh726 we can easily turn this back on |
@bors rollup=never |
📣 Toolstate changed by #127670! Tested on commit 88fa119. 💔 reference on windows: test-pass → test-fail (cc @Havvy @matthewjasper @ehuss). |
Tested on commit rust-lang/rust@88fa119. Direct link to PR: <rust-lang/rust#127670> 💔 reference on windows: test-pass → test-fail (cc @Havvy @matthewjasper @ehuss). 💔 reference on linux: test-pass → test-fail (cc @Havvy @matthewjasper @ehuss).
☀️ Test successful - checks-actions |
@compiler-errors Can you please update the reference at https://github.com/rust-lang/reference/blob/master/src/attributes/limits.md#the-type_length_limit-attribute? This PR has broken the test there, and I'm not sure how the documentation should be updated. Is this attribute deprecated now? |
Sure. The attribute is not deprecated per se, but it doesn't do anything right now. Previously it didn't do anything in real code, but it was easier to make a pathological example like rust-lang/reference#1026 did after it previously broke in #72412. See #125460. I'll just remove the test. Not sure what we should do with the documentation. I'll probably just mention the attr doesn't do anything (which isn't totally true -- it actually is used in diagnostics, but I don't want to encourage people to use the attr). |
Finished benchmarking commit (88fa119): comparison URL. Overall result: ✅ improvements - 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)This benchmark run did not return any relevant results for this metric. CyclesResults (primary -1.0%)This 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: 702.998s -> 699.561s (-0.49%) |
Effectively disables the type length limit by introducing a
-Zenforce-type-length-limit
which defaults tofalse
, since making the length limit actually be enforced ended up having a worse fallout than expected. We still keep the code around, but the type length limit attr is now a noop (except for its usage in some diagnostics code?).r? @lcnr -- up to you to decide what team consensus we need here since this reverses an FCP decision.
Reopens #125460 (if we decide to reopen it or keep it closed)
Effectively reverses the decision FCP'd in #125507
Closes #127346