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

Downgrade const eval dangling ptr in final to future incompat lint #122204

Conversation

pnkfelix
Copy link
Member

@pnkfelix pnkfelix commented Mar 8, 2024

Short term band-aid for issue #121610, downgrading the prior hard error to a future-incompat lint (tracked in issue #122153).

Note we should not mark #121610 as resolved until after this (or something analogous) is beta backported.

@rustbot
Copy link
Collaborator

rustbot commented Mar 8, 2024

r? @fmease

rustbot has assigned @fmease.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot
Copy link
Collaborator

rustbot commented Mar 8, 2024

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 8, 2024
@wesleywiser
Copy link
Member

r? wesleywiser

@rustbot rustbot assigned wesleywiser and unassigned fmease Mar 8, 2024
@rust-log-analyzer

This comment has been minimized.

@pnkfelix
Copy link
Member Author

pnkfelix commented Mar 8, 2024

ugh I have to do more updating to placate our tidy+diagnostics checking

@rustbot author

@rustbot rustbot 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 Mar 8, 2024
@pnkfelix pnkfelix force-pushed the downgrade-const-eval-dnagling-ptr-in-final-to-future-incompat-lint branch from 410d2a2 to 611c62f Compare March 8, 2024 20:46
@pnkfelix
Copy link
Member Author

pnkfelix commented Mar 8, 2024

Okay hopefully the tidy and diagnostic checks are all placated now.

@rustbot reviewer

@rustbot rustbot 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 Mar 8, 2024
@rust-log-analyzer

This comment has been minimized.

const INTERIOR_MUTABLE_BEHIND_RAW: *mut i32 = &UnsafeCell::new(42) as *const _ as *mut _;
//~^ ERROR: mutable pointer in final value
//~| WARNING this was previously accepted by the compiler
Copy link
Member

Choose a reason for hiding this comment

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

It looks like we have some places where we were already generating an error but now we report the future incompatibility warning. That's perhaps a bit confusing but I don't think it needs to block the PR.

Copy link
Member

Choose a reason for hiding this comment

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

Note that this file uses -Zunleash-the-miri-inside-of-you. I am not sure if this can even be triggered on stable.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could just not emit the future incompat lint if unleash mode is on

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 the error in unleash mode, it's the only way unleash mode can be sound.

Copy link
Member Author

Choose a reason for hiding this comment

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

Could just not emit the future incompat lint if unleash mode is on

As a variation on the above, I was musing that if any future-incompat lint is set to deny or forbid, then we could stop emitting that boilerplate about the thing being potentially rejected in a future version of the compiler, just to cut down on the diagnostic flooding...

@RalfJung
Copy link
Member

RalfJung commented Mar 9, 2024

The compiler part of the diff LGTM.

@pnkfelix pnkfelix added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Mar 11, 2024
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Mar 12, 2024

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

…tests that were expecting the hard error for it.

I attempted to do this in a manner that preserved the line numbers to reduce the
review effort on the resulting diff, but we still have to deal with the
ramifications of how a future-incompat lint behaves compared to a hard-error (in
terms of its impact on the diagnostic output).
@pnkfelix pnkfelix force-pushed the downgrade-const-eval-dnagling-ptr-in-final-to-future-incompat-lint branch from 2e2fe68 to 9c33cc6 Compare March 13, 2024 14:59
Copy link
Member

@wesleywiser wesleywiser left a comment

Choose a reason for hiding this comment

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

r=me when CI is green 👍

@pnkfelix
Copy link
Member Author

Regarding the beta-nomination: This ended up requiring a bit more changes than I had anticipated when we discussed it last week.

Another option, that may be more palatable for backporting, would be to just disable the hard-error with no lint at all on beta, and let the future-incompat code ride the train.

@RalfJung
Copy link
Member

Almost all the changes are in tests. So in terms of the diffstat I don't think that would be much smaller?

@rust-log-analyzer

This comment has been minimized.

@pnkfelix
Copy link
Member Author

pnkfelix commented Mar 13, 2024

oh. of course. need to handle tests that are 32-bit specific (sigh)

(I'll admit its not immediately obvious to me why the CI test above is hitting a 32-bit target from that specific context that it listed with the command /checkout/obj/build/x86_64-unknown-linux-gnu/stage1-codegen/x86_64-unknown-linux-gnu/release/y test --use-system-gcc --use-backend gcc --out-dir /checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/cg_gcc --release --mini-tests --std-tests, but I'll figure it out.)

