Skip to content

rustup to rustc 1.15.0-dev (ace092f56 2016-12-13) #93

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

Merged
merged 6 commits into from
Dec 15, 2016

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Dec 14, 2016

Due to always_encode_mir we can now run tests that require auxiliary builds.

@@ -27,6 +27,15 @@ fn run_pass() {
compiletest::run_tests(&config);
}

fn miri_pass(path: &str, target: &str) {
let mut config = compiletest::default_config();
config.mode = "mir-opt".parse().expect("Invalid mode");
Copy link
Contributor Author

@oli-obk oli-obk Dec 14, 2016

Choose a reason for hiding this comment

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

mir-opt is not ideal

  1. it runs the optimization passes (but I think we run miri before those are run)
  2. it generates a final binary that is then executed on the host.

The clean solution would be to add a "compile-pass" test to compiletest

But it saves us this giant mess below, which would just get bigger if we started to play with auxiliary builds.

let offset = ptr.offset as usize;
match alloc.bytes[offset..].iter().position(|&c| c == 0) {
Some(size) => {
if self.relocations(ptr, size as u64)?.count() != 0 {
Copy link
Member

Choose a reason for hiding this comment

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

I think this and the check_defined need to use size + 1 to ensure the found null byte also is defined and not part of a relocation.


"getenv" => {
{
let name = args[0].read_ptr(&self.memory)?;
Copy link
Member

Choose a reason for hiding this comment

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

s/name/name_ptr/ for clarity.

}

// unix panic code inside libstd will read the return value of this function
"pthread_rwlock_rdlock" => {
Copy link
Member

Choose a reason for hiding this comment

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

This is fine for our current experimentation, but I guess we'll need a plan for stuff like this eventually. For the real CTFE that gets put into rustc, it would probably make more sense to have cfg(const) sections that avoid needing the libc calls.

@eddyb Have you thought about this? Panicking, printing when panicking, and even Vec allocation all currently require us to hook libc calls, which might be considered too dirty...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

real ctfe can only evaluate const fn.

Also we can't do any cfg(const) if we don't want to end up compiling libstd twice (once for the target and once for const eval).

Copy link
Member

Choose a reason for hiding this comment

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

We want to make panic allowed in real CTFE in some form, though, don't we? Otherwise we can't call anything that has any code path leading to panic.

It doesn't need to be literally cfg(), but something to tell CTFE what to do instead of going into low-level code.

Copy link
Member

Choose a reason for hiding this comment

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

Allowing panicking in const fn implies that whatever functions panic!() calls (transitively) all need to be const fn unless we cut it off somewhere with a const fn-specific feature to tell CTFE to do something differently from what the native code does.

Copy link
Member

Choose a reason for hiding this comment

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

In fact, hooking libc calls is effectively just one version of "tell CTFE to do something differently", but we could perhaps do it in a more first-class, maintainable way if it was explicit in the code implementing panic.

But I'm still pretty unsure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The easiest thing would be to create an unstable attribute that libstd can use to annotate functions that CTFE should not go into this function, but instead do some magic.

@solson solson merged commit cbffb3b into rust-lang:master Dec 15, 2016
@solson
Copy link
Member

solson commented Dec 15, 2016

Oh, it seems like these changes aren't in an actual nightly release yet, just in master?

@solson
Copy link
Member

solson commented Dec 15, 2016

It looks like the latest nightly is 9 days old: https://static.rust-lang.org/dist/channel-rust-nightly-date.txt (2016-12-06 at time of writing)

@solson
Copy link
Member

solson commented Dec 15, 2016

I will revert this to keep master building for anyone new who comes along, but I'm also keeping these changes alive in a new branch: https://github.com/solson/miri/tree/always_encode_mir

@oli-obk oli-obk deleted the always_encode_mir branch December 15, 2016 09:21
github-actions bot pushed a commit that referenced this pull request Apr 18, 2025
Introduce and use specialized `//@ ignore-auxiliary` for test support files instead of using `//@ ignore-test`

### Summary

Add a semantically meaningful directive for ignoring test *auxiliary* files. This is for auxiliary files that *participate* in actual tests but should not be built by `compiletest` (i.e. these files are involved through `mod xxx;` or `include!()` or `#[path = "xxx"]`, etc.).

### Motivation

A specialized directive like `//@ ignore-auxiliary` makes it way easier to audit disabled tests via `//@ ignore-test`.
  - These support files cannot use the canonical `auxiliary/` dir because they participate in module resolution or are included, or their relative paths can be important for test intention otherwise.

Follow-up to:
- #139705
- #139783
- #139740

See also discussions in:

- [#t-compiler > Directive name for non-test aux files?](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Directive.20name.20for.20non-test.20aux.20files.3F/with/512773817)
- [#t-compiler > Handling disabled `//@ ignore-test` tests](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Handling.20disabled.20.60.2F.2F.40.20ignore-test.60.20tests/with/512005974)
- [#t-compiler/meetings > [steering] 2025-04-11 Dealing with disabled tests](https://rust-lang.zulipchat.com/#narrow/channel/238009-t-compiler.2Fmeetings/topic/.5Bsteering.5D.202025-04-11.20Dealing.20with.20disabled.20tests/with/511717981)

### Remarks on remaining unconditionally disabled tests under `tests/`

After this PR, against commit 79a272c6402, only **14** remaining test files are disabled through `//@ ignore-test`:

<details>
<summary>Remaining `//@ ignore-test` files under `tests/`</summary>

```
tests/debuginfo/drop-locations.rs
4://@ ignore-test (broken, see #128971)

tests/rustdoc/macro-document-private-duplicate.rs
1://@ ignore-test (fails spuriously, see issue #89228)

tests/rustdoc/inline_cross/assoc-const-equality.rs
3://@ ignore-test (FIXME: #125092)

tests/ui/match/issue-27021.rs
7://@ ignore-test (#54987)

tests/ui/match/issue-26996.rs
7://@ ignore-test (#54987)

tests/ui/issues/issue-49298.rs
9://@ ignore-test (#54987)

tests/ui/issues/issue-59756.rs
2://@ ignore-test (rustfix needs multiple suggestions)

tests/ui/precondition-checks/write.rs
5://@ ignore-test (unimplemented)

tests/ui/precondition-checks/read.rs
5://@ ignore-test (unimplemented)

tests/ui/precondition-checks/write_bytes.rs
5://@ ignore-test (unimplemented)

tests/ui/explicit-tail-calls/drop-order.rs
2://@ ignore-test: tail calls are not implemented in rustc_codegen_ssa yet, so this causes 🧊

tests/ui/panics/panic-short-backtrace-windows-x86_64.rs
3://@ ignore-test (#92000)

tests/ui/json/json-bom-plus-crlf-multifile-aux.rs
3://@ ignore-test Not a test. Used by other tests

tests/ui/traits/next-solver/object-soundness-requires-generalization.rs
2://@ ignore-test (see #114196)
```
</details>

Of these, most are either **unimplemented**, or **spurious**, or **known-broken**. The outstanding one is `tests/ui/json/json-bom-plus-crlf-multifile-aux.rs` which I did not want to touch in *this* PR -- that aux file has load-bearing BOM and carriage returns and byte offset matters. I think those test files that require special encoding / BOM probably are better off as `run-make` tests. See #139968 for that aux file.

### Review advice

- Best reviewed commit-by-commit.
- The directive name diverged from the most voted `//@ auxiliary` because I think that's easy to confuse with `//@ aux-{crate,dir}`.

r? compiler
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants