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

Rename Machine memory hooks to suggest when they run #100600

Merged
merged 1 commit into from
Aug 16, 2022

Conversation

saethlin
Copy link
Member

Some of the other memory hooks start with before_ or after_ to indicate that they run before or after a certain operation. These don't, so I was a bit confused as to when they are supposed to run.

memory_read can be read two ways in English, "memory was read" or "this is a memory read" so without the prefix this was especially ambiguous.

@rustbot
Copy link
Collaborator

rustbot commented Aug 15, 2022

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Aug 15, 2022
@rust-highfive
Copy link
Collaborator

r? @davidtwco

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 15, 2022
@RalfJung
Copy link
Member

r? @RalfJung

@rust-highfive rust-highfive assigned RalfJung and unassigned davidtwco Aug 15, 2022
@RalfJung
Copy link
Member

Uh, what happened with CI?
LGTM but we should get the PR CI to run... let me try to close and reopen the PR.

@RalfJung RalfJung closed this Aug 15, 2022
@RalfJung RalfJung reopened this Aug 15, 2022
@RalfJung
Copy link
Member

r=me when CI is green
@bors delegate+

@bors
Copy link
Contributor

bors commented Aug 15, 2022

✌️ @saethlin can now approve this pull request

@saethlin
Copy link
Member Author

Hm let's see if I just amend with no changes and force-push...

@saethlin saethlin force-pushed the rename-memory-hooks branch from 1f36517 to a5cc3a0 Compare August 15, 2022 23:55
@RalfJung
Copy link
Member

It's a general CI problem currently, nothing wrong with this PR.

So I'll just trust that you did a check-build locally...
@bors r+

@bors
Copy link
Contributor

bors commented Aug 16, 2022

📌 Commit a5cc3a0 has been approved by RalfJung

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 Aug 16, 2022
@saethlin
Copy link
Member Author

Yes the PR passes x check 😅

bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 16, 2022
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#100338 (when there are 3 or more return statements in the loop)
 - rust-lang#100384 (Add support for generating unique profraw files by default when using `-C instrument-coverage`)
 - rust-lang#100460 (Update the minimum external LLVM to 13)
 - rust-lang#100567 (Add missing closing quote)
 - rust-lang#100590 (Suggest adding an array length if possible)
 - rust-lang#100600 (Rename Machine memory hooks to suggest when they run)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 88af506 into rust-lang:master Aug 16, 2022
@rustbot rustbot added this to the 1.65.0 milestone Aug 16, 2022
bors added a commit to rust-lang/miri that referenced this pull request Aug 16, 2022
@saethlin saethlin deleted the rename-memory-hooks branch September 3, 2022 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

6 participants