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

Rollup of 12 pull requests #59289

Closed
wants to merge 48 commits into from
Closed

Rollup of 12 pull requests #59289

wants to merge 48 commits into from

Conversation

Centril
Copy link
Contributor

@Centril Centril commented Mar 19, 2019

Successful merges:

Failed merges:

r? @ghost

clarfonthey and others added 30 commits January 22, 2019 17:52
There are two moments when a BufRead object needs to empty it's internal
buffer:

- In a seek call;
- In a read call when all data in the internal buffer had been already
  consumed and the output buffer has a greater or equal size than the
  internal buffer.

In both cases, the buffer was not being properly emptied, but only
marked as consumed (self.pos = self.cap). That should be no problem if
the inner reader is only Read, but if it is Seek as well, then it's
possible to access the data in the buffer by using the seek_relative
method. In order to prevent this from happening, both self.pos and
self.cap should be set to 0.

Two test cases were added to detect that failure:

- test_buffered_reader_invalidated_after_read
- test_buffered_reader_invalidated_after_seek

Both tests are very similar to each other. The inner reader contains the
following data: [5, 6, 7, 0, 1, 2, 3, 4]. The buffer capacity is 3
bytes.

- First, we call fill_buffer, which loads [5, 6, 7] into the internal
  buffer, and then consume those 3 bytes.
- Then we either read the 5 remaining bytes in a single read call or we
  move to the end of the stream by calling seek. In both cases the
  buffer should be emptied to prevent the previous data [5, 6, 7] from
  being read.
- We now call seek_relative(-2) and read two bytes, which should give us
  the last 2 bytes of the stream: [3, 4].

Before this commit, the the seek_relative method would consider that
we're still in the range of the internal buffer, so instead of fetching
data from the inner reader, it would return the two last bytes that were
incorrectly still in the buffer: [6, 7]. Therefore, the test would fail.

Now, when seek_relative is called the buffer is empty. So the expected
data [3, 4] is fetched from the inner reader and the test passes.
…);` and potentially instantiated at different types.

(Updated to reflect changes in diagnostic output and compiletest infrastructure.)
Co-authored-by: Stephan Schauerte <[email protected]>
fabric-and-ink and others added 17 commits March 17, 2019 21:25
The use-case of `todo!()` macro is to be a much easier to type
alternative to `unimplemented!()` macro.
Add todo!() macro

The primary use-case of `todo!()` macro is to be a much easier to type
alternative to `unimplemented!()` macro.

EDIT: hide unpopular proposal about re-purposing unimplemented

<details>
However, instead of just replacing `unimplemented!()`, it gives it a
more nuanced meaning: a thing which is intentionally left
unimplemented and which should not be called at runtime. Usually,
you'd like to prevent such cases statically, but sometimes you, for
example, have to implement a trait only some methods of which are
applicable. There are examples in the wild of code doing this thing,
and in this case, the current message of `unimplemented`, "not *yet*
implemented" is slightly misleading.

With the addition of TODO, you have three nuanced choices for a
`!`-returning macro (in addition to a good-old panic we all love):

  * todo!()
  * unreachable!()
  * unimplemented!()

Here's a rough guideline what each one means:

- `todo`: use it during development, as a "hole" or placeholder. It
  might be a good idea to add a pre-commit hook which checks that
  `todo` is not accidentally committed.

- `unreachable!()`: use it when your code can statically guarantee
  that some situation can not happen. If you use a library and hit
  `unreachable!()` in the library's code, it's definitely a bug in the
  library. It's OK to have `unreachable!()` in the code base,
  although, if possible, it's better to replace it with
  compiler-verified exhaustive checks.

- `unimplemented!()`: use it when the type checker forces you to
  handle some situation, but there's a contract that a callee must not
  actually call the code. If you use a library and hit
  `unimplemented!()`, it's probably a bug in your code, though
  it *could* be a bug in the library (or library docs) as well. It is
  ok-ish to see an `unimplemented!()` in real code, but it usually
  signifies a clunky, eyebrow-rising API.
</details>
…constraints-on-bindings-too, r=nikomatsakis

extra testing of how NLL handles wildcard type `_`

test that wildcard type `_` is not duplicated by `type Foo<X> = (X, X);` and potentially instantiated at different types when used in type ascriptions in let bindings.

(NLL's handling of this for the type ascription *expression form* is currently broken, or at least differs from what AST-borrowck does. I'll file a separate bug about that. Its not something critical to address since that expression is guarded by `#![feature(type_ascription)]`.)

cc rust-lang#55748
…petrochenkov

Lint for redundant imports

This is an attempt at solving rust-lang#10178. The changes are suggested by `@petrochenkov`. I need some feedback on how to continue, since I get a lot of false positives in macro invocations in `libcore`. I suspect that the test

```
...
if directive.parent_scope.expansion == Mark::root() {
    return;
}
...
```

is wrong. But I don't know how to check correctly if I am at a macro expansion root – given that this is the reason for the errors in the first place.

Todos:

- [x] Solve problem with false positives
- [x] Turn hard error into warning
- [x] Add unit tests

Closes rust-lang#10178
Clarify distinction between floor() and trunc()

`floor()` rounds towards `-INF`, `trunc()` rounds towards 0.
This PR clarifies this in the examples.
…iplett

Add new test case for possible bug in BufReader

When reading a large chunk from a BufReader, if all the bytes from the buffer have been already consumed, the internal buffer is bypassed entirely. However, it is not invalidated, and it's possible to access its contents using the `seek_relative` method, because it tries to reuse the existing buffer.
…uietMisdreavus

