Add "missing unsafe" diagnostics #4587

Nashenas88 · 2020-05-23T21:59:12Z

Addresses #190

crates/ra_hir/src/diagnostics.rs

Nashenas88 · 2020-05-23T22:02:05Z

~~Just realized I'm missing tests for the changes in ra_hir(_ty).~~

Tests added but are failing on the latest commit.

crates/ra_hir_ty/src/expr.rs

bjorn3

There is a syntax error in multiple tests

crates/ra_ide/src/syntax_highlighting/tests.rs

crates/ra_hir_ty/src/tests.rs

Nashenas88 · 2020-05-24T19:39:23Z

I also realized there was no tracking in place for unsafe blocks. I've started adding support for that by initially just creating an Expr::UnsafeBlock, but I'm not sure how to determine whether an unsafe expr is within an unsafe block or not. Would I have to change the existing iteration in unsafe_expressions to walk the expression tree rather than iterating over every expr in body (currently done with db.body(def).exprs.iter()? I've been assuming the latter doesn't need to be manually walked.

flodiebold · 2020-05-24T20:21:44Z

Expr::UnsafeBlock

That should probably just be an Expr::Unsafe with a block inside it.

Would I have to change the existing iteration in unsafe_expressions to walk the expression tree rather than iterating over every expr in body (currently done with db.body(def).exprs.iter()? I've been assuming the latter doesn't need to be manually walked.

I think we'd need a function for walking the expression tree, we don't have that currently.

Nashenas88 · 2020-05-24T20:29:04Z

@flodiebold You commented right after a figured out a working solution. Do you think what I came up with is not efficient? Should I still go ahead an work on a walking-based impl?

Nashenas88 · 2020-05-24T20:37:02Z

~~I'm not understanding the test failures:~~

thread 'syntax_highlighting::tests::test_unsafe_highlighting' panicked at 'Forgot to remove debug-print?', crates/ra_ide/src/syntax_highlighting/tests.rs:288:5

~~But that points to this line:~~

assert_eq_text!(expected_html, actual_html);

~~in the new test I added. I can't reproduce this locally.~~

Somehow the failure revealed itself after rebasing. Not sure if maybe I also needed to run a clean, but it's working now so ¯\_(ツ)_/¯

crates/ra_hir_ty/src/diagnostics.rs

crates/ra_hir_ty/src/expr.rs

crates/ra_ide/src/syntax_highlighting.rs

crates/ra_hir_def/src/body/lower.rs

matklad

Looks like I forgot to hit "submit review" yesterday :-(

crates/ra_hir_ty/src/expr.rs

crates/ra_ide/src/syntax_highlighting.rs

crates/ra_hir_def/src/body.rs

crates/ra_hir_def/src/body/lower.rs

crates/ra_hir_ty/src/expr.rs

crates/ra_hir_def/src/body/lower.rs

crates/ra_hir_ty/src/unsafe_validation.rs

crates/ra_ide/src/syntax_highlighting.rs

… to account for derefs of raw ptrs

…s passing

…afe exprs within unsafe block

…unsafe use, update tests

…nsafe exprs are within unsafe blocks

…r that case

…dation to its own file

Move unsafe_expressions to unsafe_validation.rs, replace vec tracking of child exprs with inline macro, add debug assert to ensure tracked children match walked children exactly

…gn unsafe validator

matklad · 2020-06-27T16:01:18Z

bors r+

4587: Add "missing unsafe" diagnostics r=matklad a=Nashenas88 Addresses #190 Co-authored-by: Paul Daniel Faria <Nashenas88@users.noreply.github.com>

matklad · 2020-06-27T16:02:20Z

bors d+

bors · 2020-06-27T16:02:21Z

✌️ Nashenas88 can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

bors · 2020-06-27T16:02:58Z

Canceled.

Nashenas88 · 2020-06-27T16:03:13Z

bors r+

bors · 2020-06-27T16:13:51Z

Build succeeded:

Nashenas88 commented May 23, 2020

View reviewed changes

crates/ra_hir/src/diagnostics.rs Outdated Show resolved Hide resolved

bjorn3 reviewed May 23, 2020

View reviewed changes

crates/ra_hir_ty/src/expr.rs Outdated Show resolved Hide resolved

Nashenas88 commented May 24, 2020

View reviewed changes

crates/ra_hir_ty/src/expr.rs Outdated Show resolved Hide resolved

Nashenas88 commented May 24, 2020

View reviewed changes

crates/ra_hir_ty/src/expr.rs Outdated Show resolved Hide resolved

flodiebold reviewed May 24, 2020

View reviewed changes

crates/ra_hir_ty/src/expr.rs Outdated Show resolved Hide resolved

bjorn3 suggested changes May 24, 2020

View reviewed changes

flodiebold reviewed May 24, 2020

View reviewed changes

crates/ra_hir_ty/src/tests.rs Outdated Show resolved Hide resolved

Nashenas88 mentioned this pull request May 24, 2020

Track repr(packed) and track unsafe references of fields to packed structs. #4600

Closed

Nashenas88 force-pushed the highlight-unsafe branch from 1048f90 to 85c7c01 Compare May 24, 2020 23:45

Nashenas88 changed the title ~~Add unsafe diagnostics and unsafe highlighting~~ Add missing unsafe diagnostics and unsafe highlighting May 25, 2020

Nashenas88 requested a review from flodiebold May 25, 2020 18:30

flodiebold reviewed May 26, 2020

View reviewed changes

Nashenas88 commented May 28, 2020

View reviewed changes

crates/ra_hir_def/src/body/lower.rs Outdated Show resolved Hide resolved

matklad reviewed May 28, 2020

View reviewed changes

crates/ra_hir_ty/src/expr.rs Outdated Show resolved Hide resolved

crates/ra_hir_ty/src/expr.rs Outdated Show resolved Hide resolved

crates/ra_ide/src/syntax_highlighting.rs Outdated Show resolved Hide resolved

Nashenas88 force-pushed the highlight-unsafe branch from 2764a37 to 5fdeede Compare May 28, 2020 13:37

matklad reviewed May 28, 2020

View reviewed changes

crates/ra_hir_def/src/body.rs Outdated Show resolved Hide resolved

crates/ra_hir_def/src/body/lower.rs Outdated Show resolved Hide resolved

crates/ra_hir_def/src/body/lower.rs Outdated Show resolved Hide resolved

crates/ra_hir_ty/src/expr.rs Outdated Show resolved Hide resolved

Nashenas88 force-pushed the highlight-unsafe branch from b1ef673 to 78e9d20 Compare May 30, 2020 01:44

Nashenas88 requested a review from matklad June 1, 2020 12:05

matklad reviewed Jun 2, 2020

View reviewed changes

crates/ra_hir_def/src/body/lower.rs Show resolved Hide resolved

matklad reviewed Jun 2, 2020

View reviewed changes

crates/ra_hir_ty/src/unsafe_validation.rs Outdated Show resolved Hide resolved

matklad reviewed Jun 2, 2020

View reviewed changes

crates/ra_ide/src/syntax_highlighting.rs Show resolved Hide resolved

Nashenas88 force-pushed the highlight-unsafe branch from 78e9d20 to 19ef61e Compare June 2, 2020 23:01

Nashenas88 requested a review from matklad June 2, 2020 23:04

Nashenas88 changed the title ~~Add missing unsafe diagnostics and unsafe highlighting~~ Add missing unsafe diagnostics Jun 2, 2020

Nashenas88 changed the title ~~Add missing unsafe diagnostics~~ Add "missing unsafe" diagnostics Jun 2, 2020

Nashenas88 added 17 commits June 27, 2020 10:09

Move diagnostics back into expr, add tests for diagnostics, fix logic…

daf1cac

… to account for derefs of raw ptrs

Add tests covering unsafe blocks, more attempts to get call expr test…

b358fbf

…s passing

Remove UnnecessaryUnsafe diagnostic, Fix Expr::Call unsafe analysis

499d4c4

Fix typo in test

c622551

Add missing self param to test

3df0f9c

Track unsafe blocks, don't trigger missing unsafe diagnostic when uns…

278cbf1

…afe exprs within unsafe block

Rename Expr::UnsafeBlock to Expr::Unsafe

b956988

Address review comments, have MissingUnsafe diagnostic point to each …

9ce44be

…unsafe use, update tests

Track expr parents during lowering, use parent map when checking if u…

7f2219d

…nsafe exprs are within unsafe blocks

Account for deref token in syntax highlighting of unsafe, add test fo…

6c16823

…r that case

Add HighlightTag::Operator, use it for unsafe deref. Move unsafe vali…

f678e0d

…dation to its own file

unsafe: Clean up, improve tracking, add debug_assert

2608a6f

Move unsafe_expressions to unsafe_validation.rs, replace vec tracking of child exprs with inline macro, add debug assert to ensure tracked children match walked children exactly

Fix issues caused during rebase

f78df42

Remove track_parent and parent_map, replace with simple walk in missi…

2fc92fa

…gn unsafe validator

Revert ide highlighting changes (addressing on another branch)

2ca52bb

Cleanup changes leftover from previous tracking attempt

28bb8ed

Remove unneeded code, filename from tests, fix rebasing issues

b1992b4

Nashenas88 force-pushed the highlight-unsafe branch from 8fd8f23 to b1992b4 Compare June 27, 2020 15:41

Nashenas88 added 2 commits June 27, 2020 11:55

Improve perf of finding unsafe exprs

b7e25ba

Simplify unsafe expr collection match

68a649d

bors bot added a commit that referenced this pull request Jun 27, 2020

Merge #4587

685722b

4587: Add "missing unsafe" diagnostics r=matklad a=Nashenas88 Addresses #190 Co-authored-by: Paul Daniel Faria <Nashenas88@users.noreply.github.com>

Remove html from gitignore so highlight snapshots are not ignored

9777d2c

bors bot merged commit 45fc8d5 into rust-lang:master Jun 27, 2020

Nashenas88 deleted the highlight-unsafe branch June 27, 2020 16:14

Add "missing unsafe" diagnostics #4587

Add "missing unsafe" diagnostics #4587

Uh oh!

Conversation

Nashenas88 commented May 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Nashenas88 commented May 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bjorn3 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Nashenas88 commented May 24, 2020

Uh oh!

flodiebold commented May 24, 2020

Uh oh!

Nashenas88 commented May 24, 2020

Uh oh!

Nashenas88 commented May 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

matklad left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

matklad commented Jun 27, 2020

Uh oh!

matklad commented Jun 27, 2020

Uh oh!

bors bot commented Jun 27, 2020

Uh oh!

bors bot commented Jun 27, 2020

Uh oh!

Nashenas88 commented Jun 27, 2020

Uh oh!

bors bot commented Jun 27, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Nashenas88 commented May 23, 2020 •

edited

Loading

Nashenas88 commented May 23, 2020 •

edited

Loading

Nashenas88 commented May 24, 2020 •

edited

Loading