rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
96.62k stars 12.48k forks source link

regression: overflow evaluating, Send/Sync? #61472

Closed Mark-Simulacrum closed 5 years ago

Mark-Simulacrum commented 5 years ago

root: fuzzy-pickles - 1 detected crates which regressed due to this; cc @shepmaster -- resolved

* fuzzy-pickles-0.1.0: [start](https://crater-reports.s3.amazonaws.com/beta-1.36-2/1.35.0/reg/fuzzy-pickles-0.1.0/log.txt) v. [end](https://crater-reports.s3.amazonaws.com/beta-1.36-2/beta-2019-05-30/reg/fuzzy-pickles-0.1.0/log.txt); cc @shepmaster

root: lang-c - 1 detected crates which regressed due to this; cc @vickenty -- resolved

``` * lang-c-0.5.1: [start](https://crater-reports.s3.amazonaws.com/beta-1.36-2/1.35.0/reg/lang-c-0.5.1/log.txt) v. [end](https://crater-reports.s3.amazonaws.com/beta-1.36-2/beta-2019-05-30/reg/lang-c-0.5.1/log.txt); cc @vickenty ```

root: conch-runtime - 1 detected crates which regressed due to this; cc @ipetkov -- resolved

* vt6/6shell: [start](https://crater-reports.s3.amazonaws.com/beta-1.36-2/1.35.0/gh/vt6.6shell/log.txt) v. [end](https://crater-reports.s3.amazonaws.com/beta-1.36-2/beta-2019-05-30/gh/vt6.6shell/log.txt); cc @vt6

root: kailua_langsvr - 1 detected crates which regressed due to this; cc @lifthrasiir -- cannot reproduce, appears fixed?

* devcat-studio/kailua: [start](https://crater-reports.s3.amazonaws.com/beta-1.36-2/1.35.0/gh/devcat-studio.kailua/log.txt) v. [end](https://crater-reports.s3.amazonaws.com/beta-1.36-2/beta-2019-05-30/gh/devcat-studio.kailua/log.txt); cc @devcat-studio
ipetkov commented 5 years ago

A fix for conch-runtime has been published to crates.io, but build times will suffer due to #60846

shepmaster commented 5 years ago

I've been told that this is due to #60444. What's strange about the fuzzy-pickles regression is that it only appears when running rustdoc. Why would running rustdoc have different behavior for evaluating these bounds?

pnkfelix commented 5 years ago

Maybe it arises from rustdoc's attempt to determine what traits to list as implemented? I don't think rustc itself will attempt to check if a type implements Send/Sync until you actually try to use a type in a context requiring one of those, but rustdoc certainly does check that for every struct/enum/union definition it documents, right?

Update: of course, if that is indeed the case, then that is a not-great failure mode for rustdoc vs rustc...

Update 2: I think I have validated the above hypothesis for the cause. I tried adding the line below to ast.rs in fuzzy-pickles:

const _ASSERT_FILE_IS_SEND: () = { struct S<T: Send>(Option<T>); S::<File>(None); () };

which acts as a way to force rustc to check up front whether a type is Send.

pnkfelix commented 5 years ago

(BTW adding #![recursion_limit="128"], as suggested by the diagnostic, does at least seem to address the problem for fuzzy-pickles....)

nikomatsakis commented 5 years ago

@shepmaster rustdoc does some different access patterns, especially around traits -- for example, it wants to print out whether or not a type is Send, even if there is no need for that (i.e., the crate doesn't ever do anything that would require Send).

nikomatsakis commented 5 years ago

So: I'm working on addressing the performance hurdles here, but I think that the "correctness side" is correct, and we ought not to revert it. The only real question is whether to issue a warning period. I'm not really sure how to go about doing that -- it may be possible, I'd have to think on it. I'd prefer to focus on improving the performance, however.

shepmaster commented 5 years ago

Creating a sequence of 63 nested structs triggers this problem:

N=63; (echo "struct S0;"; for i in $(seq 0 $N); do echo "struct S$(( $i + 1 ))(S${i});"; done; echo "fn is_send<T: Send>() {}"; echo "fn main() { is_send::<S${N}>(); }") > long.rs
$ rustc +nightly long.rs
error[E0275]: overflow evaluating the requirement `S0: std::marker::Send`
  --> long.rs:67:13
   |
67 | fn main() { is_send::<S63>(); }
   |             ^^^^^^^^^^^^^^
   |
   = help: consider adding a `#![recursion_limit="128"]` attribute to your crate
   = note: required because it appears within the type `S1`
   = note: required because it appears within the type `S2`
   = note: required because it appears within the type `S3`
   = note: required because it appears within the type `S4`
   = note: ... blah blah blah ...
   = note: required because it appears within the type `S61`
   = note: required because it appears within the type `S62`
   = note: required because it appears within the type `S63`
note: required by `is_send`
  --> long.rs:66:1
   |
66 | fn is_send<T: Send>() {}
   | ^^^^^^^^^^^^^^^^^^^^^
Whole program ```rust struct S0; struct S1(S0); struct S2(S1); struct S3(S2); struct S4(S3); struct S5(S4); struct S6(S5); struct S7(S6); struct S8(S7); struct S9(S8); struct S10(S9); struct S11(S10); struct S12(S11); struct S13(S12); struct S14(S13); struct S15(S14); struct S16(S15); struct S17(S16); struct S18(S17); struct S19(S18); struct S20(S19); struct S21(S20); struct S22(S21); struct S23(S22); struct S24(S23); struct S25(S24); struct S26(S25); struct S27(S26); struct S28(S27); struct S29(S28); struct S30(S29); struct S31(S30); struct S32(S31); struct S33(S32); struct S34(S33); struct S35(S34); struct S36(S35); struct S37(S36); struct S38(S37); struct S39(S38); struct S40(S39); struct S41(S40); struct S42(S41); struct S43(S42); struct S44(S43); struct S45(S44); struct S46(S45); struct S47(S46); struct S48(S47); struct S49(S48); struct S50(S49); struct S51(S50); struct S52(S51); struct S53(S52); struct S54(S53); struct S55(S54); struct S56(S55); struct S57(S56); struct S58(S57); struct S59(S58); struct S60(S59); struct S61(S60); struct S62(S61); struct S63(S62); struct S64(S63); fn is_send() {} fn main() { is_send::(); } ```
pnkfelix commented 5 years ago

(This may be an instance of the umbrella issue #61960 .)

pnkfelix commented 5 years ago

I have confirmed that that the fuzzy-pickles issue was resolved somewhere between nightly-2019-06-16 (0dc9e9c10) and nightly-2019-06-17 (4edff843d) which is enough to convince me that PR #61754 probably resolved the problem here for fuzzy-pickles.

pnkfelix commented 5 years ago

@shepmaster By the way, I'm not sure your long.rs demonstrates the regression here. From what I can tell, that code does not involve a cyclic structure that we end up throwing out (and would not have previously thrown out).

did you double-check that your long.rs actually compiles on stable? From what I can tell, the stable rustc (rustc 1.35.0 (3c235d560 2019-05-20)) issues the same overflow error diagnostic.

pnkfelix commented 5 years ago

I have confirmed that that the conch-runtime issue was resolved somewhere between nightly-2019-06-16 (0dc9e9c) and nightly-2019-06-17 (4edff84) which is enough to convince me that PR #61754 probably resolved the problem here for conch-runtime

shepmaster commented 5 years ago

did you double-check that your long.rs actually compiles on stable

😞 I did not. I apologize for the noise!

nikomatsakis commented 5 years ago

I have confirmed that the lang-c crate issue was resolved between nightly-2019-06-16 and nightly-2019-06-17.

nikomatsakis commented 5 years ago

I am not able to reproduce the problems from kailua_langsvr -- everything seems to build fine when I checkout the particular commit, both with today's nightly and with nightly-2019-06-16. I tried doing: carog check, cargo build, and cargo doc.

pnkfelix commented 5 years ago

This all seems like evidence that the regression is addressed by #61754.

(So the only question remaining is whether to backport, a question which will be addressed this week; see discussion on the PR itself)

pnkfelix commented 5 years ago

(I'm going to close this as resolved, as the bug is fixed on nightly, and the decision of whether to backport PR #61754 will be made independently of whether this issue is closed or open.)

mati865 commented 5 years ago

I think you meant #61754