Add default keyword handling in rustdoc

Fixes rust-lang#58898.

r? @QuietMisdreavus
Fix a tiny error in documentation of std::pin.

`new_unmoved` must be `mut` for passing to `std::mem::swap`.
Add peer_addr function to UdpSocket

Fixes rust-lang#59104

This is my first pull request to Rust, so opening early for some feedback.

My biggest question is: where do I add tests?

Any comments very much appreciated!
Be more discerning on when to attempt suggesting a comma in a macro invocation

Fix rust-lang#58796.
…cell-map-split, r=cramertj,Centril

Stabilize refcell_map_split feature

Closes rust-lang#51476.
@Centril
Copy link
Contributor Author

Centril commented Mar 19, 2019

@bors r+ p=12

@bors
Copy link
Contributor

bors commented Mar 19, 2019

📌 Commit 631534c has been approved by Centril

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Mar 19, 2019
@bors
Copy link
Contributor

bors commented Mar 19, 2019

⌛ Testing commit 631534c with merge bfd08ab511b0825b0462d38da4fa04bbfcb6e248...

@bors
Copy link
Contributor

bors commented Mar 19, 2019

💔 Test failed - checks-travis

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 19, 2019
@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-aux of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
[01:51:55] [RUSTC-TIMING] crossbeam_channel test:false 2.108
[01:51:55]    Compiling ignore v0.4.6
[01:52:08] [RUSTC-TIMING] ignore test:false 12.702
[01:52:08]    Compiling cargo v0.36.0 (/checkout/src/tools/cargo)
[01:52:09] warning: the item `libc` is imported redundantly
[01:52:09]     |
[01:52:09] 310 |         use libc;
[01:52:09]     |             ^^^^
[01:52:09]     |
[01:52:09]     |
[01:52:09]     = note: #[warn(unused_imports)] on by default
[01:52:09] 
[01:52:09] warning: the item `OsStr` is imported redundantly
[01:52:09]    --> src/tools/cargo/src/cargo/util/paths.rs:221:9
[01:52:09]     |
[01:52:09] 2   | use std::ffi::{OsStr, OsString};
[01:52:09]     |                ----- the item `OsStr` is already imported here
[01:52:09] 221 |     use std::ffi::OsStr;
[01:52:09]     |         ^^^^^^^^^^^^^^^
[01:52:09] 
[01:55:24] [RUSTC-TIMING] cargo test:false 195.917
---
[01:55:51] [RUSTC-TIMING] rusty_fork test:false 2.983
[01:55:51]    Compiling proptest v0.8.7
[01:56:14] [RUSTC-TIMING] proptest test:false 22.388
[01:56:14]    Compiling cargo v0.36.0 (/checkout/src/tools/cargo)
[01:56:15] warning: the item `libc` is imported redundantly
[01:56:15]    --> src/tools/cargo/tests/testsuite/death.rs:134:9
[01:56:15] 134 |     use libc;
[01:56:15]     |         ^^^^
[01:56:15]     |
[01:56:15]     = note: #[warn(unused_imports)] on by default
[01:56:15]     = note: #[warn(unused_imports)] on by default
[01:56:15] 
[01:56:16] error: the item `libc` is imported redundantly
[01:56:16]     |
[01:56:16] 310 |         use libc;
[01:56:16]     |             ^^^^
[01:56:16]     |
[01:56:16]     |
[01:56:16] note: lint level defined here
[01:56:16]    --> src/tools/cargo/src/cargo/lib.rs:1:24
[01:56:16]     |
[01:56:16] 1   | #![cfg_attr(test, deny(warnings))]
[01:56:16]     |                        ^^^^^^^^
[01:56:16]     = note: #[deny(unused_imports)] implied by #[deny(warnings)]
[01:56:16] 
[01:56:16] error: the item `OsStr` is imported redundantly
[01:56:16]    --> src/tools/cargo/src/cargo/util/paths.rs:221:9
[01:56:16]     |
[01:56:16] 2   | use std::ffi::{OsStr, OsString};
[01:56:16]     |                ----- the item `OsStr` is already imported here
[01:56:16] 221 |     use std::ffi::OsStr;
[01:56:16]     |         ^^^^^^^^^^^^^^^
[01:56:16] 
[01:56:23] error: aborting due to 2 previous errors
---
[01:57:40] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "test" "--target" "x86_64-unknown-linux-gnu" "-j" "4" "--release" "--locked" "--color" "always" "--manifest-path" "/checkout/src/tools/cargo/Cargo.toml" "--features" "rustc-workspace-hack/all-static"
[01:57:40] expected success, got: exit code: 101
[01:57:40] 
[01:57:40] 
[01:57:40] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/test/run-pass/pretty src/test/run-fail/pretty src/test/run-pass-valgrind/pretty src/test/run-pass-fulldeps/pretty src/tools/cargo src/tools/cargotest
[01:57:40] Build completed unsuccessfully in 0:26:26
[01:57:40] make: *** [check-aux] Error 1
[01:57:40] Makefile:50: recipe for target 'check-aux' failed
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:0109f080
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Tue Mar 19 12:12:09 UTC 2019
---
travis_time:end:03cdde48:start=1552997531338367328,finish=1552997531345228959,duration=6861631
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:10e3c88b
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:1b80c3ef
travis_time:start:1b80c3ef
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:01218110
$ dmesg | grep -i kill

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@Centril Centril closed this Mar 19, 2019
@Centril Centril deleted the rollup branch March 19, 2019 12:32
@Centril Centril added the rollup A PR which is a rollup label Oct 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rollup A PR which is a rollup S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.