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

compilertest (UI test): Support custom normalization. #43083

Merged
merged 3 commits into from
Jul 12, 2017

Conversation

kennytm
Copy link
Member

@kennytm kennytm commented Jul 6, 2017

Closes #42434.

Adds this header for UI tests:

// normalize-stderr-32bit: "fn() (32 bits)" -> "fn() ($PTR bits)"

It will normalize the stderr output on 32-bit platforms, by replacing all instances of fn() (32 bits) by fn() ($PTR bits).

Extends the UI tests in #42304 and #41968 to 32-bit targets.

r? @nikomatsakis

@alexcrichton
Copy link
Member

Looks like there may be some travis failure?

@alexcrichton alexcrichton added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jul 6, 2017
@kennytm
Copy link
Member Author

kennytm commented Jul 6, 2017

@alexcrichton Travis is fine. dist-aarch64-linux spuriously failed because it was unable to download stamp. (Ideally Travis-PR should only run one job but that's not possible AFAIK).

@alexcrichton alexcrichton 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 Jul 6, 2017
@nikomatsakis
Copy link
Contributor

Ooh, exciting :)

Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great. Could you however update the UI README file to describe how it works?

It's possible that this file should be merge with the more general COMPILER_TESTS.md README, as well.

// ignore-emscripten
// ignore 32-bit platforms (test output is different)
// normalize-stderr-32bit: "&str (64 bits)" -> "&str ($STR bits)"
// normalize-stderr-64bit: "&str (128 bits)" -> "&str ($STR bits)"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice

@kennytm kennytm force-pushed the fix-42434-custom-stdxxx-normalization branch from 904eb24 to 7efb58c Compare July 6, 2017 18:11
@kennytm
Copy link
Member Author

kennytm commented Jul 6, 2017

@nikomatsakis The two READMEs are merged in 7efb58c.

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jul 7, 2017

📌 Commit 7efb58c has been approved by nikomatsakis

@nikomatsakis
Copy link
Contributor

@kennytm nice

@bors
Copy link
Contributor

bors commented Jul 8, 2017

⌛ Testing commit 7efb58c with merge 9ce08cbdc414ca7eb414d095ea654449443cace3...

@bors
Copy link
Contributor

bors commented Jul 8, 2017

💔 Test failed - status-travis

@kennytm
Copy link
Member Author

kennytm commented Jul 8, 2017

dist-powerpc-linux stalled during make prepare. Spurious.

Attempting with retry: make prepare
[00:08:34] downloading https://static.rust-lang.org/dist/2017-06-15/rust-std-beta-x86_64-unknown-linux-gnu.tar.gz
######################################################################## 100.0%
[00:10:11] extracting /checkout/obj/build/cache/2017-06-15/rust-std-beta-x86_64-unknown-linux-gnu.tar.gz
[00:10:11] downloading https://static.rust-lang.org/dist/2017-06-15/rustc-beta-x86_64-unknown-linux-gnu.tar.gz
######################################################################## 100.0%
[00:11:40] extracting /checkout/obj/build/cache/2017-06-15/rustc-beta-x86_64-unknown-linux-gnu.tar.gz
[00:11:40] downloading https://static.rust-lang.org/dist/2017-06-15/cargo-beta-x86_64-unknown-linux-gnu.tar.gz
######################################################################## 100.0%
[00:11:43] extracting /checkout/obj/build/cache/2017-06-15/cargo-beta-x86_64-unknown-linux-gnu.tar.gz
[00:11:43]     Updating registry `https://github.com/rust-lang/crates.io-index`
[00:17:24]     Updating git repository `https://github.com/rust-lang/cargo`

No output has been received in the last 30m0s, this potentially indicates a stalled build or something wrong with the build itself.

Check the details on how to adjust your build configuration on: https://docs.travis-ci.com/user/common-build-problems/#Build-times-out-because-no-output-was-received

The build has been terminated

@alexcrichton
Copy link
Member

@bors: retry

@bors
Copy link
Contributor

bors commented Jul 9, 2017

⌛ Testing commit 7efb58c with merge dcc7f0f...

bors added a commit that referenced this pull request Jul 9, 2017
… r=nikomatsakis

compilertest (UI test): Support custom normalization.

Closes #42434.

Adds this header for UI tests:

