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.

Sign up for GitHub

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

Jump to bottom

A way forward for pointer equality in const eval #73398

Merged
merged 7 commits into from
Jun 23, 2020
Merged

A way forward for pointer equality in const eval #73398

merged 7 commits into from
Jun 23, 2020

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Jun 16, 2020

r? @varkor on the first commit and @RalfJung on the second commit

cc #53020

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 16, 2020
@rust-highfive

This comment has been minimized.

Sign in to view
@oli-obk
Copy link
Contributor Author

oli-obk commented Jun 16, 2020 •
edited
Loading

r? @RalfJung for the miri changes and general scheme. I'll find someone to review everything else once we both can agree on a design

@rust-highfive rust-highfive assigned RalfJung and unassigned varkor Jun 16, 2020
@rust-highfive

This comment has been minimized.

Sign in to view
@rust-highfive

This comment has been minimized.

Sign in to view
@bors
Copy link
Contributor

bors commented Jun 19, 2020

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

@oli-obk
Copy link
Contributor Author

oli-obk commented Jun 21, 2020

@bors r=varkor,RalfJung,nagisa

@bors
Copy link
Contributor

bors commented Jun 21, 2020

📌 Commit e465b22 has been approved by varkor,RalfJung,nagisa

@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 Jun 21, 2020
Manishearth added a commit to Manishearth/rust that referenced this pull request Jun 21, 2020
@Manishearth
Rollup merge of rust-lang#73398 - oli-obk:const_raw_ptr_cmp, r=varkor…
5d26903 
…,RalfJung,nagisa

A way forward for pointer equality in const eval

r? @varkor on the first commit and @RalfJung on the second commit

cc rust-lang#53020
@Manishearth Manishearth mentioned this pull request Jun 21, 2020
Closed
Manishearth added a commit to Manishearth/rust that referenced this pull request Jun 22, 2020
@Manishearth
Rollup merge of rust-lang#73398 - oli-obk:const_raw_ptr_cmp, r=varkor…
4958a5b 
…,RalfJung,nagisa

A way forward for pointer equality in const eval

r? @varkor on the first commit and @RalfJung on the second commit

cc rust-lang#53020
@Manishearth Manishearth mentioned this pull request Jun 22, 2020
Closed
Manishearth added a commit to Manishearth/rust that referenced this pull request Jun 22, 2020
@Manishearth
Rollup merge of rust-lang#73398 - oli-obk:const_raw_ptr_cmp, r=varkor…
5b219f2 
…,RalfJung,nagisa

A way forward for pointer equality in const eval

r? @varkor on the first commit and @RalfJung on the second commit

cc rust-lang#53020
This was referenced Jun 22, 2020
Closed
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 23, 2020
@bors
Auto merge of rust-lang#73643 - Manishearth:rollup-68dr8fz, r=Manishe…
1557fb0 
…arth

Rollup of 9 pull requests

Successful merges:

 - rust-lang#72271 (Improve compiler error message for wrong generic parameter order)
 - rust-lang#72493 ( move leak-check to during coherence, candidate eval)
 - rust-lang#73398 (A way forward for pointer equality in const eval)
 - rust-lang#73472 (Clean up E0689 explanation)
 - rust-lang#73496 (Account for multiple impl/dyn Trait in return type when suggesting `'_`)
 - rust-lang#73515 (Add second message for LiveDrop errors)
 - rust-lang#73567 (Clarify --extern documentation.)
 - rust-lang#73572 (Fix typos in doc comments)
 - rust-lang#73590 (bootstrap: no `config.toml` exists regression)

Failed merges:

r? @ghost
@bors bors merged commit ae38698 into rust-lang:master Jun 23, 2020
@ehuss ehuss mentioned this pull request Jun 23, 2020
Merged
This was referenced Jun 24, 2020
Closed
Closed
Closed
Closed
yvt added a commit to r3-os/r3 that referenced this pull request Jun 25, 2020
@yvt
chore: update the target compiler version to nightly-2020-06-24
b096f68 
This commit includes work-arounds for the following changes in the
compiler:

- Raw pointers as const generic parameters are now forbidden.
  <rust-lang/rust#73398>

  This means the work-around for ICE caused by `&'static [_]` const
  generic parameters doesn't work anymore. An alternative work-around is
  yet to be implemented.

