Thir unsafeck fixes #117229

matthewjasper · 2023-10-26T15:45:57Z

Recognise thread local statics in THIR unsafeck
Add suggestion for unsafe_op_in_unsafe_fn
Fix unsafe checking of let expressions

rustbot · 2023-10-26T15:46:05Z

r? @cjgillot

(rustbot has picked a reviewer for you, use r? to override)

compiler/rustc_mir_build/src/check_unsafety.rs

compiler/rustc_mir_build/src/errors.rs

compiler/rustc_mir_build/src/check_unsafety.rs

cjgillot

r=me with 3 nits

cjgillot · 2023-11-04T14:17:27Z

compiler/rustc_mir_build/src/check_unsafety.rs

+                    signature_span: tcx
+                        .hir()
+                        .span(tcx.hir().local_def_id_to_hir_id(parent_id.def_id)),


Suggested change

signature_span: tcx

.hir()

.span(tcx.hir().local_def_id_to_hir_id(parent_id.def_id)),

signature_span: tcx.def_span(parent_id.def_id),

cjgillot · 2023-11-04T14:17:54Z

compiler/rustc_mir_build/src/check_unsafety.rs

+        let parent_node: hir::Node<'_> = parent_owner.into();
+        let should_suggest = parent_node.fn_sig().map_or(false, |sig| sig.header.is_unsafe());


Nit: add a fn_sig method to hir::OwnerNode?

cjgillot · 2023-11-04T14:19:38Z

compiler/rustc_mir_build/src/check_unsafety.rs

