Rollup of 9 pull requests #60834
Merged
Rollup of 9 pull requests #60834
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…eEndedIterators. r?Manishearth
This reverts commit 54aefc6.
modify the comment on `in_cycle` to reflect changes requested by ariel and myself.
Over in rust-lang#60378, we made `rustc` switch LLVM target triples dynamically based on the `MACOSX_DEPLOYMENT_TARGET` environment variable. This change was made to align with `clang`'s behavior, and therefore make cross-language LTO feasible on OS X. Otherwise, `rustc` would produce LLVM bitcode files with a target triple of `x86_64-apple-darwin`, `clang` would produce LLVM bitcode files with a target triple of `x86_64-apple-macosx$VERSION`, and the linker would complain. This change worked fine, except for one corner case: if you didn't have `MACOSX_DEPLOYMENT_TARGET` set, and you wanted to do LTO on just Rust code, you'd get warning messages similar to: ``` warning: Linking two modules of different target triples: ' is 'x86_64-apple-macosx10.7.0' whereas 'main.7rcbfp3g-cgu.4' is 'x86_64-apple-darwin' ``` This message occurs because libstd is compiled with `MACOSX_DEPLOYMENT_TARGET` set to 10.7. The LLVM bitcode distributed in libstd's rlibs, then, is tagged with the target triple of `x86_64-apple-macosx10.7.0`, while the bitcode `rustc` produces for "user" code is tagged with the target triple of `x86_64-apple-darwin`. It's not good to have LTO on just Rust code (probably much more common than cross-language LTO) warn by default. These warnings also break Cargo's testsuite. This change defaults to acting as though `MACOSX_DEPLOYMENT_TARGET` was set to 10.7. "user" code will then be given a target triple that is equivalent to the target triple libstd bitcode is already using. The above warning will therefore go away. `rustc` already assumes that compiling without `MACOSX_DEPLOYMENT_TARGET` means that we're compiling for a target compatible with OS X 10.7 (e.g. that things like TLS work properly). So this change is really just making things conform more closely to the status quo. (It's also worth noting that before and after this patch, compiling with `MACOSX_DEPLOYMENT_TARGET` set to, say, 10.9, works just fine: target triples with an "apple" version ignore OS versions when checking compatibility, so bitcode with a `x86_64-apple-macosx10.7.0` triple works just fine with bitcode with a `x86_64-apple-macosx10.9.0` triple.)
Co-Authored-By: Mazdak Farrokhzad <twingoow@gmail.com>
Changes: ```` Rustfmt all the things Clippy dogfood Update for compiletest changes Use symbols instead of strings Rustup to rustc 1.36.0-nightly (1764b29 2019-05-12) Add regression test for identity_conversion FP UI test cleanup: Extract many_single_char_names tests Add tests for empty_loop lint Add in_macro again Rename in_macro to in_macro_or_desugar ````
Add implementations of last in terms of next_back on a bunch of DoubleEndedIterators Provided a `DoubleEndedIterator` has finite length, `Iterator::last` is equivalent to `DoubleEndedIterator::next_back`. But searching forwards through the iterator when it's unnecessary is obviously not good for performance. I ran into this on one of the collection iterators. I tried adding appropriate overloads for a bunch of the iterator adapters like filter, map, etc, but I ran into a lot of type inference failures after doing so. The other interesting case is what to do with `Repeat`. Do we consider it part of the contract that `Iterator::last` will loop forever on it? The docs do say that the iterator will be evaluated until it returns None. This is also relevant for the adapters, it's trivially easy to observe whether a `Map` adapter invoked its closure a zillion times or just once for the last element.
as_ptr returns a read-only pointer Add comments to `as_ptr` methods to warn that these are read-only pointers, and writing to them is UB. [It was pointed out](https://internals.rust-lang.org/t/as-ptr-vs-as-mut-ptr/9940) that `CStr` does not even have an `as_mut_ptr`. I originally was going to add one, but there is no method at all that would mutate a `CStr`. Was that a deliberate choice or should I add an `as_mut_ptr` (similar to [what I did for `str`](rust-lang#58200))?
…r-investigation, r=pnkfelix forego caching for all participants in cycles, apart from root node This is a targeted fix for rust-lang#60010, which uncovered a pretty bad failure of our caching strategy in the face of coinductive cycles. The problem is explained in the comment in the PR on the new field, `in_cycle`, but I'll reproduce it here: > Starts out as false -- if, during evaluation, we encounter a > cycle, then we will set this flag to true for all participants > in the cycle (apart from the "head" node). These participants > will then forego caching their results. This is not the most > efficient solution, but it addresses rust-lang#60010. The problem we > are trying to prevent: > > - If you have `A: AutoTrait` requires `B: AutoTrait` and `C: NonAutoTrait` > - `B: AutoTrait` requires `A: AutoTrait` (coinductive cycle, ok) > - `C: NonAutoTrait` requires `A: AutoTrait` (non-coinductive cycle, not ok) > > you don't want to cache that `B: AutoTrait` or `A: AutoTrait` > is `EvaluatedToOk`; this is because they were only considered > ok on the premise that if `A: AutoTrait` held, but we indeed > encountered a problem (later on) with `A: AutoTrait. So we > currently set a flag on the stack node for `B: AutoTrait` (as > well as the second instance of `A: AutoTrait`) to supress > caching. > > This is a simple, targeted fix. The correct fix requires > deeper changes, but would permit more caching: we could > basically defer caching until we have fully evaluated the > tree, and then cache the entire tree at once. I'm not sure what the impact of this fix will be in terms of existing crates or performance: we were accepting incorrect code before, so there will perhaps be some regressions, and we are now caching less. As the comment above notes, we could do a lot better than this fix, but that would involve more invasive rewrites. I thought it best to start with something simple. r? @pnkfelix -- but let's do crater/perf run cc @arielb1
…lacrum Allow subdirectories to be tested by x.py test Fixes rust-lang#60718. As far as I can tell, multiple `--test-args` flags are ignored (only the first is respected), so if you specify a subdirectory, you won't also be able to filter using `--test-args`. If you don't specify a subdirectory, `--test-args` will continue working as usual, so this is strictly an improvement on the current state of affairs.
fix Miri This reverts rust-lang#60156, which turned out to be a dead end (see rust-lang#60469). r? @oli-obk
…followup, r=estebank default to $ARCH-apple-macosx10.7.0 LLVM triple for darwin targets Over in rust-lang#60378, we made `rustc` switch LLVM target triples dynamically based on the `MACOSX_DEPLOYMENT_TARGET` environment variable. This change was made to align with `clang`'s behavior, and therefore make cross-language LTO feasible on OS X. Otherwise, `rustc` would produce LLVM bitcode files with a target triple of `x86_64-apple-darwin`, `clang` would produce LLVM bitcode files with a target triple of `x86_64-apple-macosx$VERSION`, and the linker would complain. This change worked fine, except for one corner case: if you didn't have `MACOSX_DEPLOYMENT_TARGET` set, and you wanted to do LTO on just Rust code, you'd get warning messages similar to: ``` warning: Linking two modules of different target triples: ' is 'x86_64-apple-macosx10.7.0' whereas 'main.7rcbfp3g-cgu.4' is 'x86_64-apple-darwin' ``` This message occurs because libstd is compiled with `MACOSX_DEPLOYMENT_TARGET` set to 10.7. The LLVM bitcode distributed in libstd's rlibs, then, is tagged with the target triple of `x86_64-apple-macosx10.7.0`, while the bitcode `rustc` produces for "user" code is tagged with the target triple of `x86_64-apple-darwin`. It's not good to have LTO on just Rust code (probably much more common than cross-language LTO) warn by default. These warnings also break Cargo's testsuite. This change defaults to acting as though `MACOSX_DEPLOYMENT_TARGET` was set to 10.7. "user" code will then be given a target triple that is equivalent to the target triple libstd bitcode is already using. The above warning will therefore go away. `rustc` already assumes that compiling without `MACOSX_DEPLOYMENT_TARGET` means that we're compiling for a target compatible with OS X 10.7 (e.g. that things like TLS work properly). So this change is really just making things conform more closely to the status quo. (It's also worth noting that before and after this patch, compiling with `MACOSX_DEPLOYMENT_TARGET` set to, say, 10.9, works just fine: target triples with an "apple" version ignore OS versions when checking compatibility, so bitcode with a `x86_64-apple-macosx10.7.0` triple works just fine with bitcode with a `x86_64-apple-macosx10.9.0` triple.)
…n-existential-types, r=oli-obk Allow late-bound regions in existential types closes rust-lang#60655 r? @oli-obk
…r-future, r=Centril Improve the "must use" lint for `Future` Fixes rust-lang#60797
submodules: update clippy from 3710ec5 to ad3269c Changes: ```` Rustfmt all the things Clippy dogfood Update for compiletest changes Use symbols instead of strings Rustup to rustc 1.36.0-nightly (1764b29 2019-05-12) Add regression test for identity_conversion FP UI test cleanup: Extract many_single_char_names tests Add tests for empty_loop lint Add in_macro again Rename in_macro to in_macro_or_desugar ```` r? @oli-obk
|
@bors r+ p=9 rollup=never |
|
📌 Commit 2e844ef has been approved by |
bors
added
the
S-waiting-on-bors
bors
added a commit
that referenced
this pull request
May 14, 2019
Rollup of 9 pull requests Successful merges: - #60130 (Add implementations of last in terms of next_back on a bunch of DoubleEndedIterators) - #60443 (as_ptr returns a read-only pointer) - #60444 (forego caching for all participants in cycles, apart from root node) - #60719 (Allow subdirectories to be tested by x.py test) - #60780 (fix Miri) - #60788 (default to $ARCH-apple-macosx10.7.0 LLVM triple for darwin targets) - #60799 (Allow late-bound regions in existential types) - #60808 (Improve the "must use" lint for `Future`) - #60819 (submodules: update clippy from 3710ec5 to ad3269c) Failed merges: r? @ghost
|
☀️ Test successful - checks-travis, status-appveyor |
|
📣 Toolstate changed by #60834! Tested on commit 372be4f. 🎉 clippy-driver on windows: build-fail → test-pass (cc @Manishearth @llogiq @mcarton @oli-obk @phansch, @rust-lang/infra). |
rust-highfive
added a commit
to rust-lang-nursery/rust-toolstate
that referenced
this pull request
May 14, 2019
Tested on commit rust-lang/rust@372be4f. Direct link to PR: <rust-lang/rust#60834> 🎉 clippy-driver on windows: build-fail → test-pass (cc @Manishearth @llogiq @mcarton @oli-obk @phansch, @rust-lang/infra). 🎉 clippy-driver on linux: build-fail → test-pass (cc @Manishearth @llogiq @mcarton @oli-obk @phansch, @rust-lang/infra). 🎉 miri on windows: build-fail → test-pass (cc @oli-obk @RalfJung @eddyb, @rust-lang/infra). 🎉 miri on linux: build-fail → test-pass (cc @oli-obk @RalfJung @eddyb, @rust-lang/infra). 🎉 rls on windows: build-fail → test-fail (cc @Xanewok, @rust-lang/infra). 🎉 rls on linux: build-fail → test-fail (cc @Xanewok, @rust-lang/infra).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Successful merges:
Future#60808 (Improve the "must use" lint forFuture)Failed merges:
r? @ghost