- Mutable references are now forbidden in all constant contexts except
  for `const fn`s.
  <rust-lang/rust#72934>

  Working around this change was as easy as mechanically replacing
  `const` with `const fn` for most cases. The only tricky case was the
  definition of `id_map` inside `build!`, which is a macro meant to be
  called inside a `const` item. The type of `id_map` isn't explicitly
  specified but is rather implied by a given configuration function, so
  this couldn't be mechanically replaced with a `const` item. To resolve
  this problem, I added an explicit type to `build!`'s signature.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Jun 25, 2020
@Dylan-DPC
Rollup merge of rust-lang#73673 - ehuss:fix-ptr-docs, r=oli-obk
da1e916 
Fix ptr doc warnings.

rust-lang#73398 added some stray backtick lines which cause warnings when the docs are built.
Manishearth added a commit to Manishearth/rust that referenced this pull request Jun 26, 2020
@Manishearth
Rollup merge of rust-lang#73673 - ehuss:fix-ptr-docs, r=oli-obk
57167a9 
Fix ptr doc warnings.

rust-lang#73398 added some stray backtick lines which cause warnings when the docs are built.
Manishearth added a commit to Manishearth/rust that referenced this pull request Jun 26, 2020
@Manishearth
Rollup merge of rust-lang#73673 - ehuss:fix-ptr-docs, r=oli-obk
d51c959 
Fix ptr doc warnings.

rust-lang#73398 added some stray backtick lines which cause warnings when the docs are built.
Manishearth added a commit to Manishearth/rust that referenced this pull request Jun 26, 2020
@Manishearth
Rollup merge of rust-lang#73673 - ehuss:fix-ptr-docs, r=oli-obk
91f8766 
Fix ptr doc warnings.

rust-lang#73398 added some stray backtick lines which cause warnings when the docs are built.
bjorn3
bjorn3 reviewed Jun 27, 2020
View reviewed changes
src/libcore/slice/mod.rs
if self.as_ptr() == other.as_ptr() {
return true;
}

// While performance would suffer if `guaranteed_eq` just returned `false`
// for all arguments, correctness and return value of this function are not affected.
#[cfg(not(bootstrap))]
Copy link
Member

Choose a reason for hiding this comment

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

Why was this changed? This function is not const.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not yet, we're working towards that. I have a local branch where I'm trying to run this in -Zunleash-the-miri-inside-of-you mode.

RalfJung
RalfJung reviewed Jun 27, 2020
View reviewed changes
src/librustc_mir/interpret/intrinsics.rs
@@ -291,6 +291,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
let offset_ptr = ptr.ptr_wrapping_signed_offset(offset_bytes, self);
self.write_scalar(offset_ptr, dest)?;
}
sym::ptr_guaranteed_eq | sym::ptr_guaranteed_ne => {
Copy link
Member

Choose a reason for hiding this comment

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

I just realized that this implementation will also be used by Miri, which is a bug. Can we somehow make it be used only during CTCE?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

doesn't miri fall back to this only if it has no own impl? So we can just give miri an impl that behaves differently.

Copy link
Member

Choose a reason for hiding this comment

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

Miri prefers the CTFE impls, to make sure we test those properly.

I am working on a PR that adds these to Miri in a way that it prefers its own impls.

Copy link
Member

Choose a reason for hiding this comment

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

Here it is: rust-lang/miri#1459

@oli-obk oli-obk deleted the const_raw_ptr_cmp branch June 27, 2020 09:41
@JohnTitor JohnTitor mentioned this pull request Jun 27, 2020
Merged
Manishearth added a commit to Manishearth/rust that referenced this pull request Jun 28, 2020
@Manishearth
Rollup merge of rust-lang#73795 - JohnTitor:tests-for-const-fn-ptrs, …
2c1b732 
…r=oli-obk

Add some `const_compare_raw_pointers`-related regression tests

Closes rust-lang#71381
Closes rust-lang#71382
Closes rust-lang#71611
Closes rust-lang#72352

r? @oli-obk, the author of rust-lang#73398
@kanru kanru mentioned this pull request Jul 20, 2020
Closed
@cuviper cuviper added this to the 1.46 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bjorn3 bjorn3 bjorn3 left review comments

@varkor varkor varkor left review comments

@Aaron1011 Aaron1011 Aaron1011 left review comments

@nagisa nagisa nagisa left review comments

@RalfJung RalfJung RalfJung approved these changes

Assignees

@RalfJung RalfJung

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.46
Development

Successfully merging this pull request may close these issues.
9 participants
@oli-obk @rust-highfive @bors @RalfJung @nagisa @Aaron1011 @varkor @bjorn3 @cuviper