Use PathBuf instead of String where applicable by oli-obk · Pull Request #46335 · rust-lang/rust

oli-obk · 2017-11-28T16:30:26Z

kennytm · 2017-11-28T16:40:31Z

Is this FIXME still relevant in this place?

I think it's fixed with this PR, but there's no repro example on the issue

oli-obk · 2017-11-29T13:49:50Z

Travis likes it now

bors · 2017-12-01T18:50:59Z

☔ The latest upstream changes (presumably #46338) made this pull request unmergeable. Please resolve the merge conflicts.

oli-obk · 2017-12-01T22:40:41Z

rebased

jseyfried · 2017-12-01T23:17:06Z

Excellent, looks great! Will do detailed review tomorrow.

jseyfried · 2017-12-04T03:04:39Z

Why do we need PathBuf -> String -> PathBuf here?

lol. I have no idea what I did there. That was probably some intermediate step to get stuff to compile.

Ah no, I remember why. This is FileName -> String -> PathBuf. I should probably change the path field to be a FileName

kennytm · 2017-12-04T16:43:45Z

There are two run-pass-fulldeps tests failed with "position 0 does not resolve to a source location".

Details

[01:01:07] ---- [run-pass] run-pass-fulldeps/issue-35829.rs stdout ----
[01:01:07] [01:01:07] error: test run failed!
[01:01:07] status: exit code: 101
[01:01:07] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass-fulldeps/issue-35829.stage2-x86_64-unknown-linux-gnu"
[01:01:07] stdout:
[01:01:07] ------------------------------------------
[01:01:07] [01:01:07] ------------------------------------------
[01:01:07] stderr:
[01:01:07] ------------------------------------------
[01:01:07] thread 'main' panicked at 'Some tests failed', /checkout/src/tools/compiletest/src/main.rs:331:21
[01:01:07] thread 'main' panicked at 'position 0 does not resolve to a source location', /checkout/src/libsyntax/codemap.rs:632:8
[01:01:07] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[01:01:07] [01:01:07] ------------------------------------------
[01:01:07] [01:01:07] thread '[run-pass] run-pass-fulldeps/issue-35829.rs' panicked at 'explicit panic', /checkout/src/tools/compiletest/src/runtest.rs:2570:8
[01:01:07] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[01:01:07] [01:01:07] ---- [run-pass] run-pass-fulldeps/qquote.rs stdout ----
[01:01:07] [01:01:07] error: test run failed!
[01:01:07] status: exit code: 101
[01:01:07] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass-fulldeps/qquote.stage2-x86_64-unknown-linux-gnu"
[01:01:07] stdout:
[01:01:07] ------------------------------------------
[01:01:07] [01:01:07] ------------------------------------------
[01:01:07] stderr:
[01:01:07] ------------------------------------------
[01:01:07] thread 'main' panicked at 'position 0 does not resolve to a source location', /checkout/src/libsyntax/codemap.rs:632:8
[01:01:07] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[01:01:07] [01:01:07] ------------------------------------------
[01:01:07] [01:01:07] thread '[run-pass] run-pass-fulldeps/qquote.rs' panicked at 'explicit panic', /checkout/src/tools/compiletest/src/runtest.rs:2570:8
[01:01:07] [01:01:07] [01:01:07] failures:
[01:01:07] [run-pass] run-pass-fulldeps/issue-35829.rs
[01:01:07] [run-pass] run-pass-fulldeps/qquote.rs
[01:01:07] [01:01:07] test result: FAILED. 84 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out

oli-obk · 2017-12-05T13:27:46Z

I assumed (wrongly) that span_to_unmapped_path handled DUMMY_SP, reintroduced the check that was there berfore

jseyfried · 2017-12-05T21:14:32Z

Makes sense! @bors r+

bors · 2017-12-05T21:14:33Z

📌 Commit 7870500 has been approved by jseyfried

bors · 2017-12-05T22:44:05Z

⌛ Testing commit 78705008cc10f1a0fc8495b79ae87561330db61f with merge 4fd6bd5f3faa692795a2907ff09115df2071fe62...

bors · 2017-12-06T00:58:06Z

💔 Test failed - status-travis

oli-obk · 2017-12-06T05:33:22Z

Legit. Aux builder pretty-fulldeps

oli-obk · 2017-12-06T14:02:14Z

That was a weird one. ./x.py test never catches it. ./x.py test src/run-pass-fulldeps/pretty doesn't catch it if ./x.py test ran through successfully before.

kennytm · 2017-12-06T14:05:55Z

@oli-obk That sounds like a rustbuild bug...

oli-obk · 2017-12-06T14:10:33Z

@kennytm I'm not sure, since it's explicitly done on the aux builder, this might be intentional

kennytm · 2017-12-06T14:12:39Z

@oli-obk The ./x.py test never catches it is intentional, but ./x.py test src/run-pass-fulldeps/pretty doesn't catch it is certainly not. But I'm assuming you're running it in --stage 2 since some tests can't be run in earlier stages.

kennytm · 2017-12-14T09:33:54Z

@bors r=jseyfried

bors · 2017-12-14T09:33:55Z

📌 Commit c273d45 has been approved by jseyfried

bors · 2017-12-14T10:08:45Z

🔒 Merge conflict

bors · 2017-12-14T10:11:58Z

☔ The latest upstream changes (presumably #46605) made this pull request unmergeable. Please resolve the merge conflicts.

kennytm · 2017-12-14T10:44:13Z

@bors delegate+

bors · 2017-12-14T10:44:15Z

✌️ @oli-obk can now approve this pull request

oli-obk · 2017-12-14T11:51:24Z

@bors r=jseyfried

bors · 2017-12-14T11:51:25Z

📌 Commit d732da8 has been approved by jseyfried

bors · 2017-12-14T12:50:07Z

⌛ Testing commit d732da8 with merge 8624ea5...

@jseyfried

Use PathBuf instead of String where applicable r? @jseyfried

bors · 2017-12-14T15:37:34Z

☀️ Test successful - status-appveyor, status-travis
Approved by: jseyfried
Pushing 8624ea5 to master...

nrc · 2017-12-15T02:47:35Z

@oli-obk RLS and Rustfmt have both been fixed (using your commits), could you send a PR to update them in the Rust repo and unbreak them for testing (if you only touch the submodules and Cargo.lock, then pre-emptive r=me, p=1).

oli-obk · 2017-12-15T07:22:25Z

on it

This fixes an accidental regression rust-lang#46335 where the behavior of `Path::ends_with` is different from `str::ends_with` (paths operate over components, strs operate over chars).

…on, r=estebank rustc: Spawn `cmd /c` for `.bat` scripts This fixes an accidental regression rust-lang#46335 where the behavior of `Path::ends_with` is different from `str::ends_with` (paths operate over components, strs operate over chars).

This fixes an accidental regression rust-lang#46335 where the behavior of `Path::ends_with` is different from `str::ends_with` (paths operate over components, strs operate over chars).

rust-highfive assigned jseyfried Nov 28, 2017

kennytm reviewed Nov 28, 2017
View reviewed changes

kennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 28, 2017

oli-obk force-pushed the cleanups branch 2 times, most recently from 503e433 to 12c9233Compare November 29, 2017 11:48

oli-obk force-pushed the cleanups branch from 84ab06b to 12c9233Compare December 1, 2017 07:50

oli-obk force-pushed the cleanups branch from 12c9233 to 31ba3ecCompare December 1, 2017 20:03

jseyfried reviewed Dec 4, 2017
View reviewed changes

oli-obk force-pushed the cleanups branch from 31ba3ec to 0341494Compare December 4, 2017 08:02

oli-obk force-pushed the cleanups branch from 0341494 to 7870500Compare December 5, 2017 13:27

kennytm added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 6, 2017

kennytm added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 6, 2017

Use PathBuf instead of String where applicable
d732da8

oli-obk force-pushed the cleanups branch from c273d45 to d732da8Compare December 14, 2017 10:22

bors added a commit that referenced this pull request Dec 14, 2017
Auto merge of#46335- oli-obk:cleanups, r=jseyfried
8624ea5
Use PathBuf instead of String where applicable r? @jseyfried

bors mentioned this pull request Dec 14, 2017
Validate miri against the HIR const evaluator #45002
Merged

bors merged commit d732da8 into rust-lang:masterDec 14, 2017

oli-obk deleted the cleanups branch December 15, 2017 07:08

alexcrichton mentioned this pull request Dec 20, 2017
Support meaningful spans in the stable version of proc-macro2 dtolnay/proc-macro2#36
Merged

alexcrichton mentioned this pull request Jan 16, 2018
rustc: Spawn cmd /c for .bat scripts #47505
Merged

ehuss mentioned this pull request May 9, 2022
Docs say --remap-path-prefix treats paths as strings during matching but compiler does not #96870
Open

Uh oh!

Uh oh!

Conversation

oli-obk commented Nov 28, 2017

Uh oh!

kennytmNov 28, 2017

Choose a reason for hiding this comment

Uh oh!

oli-obkNov 28, 2017

Choose a reason for hiding this comment

Uh oh!

oli-obk commented Nov 29, 2017

Uh oh!

bors commented Dec 1, 2017

Uh oh!

oli-obk commented Dec 1, 2017

Uh oh!

jseyfried commented Dec 1, 2017

Uh oh!

jseyfriedDec 4, 2017

Choose a reason for hiding this comment

Uh oh!

oli-obkDec 4, 2017

Choose a reason for hiding this comment

Uh oh!

oli-obkDec 4, 2017

Choose a reason for hiding this comment

Uh oh!

oli-obkDec 4, 2017

Choose a reason for hiding this comment

Uh oh!

kennytm commented Dec 4, 2017

Uh oh!

oli-obk commented Dec 5, 2017

Uh oh!

jseyfried commented Dec 5, 2017

Uh oh!

bors commented Dec 5, 2017

Uh oh!

bors commented Dec 5, 2017

Uh oh!

bors commented Dec 6, 2017

Uh oh!

oli-obk commented Dec 6, 2017

Uh oh!

oli-obk commented Dec 6, 2017

Uh oh!

kennytm commented Dec 6, 2017

Uh oh!

oli-obk commented Dec 6, 2017

Uh oh!

kennytm commented Dec 6, 2017

Uh oh!

kennytm commented Dec 14, 2017

Uh oh!

bors commented Dec 14, 2017

Uh oh!

bors commented Dec 14, 2017

Uh oh!

bors commented Dec 14, 2017

Uh oh!

kennytm commented Dec 14, 2017

Uh oh!

bors commented Dec 14, 2017

Uh oh!

oli-obk commented Dec 14, 2017

Uh oh!

bors commented Dec 14, 2017

Uh oh!

bors commented Dec 14, 2017

Uh oh!

bors commented Dec 14, 2017

Uh oh!

nrc commented Dec 15, 2017

Uh oh!

oli-obk commented Dec 15, 2017

Uh oh!

Reviewers

Assignees

Labels