@@ -35,6 +35,7 @@ struct UnsafetyVisitor<'a, 'tcx> {
    param_env: ParamEnv<'tcx>,
    inside_adt: bool,
    warnings: &'a mut Vec<UnusedUnsafeWarning>,
+    suggest_unsafe_block: bool,


Could you add a doc-comment on this field?

This fixes some THIR unsafety checking errors not being emitted for let expressions in these situations.

matthewjasper · 2023-11-06T17:06:33Z

@bors r=cjgillot

bors · 2023-11-06T17:06:36Z

📌 Commit 868de8e has been approved by cjgillot

It is now in the queue for this repository.

bors · 2023-11-06T18:41:26Z

⌛ Testing commit 868de8e with merge a2ffbeb...

…=cjgillot Thir unsafeck fixes - Recognise thread local statics in THIR unsafeck - Add suggestion for unsafe_op_in_unsafe_fn - Fix unsafe checking of let expressions

rust-log-analyzer · 2023-11-06T18:44:56Z

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

bors · 2023-11-06T18:44:57Z

💔 Test failed - checks-actions

matthewjasper · 2023-11-07T09:33:13Z

@bors retry

bors · 2023-11-07T10:41:13Z

⌛ Testing commit 868de8e with merge 61a3eea...

bors · 2023-11-07T12:41:39Z

☀️ Test successful - checks-actions
Approved by: cjgillot
Pushing 61a3eea to master...

rust-timer · 2023-11-07T16:38:04Z

Finished benchmarking commit (61a3eea): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

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)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-4.5%	[-6.5%, -3.5%]	3
All ❌✅ (primary)	-	-	0

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)	-	-	0
Improvements ✅ (primary)	-0.5%	[-0.5%, -0.5%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.5%	[-0.5%, -0.5%]	1

Binary size

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.0%	[0.0%, 0.0%]	8
Regressions ❌ (secondary)	0.0%	[0.0%, 0.0%]	10
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.0%	[0.0%, 0.0%]	8

Bootstrap: 663.784s -> 662.103s (-0.25%)
Artifact size: 308.95 MiB -> 308.97 MiB (0.01%)

Update Rust toolchain from nightly-2023-11-07 to nightly-2023-11-08 without any other source changes. This is an automatically generated pull request. If any of the CI checks fail, manual intervention is required. In such a case, review the changes at https://github.com/rust-lang/rust from rust-lang@189d6c7 up to rust-lang@7adc89b. The log for this commit range is: rust-lang@7adc89b69b Auto merge of rust-lang#117680 - matthiaskrgr:rollup-kgaa4ma, r=matthiaskrgr rust-lang@518fe492f1 Rollup merge of rust-lang#117675 - zmodem:vectorize_h, r=durin42 rust-lang@f6f6fd1d1a Rollup merge of rust-lang#117639 - rustbot:docs-update, r=ehuss rust-lang@f8c67704f2 Rollup merge of rust-lang#117616 - RalfJung:unstable-target-features, r=compiler-errors rust-lang@cd5b5e08fe Rollup merge of rust-lang#115485 - DaniPopes:rustdoc-macro-consts, r=jackh726,fmease rust-lang@118a2deea5 Auto merge of rust-lang#117617 - Urgau:bump-libc-0.2.150, r=Mark-Simulacrum rust-lang@84abf837b8 manually bless a wasm-only test rust-lang@752a6132e5 llvm-wrapper: Remove include of non-existant Vectorize.h rust-lang@9bd71afb90 Auto merge of rust-lang#115904 - notriddle:notriddle/preload-bold, r=GuillaumeGomez rust-lang@187d1afa9d Auto merge of rust-lang#117297 - clubby789:fn-trait-missing-paren, r=TaKO8Ki rust-lang@61a3eea804 Auto merge of rust-lang#117229 - matthewjasper:thir-unsafeck-fixes, r=cjgillot rust-lang@114f1f6838 Auto merge of rust-lang#117610 - compiler-errors:object-hmm, r=aliemjay rust-lang@504f63efb0 Auto merge of rust-lang#117418 - compiler-errors:better_error_body, r=oli-obk rust-lang@4e0fb98a5c Auto merge of rust-lang#117006 - estebank:issue-69512, r=compiler-errors rust-lang@f926031ea5 When not finding assoc fn on type, look for builder fn rust-lang@7b97a5ca84 Auto merge of rust-lang#117511 - gurry:117406-err-packed-structs, r=compiler-errors rust-lang@5a9f07cc97 Build a better MIR body when errors are encountered rust-lang@171d5587ca Don't instantiate the binder twice when assembling object candidate rust-lang@24e14dd8b4 Only check predicates for late-bound non-lifetime vars in object candidate assembly rust-lang@bf65e3bddb Update books rust-lang@868de8e76b Visit patterns in THIR let expressions rust-lang@2b59992736 Add suggestion to THIR unsafe_op_in_unsafe_fn lint rust-lang@2b2c0f9886 Allow tests with rust-rustfix and revisions rust-lang@931692fa13 Recognise thread local statics in THIR unsafeck rust-lang@b85c6835d0 warn when using an unstable feature with -Ctarget-feature rust-lang@15719a8c1d libc: bump dependency to 0.2.150 rust-lang@4b3ece475d Emit explanatory note for move errors in packed struct derives rust-lang@904aceec7d Give a better diagnostic for missing parens in Fn* bounds rust-lang@2b858b7eb8 Format macro const literals with pretty printer rust-lang@2a92981301 rustdoc: stop preloading Source Serif 4 Bold Co-authored-by: celinval <celinval@users.noreply.github.com> Co-authored-by: Zyad Hassan <88045115+zhassan-aws@users.noreply.github.com>

rustbot assigned cjgillot Oct 26, 2023

This comment has been minimized.

Sign in to view

matthewjasper force-pushed the thir-unsafeck-fixes branch from c40cba5 to 310f3de Compare October 27, 2023 09:00

bjorn3 added the -Zthir-unsafeck Unstable option: THIR unsafeck label Oct 27, 2023

matthewjasper force-pushed the thir-unsafeck-fixes branch from 310f3de to bfedefa Compare October 31, 2023 15:48

cjgillot reviewed Oct 31, 2023

View reviewed changes

compiler/rustc_mir_build/src/check_unsafety.rs Outdated Show resolved Hide resolved

matthewjasper force-pushed the thir-unsafeck-fixes branch from bfedefa to 8afbe84 Compare November 2, 2023 11:42

cjgillot reviewed Nov 4, 2023

View reviewed changes

cjgillot 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 Nov 4, 2023

matthewjasper added 4 commits November 6, 2023 16:23

Recognise thread local statics in THIR unsafeck

931692f

Allow tests with rust-rustfix and revisions

2b2c0f9

Add suggestion to THIR unsafe_op_in_unsafe_fn lint

2b59992

Visit patterns in THIR let expressions

868de8e

This fixes some THIR unsafety checking errors not being emitted for let expressions in these situations.

matthewjasper force-pushed the thir-unsafeck-fixes branch from 8afbe84 to 868de8e Compare November 6, 2023 16:23

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 6, 2023

bors removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Nov 6, 2023

bors added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 6, 2023

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 Nov 7, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 7, 2023

bors merged commit 61a3eea into rust-lang:master Nov 7, 2023
12 checks passed

rustbot added this to the 1.75.0 milestone Nov 7, 2023

matthewjasper deleted the thir-unsafeck-fixes branch November 7, 2023 13:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Thir unsafeck fixes #117229

Thir unsafeck fixes #117229

matthewjasper commented Oct 26, 2023 •

edited

Loading

rustbot commented Oct 26, 2023

This comment has been minimized.

cjgillot left a comment

cjgillot Nov 4, 2023

cjgillot Nov 4, 2023

cjgillot Nov 4, 2023

matthewjasper commented Nov 6, 2023

bors commented Nov 6, 2023

bors commented Nov 6, 2023

rust-log-analyzer commented Nov 6, 2023

bors commented Nov 6, 2023

matthewjasper commented Nov 7, 2023

bors commented Nov 7, 2023

bors commented Nov 7, 2023

rust-timer commented Nov 7, 2023

		let parent_node: hir::Node<'_> = parent_owner.into();
		let should_suggest = parent_node.fn_sig().map_or(false, \|sig\| sig.header.is_unsafe());

Thir unsafeck fixes #117229

Thir unsafeck fixes #117229

Conversation

matthewjasper commented Oct 26, 2023 • edited Loading

rustbot commented Oct 26, 2023

This comment has been minimized.

cjgillot left a comment

Choose a reason for hiding this comment

cjgillot Nov 4, 2023

Choose a reason for hiding this comment

cjgillot Nov 4, 2023

Choose a reason for hiding this comment

cjgillot Nov 4, 2023

Choose a reason for hiding this comment

matthewjasper commented Nov 6, 2023

bors commented Nov 6, 2023

bors commented Nov 6, 2023

rust-log-analyzer commented Nov 6, 2023

bors commented Nov 6, 2023

matthewjasper commented Nov 7, 2023

bors commented Nov 7, 2023

bors commented Nov 7, 2023

rust-timer commented Nov 7, 2023

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

matthewjasper commented Oct 26, 2023 •

edited

Loading