```rust
// normalize-stderr-32bit: "fn() (32 bits)" -> "fn() ($PTR bits)"
```

It will normalize the `stderr` output on 32-bit platforms, by replacing all instances of `fn() (32 bits)` by `fn() ($PTR bits)`.

Extends the UI tests in #42304 and #41968 to 32-bit targets.

r? @nikomatsakis
@bors
Copy link
Contributor

bors commented Jul 9, 2017

💔 Test failed - status-travis

@kennytm
Copy link
Member Author

kennytm commented Jul 9, 2017

Legit. Well transmuting a 32-bit pointer to u32 is certainly valid on 32-bit platforms 😂 Going to change the test to use [u8; 3].

EDIT: Checking this test further, turns out this line is supposed to have no errors. The original test added in #31710 reads

// No error if a coercion would otherwise occur.
mem::transmute::<fn(), usize>(main);

so the intension is transmuting to a usize, not u32. The change to u32 was introduced in #42304 (d09cf46 "Update UI tests to be platform independent") which is probably a mistake during the mass replace.

I'll change the test back to use usize instead to keep the original spirit.

@kennytm kennytm force-pushed the fix-42434-custom-stdxxx-normalization branch from 7efb58c to a0344d8 Compare July 9, 2017 12:45
@kennytm
Copy link
Member Author

kennytm commented Jul 9, 2017

@nikomatsakis Updated test. Now only one UI test left (ui/transmute/main.rs) really needs normalization.

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jul 10, 2017

📌 Commit a0344d8 has been approved by nikomatsakis

@bors
Copy link
Contributor

bors commented Jul 11, 2017

⌛ Testing commit a0344d80dd6868d829527b07f3c9503b3d442326 with merge b36ecd74762fe453df1358e75ec38b33db7373f0...

@bors
Copy link
Contributor

bors commented Jul 11, 2017

💔 Test failed - status-travis

@kennytm
Copy link
Member Author

kennytm commented Jul 11, 2017

Hmm the error message is changed? Going to rebase and update the UI test.

[01:11:38] -error[E0512]: transmute called with types of different sizes
[01:11:38] +error[E0591]: can't transmute zero-sized type
[01:11:38]    --> $DIR/transmute-from-fn-item-types-error.rs:14:13
[01:11:38]     |
[01:11:38]  14 |     let i = mem::transmute(bar);
[01:11:38]     |             ^^^^^^^^^^^^^^
[01:11:38]     |
[01:11:38] -   = note: source type: unsafe fn() {bar} (0 bits)
[01:11:38] -   = note: target type: i32 (32 bits)
[01:11:38] +   = note: source type: unsafe fn() {bar}
[01:11:38] +   = note: target type: i32
[01:11:38] +   = help: cast with `as` to a pointer instead

Edit: Looks the the error message is E0512 if we are transmuting to non-pointer-sized types, E0591 otherwise, which causes this difference. Going to change the targe type to i8 instead.

@kennytm kennytm force-pushed the fix-42434-custom-stdxxx-normalization branch from a0344d8 to 34209b0 Compare July 11, 2017 08:59
@kennytm
Copy link
Member Author

kennytm commented Jul 11, 2017

@nikomatsakis Updated test.

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jul 11, 2017

📌 Commit 34209b0 has been approved by nikomatsakis

@bors
Copy link
Contributor

bors commented Jul 11, 2017

⌛ Testing commit 34209b0 with merge b360b44...

bors added a commit that referenced this pull request Jul 11, 2017
… r=nikomatsakis

compilertest (UI test): Support custom normalization.

Closes #42434.

Adds this header for UI tests:

```rust
// normalize-stderr-32bit: "fn() (32 bits)" -> "fn() ($PTR bits)"
```

It will normalize the `stderr` output on 32-bit platforms, by replacing all instances of `fn() (32 bits)` by `fn() ($PTR bits)`.

Extends the UI tests in #42304 and #41968 to 32-bit targets.

r? @nikomatsakis
@bors
Copy link
Contributor

bors commented Jul 12, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing b360b44 to master...

@bors bors merged commit 34209b0 into rust-lang:master Jul 12, 2017
@kennytm kennytm deleted the fix-42434-custom-stdxxx-normalization branch July 20, 2017 22:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

4 participants