@wesleywiser
Copy link
Member

Almost all the changes are in tests. So in terms of the diffstat I don't think that would be much smaller?

I agree with Ralf, the functional changes are only ~60 lines with the rest being in test output and the added test case. I don't see an issue with backporting that.

@wesleywiser
Copy link
Member

Thanks for getting this done @pnkfelix!

@bors r+ p=1

Resolves P-high issue we want to backport

@bors
Copy link
Contributor

bors commented Mar 13, 2024

📌 Commit 8f45a9e has been approved by wesleywiser

It is now in the queue for this repository.

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

bors commented Mar 14, 2024

⌛ Testing commit 8f45a9e with merge c7fed9f...

@bors
Copy link
Contributor

bors commented Mar 14, 2024

☀️ Test successful - checks-actions
Approved by: wesleywiser
Pushing c7fed9f to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 14, 2024
@bors bors merged commit c7fed9f into rust-lang:master Mar 14, 2024
12 checks passed
@rustbot rustbot added this to the 1.78.0 milestone Mar 14, 2024
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (c7fed9f): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
1.5% [1.5%, 1.5%] 2
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
3.8% [3.8%, 3.8%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 3.8% [3.8%, 3.8%] 1

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
2.6% [2.1%, 3.3%] 3
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 675.071s -> 677.02s (0.29%)
Artifact size: 310.73 MiB -> 310.67 MiB (-0.02%)

@apiraino
Copy link
Contributor

Beta backport approved as per compiler team on Zulip. A backport PR will be authored by the release team at the end of the current development cycle.

@rustbot label +beta-accepted

@rustbot rustbot added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Mar 14, 2024
@cuviper cuviper mentioned this pull request Mar 15, 2024
@cuviper cuviper modified the milestones: 1.78.0, 1.77.0 Mar 15, 2024
@cuviper cuviper removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Mar 15, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 15, 2024
[beta] backports

- AST validation: Improve handling of inherent impls nested within functions and anon consts rust-lang#122004
- Downgrade const eval dangling ptr in final to future incompat lint rust-lang#122204

r? cuviper
wip-sync pushed a commit to NetBSD/pkgsrc-wip that referenced this pull request Mar 29, 2024
Pkgsrc changes:
 * Adapt checksums and patches.

Upstream chnages:

Version 1.77.0 (2024-03-21)
==========================

- [Reveal opaque types within the defining body for exhaustiveness checking.]
  (rust-lang/rust#116821)
- [Stabilize C-string literals.]
  (rust-lang/rust#117472)
- [Stabilize THIR unsafeck.]
  (rust-lang/rust#117673)
- [Add lint `static_mut_refs` to warn on references to mutable statics.]
  (rust-lang/rust#117556)
- [Support async recursive calls (as long as they have indirection).]
  (rust-lang/rust#117703)
- [Undeprecate lint `unstable_features` and make use of it in the compiler.]
  (rust-lang/rust#118639)
- [Make inductive cycles in coherence ambiguous always.]
  (rust-lang/rust#118649)
- [Get rid of type-driven traversal in const-eval interning]
  (rust-lang/rust#119044),
  only as a [future compatiblity lint]
  (rust-lang/rust#122204) for now.
- [Deny braced macro invocations in let-else.]
  (rust-lang/rust#119062)

Compiler
--------

- [Include lint `soft_unstable` in future breakage reports.]
  (rust-lang/rust#116274)
- [Make `i128` and `u128` 16-byte aligned on x86-based targets.]
  (rust-lang/rust#116672)
- [Use `--verbose` in diagnostic output.]
  (rust-lang/rust#119129)
- [Improve spacing between printed tokens.]
  (rust-lang/rust#120227)
- [Merge the `unused_tuple_struct_fields` lint into `dead_code`.]
  (rust-lang/rust#118297)
- [Error on incorrect implied bounds in well-formedness check]
  (rust-lang/rust#118553),
  with a temporary exception for Bevy.
- [Fix coverage instrumentation/reports for non-ASCII source code.]
  (rust-lang/rust#119033)
- [Fix `fn`/`const` items implied bounds and well-formedness check.]
  (rust-lang/rust#120019)
- [Promote `riscv32{im|imafc}-unknown-none-elf` targets to tier 2.]
  (rust-lang/rust#118704)
- Add several new tier 3 targets:
  - [`aarch64-unknown-illumos`]
    (rust-lang/rust#112936)
  - [`hexagon-unknown-none-elf`]
    (rust-lang/rust#117601)
  - [`riscv32imafc-esp-espidf`]
    (rust-lang/rust#119738)
  - [`riscv32im-risc0-zkvm-elf`]
    (rust-lang/rust#117958)

Refer to Rust's [platform support page][platform-support-doc]
for more information on Rust's tiered platform support.

Libraries
---------

- [Implement `From<&[T; N]>` for `Cow<[T]>`.]
  (rust-lang/rust#113489)
- [Remove special-case handling of `vec.split_off
  (0)`.](rust-lang/rust#119917)

Stabilized APIs
---------------

- [`array::each_ref`]
  (https://doc.rust-lang.org/stable/std/primitive.array.html#method.each_ref)
- [`array::each_mut`]
  (https://doc.rust-lang.org/stable/std/primitive.array.html#method.each_mut)
- [`core::net`]
  (https://doc.rust-lang.org/stable/core/net/index.html)
- [`f32::round_ties_even`]
  (https://doc.rust-lang.org/stable/std/primitive.f32.html#method.round_ties_even)
- [`f64::round_ties_even`]
  (https://doc.rust-lang.org/stable/std/primitive.f64.html#method.round_ties_even)
- [`mem::offset_of!`]
  (https://doc.rust-lang.org/stable/std/mem/macro.offset_of.html)
- [`slice::first_chunk`]
  (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.first_chunk)
- [`slice::first_chunk_mut`]
  (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.first_chunk_mut)
- [`slice::split_first_chunk`]
  (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.split_first_chunk)
- [`slice::split_first_chunk_mut`]
  (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.split_first_chunk_mut)
- [`slice::last_chunk`]
  (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.last_chunk)
- [`slice::last_chunk_mut`]
  (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.last_chunk_mut)
- [`slice::split_last_chunk`]
  (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.split_last_chunk)
- [`slice::split_last_chunk_mut`]
  (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.split_last_chunk_mut)
- [`slice::chunk_by`]
  (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.chunk_by)
- [`slice::chunk_by_mut`]
  (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.chunk_by_mut)
- [`Bound::map`]
  (https://doc.rust-lang.org/stable/std/ops/enum.Bound.html#method.map)
- [`File::create_new`]
  (https://doc.rust-lang.org/stable/std/fs/struct.File.html#method.create_new)
- [`Mutex::clear_poison`]
  (https://doc.rust-lang.org/stable/std/sync/struct.Mutex.html#method.clear_poison)
- [`RwLock::clear_poison`]
  (https://doc.rust-lang.org/stable/std/sync/struct.RwLock.html#method.clear_poison)

Cargo
-----

- [Extend the build directive syntax with `cargo::`.]
  (rust-lang/cargo#12201)
- [Stabilize metadata `id` format as `PackageIDSpec`.]
  (rust-lang/cargo#12914)
- [Pull out as `cargo-util-schemas` as a crate.]
  (rust-lang/cargo#13178)
- [Strip all debuginfo when debuginfo is not requested.]
  (rust-lang/cargo#13257)
- [Inherit jobserver from env for all kinds of runners.]
  (rust-lang/cargo#12776)
- [Deprecate rustc plugin support in cargo.]
  (rust-lang/cargo#13248)

Rustdoc
-----

- [Allows links in markdown headings.]
  (rust-lang/rust#117662)
- [Search for tuples and unit by type with `()`.]
  (rust-lang/rust#118194)
- [Clean up the source sidebar's hide button.]
  (rust-lang/rust#119066)
- [Prevent JS injection from `localStorage`.]
  (rust-lang/rust#120250)

Misc
----

- [Recommend version-sorting for all sorting in style guide.]
  (rust-lang/rust#115046)

Internal Changes
----------------

These changes do not affect any public interfaces of Rust, but they represent
significant improvements to the performance or internals of rustc and related
tools.

- [Add more weirdness to `weird-exprs.rs`.]
  (rust-lang/rust#119028)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet