Skip to content

Check for inline assembly in THIR unsafeck - #85259

Merged
bors merged 2 commits into
rust-lang:masterfrom
syvb:thir-unsafeck-inline-asm
May 16, 2021
Merged

Check for inline assembly in THIR unsafeck#85259
bors merged 2 commits into
rust-lang:masterfrom
syvb:thir-unsafeck-inline-asm

Conversation

@syvb

@syvbsyvb commented May 13, 2021•
edited
Loading

Copy link
Copy Markdown
Contributor

#83129 was merged recently and added a THIR unsafe checker. This adds a check for inline assembly. (and this is 2x simpler than the MIR version, which has to check for asm and llvm_asm in two separate spots!)

see also rust-lang/project-thir-unsafeck#7

@rust-highfive

rust-highfive commented May 13, 2021

Copy link
Copy Markdown
Contributor

r? @davidtwco

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

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 13, 2021
@syvbsyvb mentioned this pull request May 13, 2021
Merged
LeSeulArtichaut
LeSeulArtichaut reviewed May 13, 2021
View reviewed changes

@LeSeulArtichautLeSeulArtichaut left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

To my untrained eye this looks good.

Maybe add a thir revision to the existing UI tests for inline assembly, if any?

@LeSeulArtichautLeSeulArtichaut mentioned this pull request May 13, 2021
Closed
13 tasks
LeSeulArtichaut
LeSeulArtichaut approved these changes May 13, 2021
View reviewed changes

@LeSeulArtichautLeSeulArtichaut left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks like inline assembly being unsafe is never tested in the suite, seems weird to me. Anyway I think this could be merged. I'm not a reviewer though, so let's ask for signoff from r? @nikomatsakis

@syvb

syvb commented May 13, 2021

Copy link
Copy Markdown
ContributorAuthor

@LeSeulArtichaut yep, I couldn't find any testing of inline assembly either. I added some thirunsafeck revisions to a few assembly tests that use unsafe properly though.

@nikomatsakis

nikomatsakis commented May 13, 2021

Copy link
Copy Markdown
Contributor

@bors r+

This looks good!

@bors

bors commented May 13, 2021

Copy link
Copy Markdown
Collaborator

📌 Commit 3e6ec4eeacf92d7a9dec11a6a8506dac5d9797b3 has been approved by nikomatsakis

@borsbors 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 May 13, 2021
@bors

bors commented May 14, 2021

Copy link
Copy Markdown
Collaborator

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

@borsbors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 14, 2021
@syvb
syvbforce-pushed the thir-unsafeck-inline-asm branch from 3e6ec4e to f23d231CompareMay 14, 2021 13:23
@syvb

syvb commented May 14, 2021

Copy link
Copy Markdown
ContributorAuthor

Merge conflicts resolved.

r? @nikomatsakis

@nikomatsakis

nikomatsakis commented May 15, 2021

Copy link
Copy Markdown
Contributor

@bors r+

@bors

bors commented May 15, 2021

Copy link
Copy Markdown
Collaborator

📌 Commit f23d231 has been approved by nikomatsakis

@borsbors 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 15, 2021
@bors

bors commented May 16, 2021

Copy link
Copy Markdown
Collaborator

⌛ Testing commit f23d231 with merge 6d525d5...

@bors

bors commented May 16, 2021

Copy link
Copy Markdown
Collaborator

☀️ Test successful - checks-actions
Approved by: nikomatsakis
Pushing 6d525d5 to master...

@borsbors added the merged-by-bors This PR was explicitly merged by bors. label May 16, 2021
@bors
bors merged commit 6d525d5 into rust-lang:masterMay 16, 2021
@rustbotrustbot added this to the 1.54.0 milestone May 16, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 9, 2021
@bors
Auto merge ofrust-lang#85263- Smittyvb:thir-unsafeck-union-field, r…
240ff4c
…=oli-obk
Check for union field accesses in THIR unsafeck
see also rust-lang#85259, rust-lang#83129, rust-lang/project-thir-unsafeck#7
r? `@LeSeulArtichaut`
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nikomatsakisnikomatsakisnikomatsakis approved these changes
+1 more reviewer
@LeSeulArtichautLeSeulArtichautLeSeulArtichaut approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@nikomatsakisnikomatsakis

Labels

merged-by-borsThis PR was explicitly merged by bors.S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Milestone

1.54.0

Development

Successfully merging this pull request may close these issues.

7 participants

@syvb@rust-highfive@nikomatsakis@bors@LeSeulArtichaut@davidtwco@rustbot