Skip to content

Contracts & Harnesses for unchecked_mul , unchecked_sub, unchecked_shl and unchecked_shr #96

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

Conversation

rajathkotyal
Copy link

@rajathkotyal rajathkotyal commented Oct 2, 2024

Towards : issue #59

Parent branch : c-0011-core-nums-yenyunw-unsafe-ints - Tracking PR #91

yew005 and others added 30 commits September 10, 2024 17:11
…ints' into c-0011-core-nums-junfengj-unchecked-shl
@rajathkotyal rajathkotyal changed the title Contracts & Harnesses for unchecked_mul and unchecked_shr Contracts & Harnesses for unchecked_mul and unchecked_shr Oct 2, 2024
@rajathkotyal rajathkotyal changed the title Contracts & Harnesses for unchecked_mul and unchecked_shr Contracts & Harnesses for unchecked_mul , unchecked_shl and unchecked_shr Oct 3, 2024
Copy link

@celinval celinval left a comment

Choose a reason for hiding this comment

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

Thanks!

@rajathkotyal
Copy link
Author

Hi @celinval , Could we get the workflows approved? Thanks!

@rajathkotyal rajathkotyal changed the title Contracts & Harnesses for unchecked_mul , unchecked_shl and unchecked_shr Contracts & Harnesses for unchecked_mul , unchecked_sub, unchecked_shl and unchecked_shr Oct 7, 2024
@celinval
Copy link

celinval commented Oct 7, 2024

@rajathkotyal we need a second reviewer to approve this PR. Maybe @carolynzech?

Copy link

@carolynzech carolynzech left a comment

Choose a reason for hiding this comment

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

Nice macros!

@carolynzech carolynzech enabled auto-merge (squash) October 7, 2024 18:08
@carolynzech carolynzech merged commit 0de4670 into model-checking:main Oct 7, 2024
7 checks passed
tautschnig pushed a commit to tautschnig/verify-rust-std that referenced this pull request Apr 29, 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:
- rust-lang#139705
- rust-lang#139783
- rust-lang#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 &model-checking#96;//@ ignore-test&model-checking#96; 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 > &model-checking#91;steering&model-checking#93; 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 79a272c, 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 rust-lang#128971)

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

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

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

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

tests/ui/issues/issue-49298.rs
9://@ ignore-test (rust-lang#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 (rust-lang#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 rust-lang#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 rust-lang#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.

6 participants