Improve safety of `LateContext::qpath_res` #81176

camsteffen · 2021-01-18T20:56:38Z

This is my first rustc code change, inspired by hacking on clippy!

The first change is to clear cached TypeckResults from LateContext when visiting a nested item. I took a hint from here.

Clippy has a qpath_res util function to avoid a possible ICE in LateContext::qpath_res. But the docs of LateContext::qpath_res promise no ICE. So this updates the LateContext method to keep its promises, and removes the util function.

Related: rust-lang/rust-clippy#4545

CC @eddyb since you've done related work
CC @flip1995 FYI

Prevents LateContext::maybe_typeck_results() from returning data in a nested item without a body. Consequently, LateContext::qpath_res is less likely to ICE when called in a nested item. Would have prevented rust-lang/rust-clippy#4545, presumably.

Actually fulfills the documented guarantees.

rust-highfive · 2021-01-18T20:56:42Z

r? @Mark-Simulacrum

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

flip1995

Clippy changes LGTM

Mark-Simulacrum · 2021-01-24T21:29:55Z

r? @oli-obk perhaps? I'm not really familiar enough with this code to be certain about what the effects of this are so can't really review.

oli-obk · 2021-01-25T09:59:20Z

@bors r+

bors · 2021-01-25T09:59:21Z

📌 Commit eaba3da has been approved by oli-obk

bors · 2021-01-25T10:00:59Z

⌛ Testing commit eaba3da with merge 1ef843b92ce45161f65092db403d4d22536ddd71...

rust-log-analyzer · 2021-01-25T10:23:59Z

The job dist-x86_64-linux failed! Check out the build log: (web) (plain)

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

    Checking crates-io v0.31.1 (/tmp/cargo/crates/crates-io)
error[E0283]: type annotations needed
   --> src/cargo/util/config/de.rs:530:63
    |
530 |                 seed.deserialize(Tuple2Deserializer(1i32, env.as_ref()))
    |                                                           |   |
    |                                                           |   |
    |                                                           |   cannot infer type for type parameter `T` declared on the trait `AsRef`
    |                                                           this method call resolves to `&T`
    |
    = note: cannot satisfy `std::string::String: AsRef<_>`
help: use the fully qualified path for the potential candidates
    |
530 |                 seed.deserialize(Tuple2Deserializer(1i32, <std::string::String as AsRef<OsStr>>::as_ref(env)))
    |                                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
530 |                 seed.deserialize(Tuple2Deserializer(1i32, <std::string::String as AsRef<std::path::Path>>::as_ref(env)))
    |                                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
530 |                 seed.deserialize(Tuple2Deserializer(1i32, <std::string::String as AsRef<str>>::as_ref(env)))
    |                                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
530 |                 seed.deserialize(Tuple2Deserializer(1i32, <std::string::String as AsRef<[u8]>>::as_ref(env)))

error: aborting due to previous error

For more information about this error, try `rustc --explain E0283`.

bors · 2021-01-25T10:26:24Z

💔 Test failed - checks-actions

m-ou-se · 2021-01-25T16:18:06Z

@bors retry

Improve safety of `LateContext::qpath_res` This is my first rustc code change, inspired by hacking on clippy! The first change is to clear cached `TypeckResults` from `LateContext` when visiting a nested item. I took a hint from [here](https://github.com/rust-lang/rust/blob/5e91c4ecc09312d8b63d250a432b0f3ef83f1df7/compiler/rustc_privacy/src/lib.rs#L1300). Clippy has a `qpath_res` util function to avoid a possible ICE in `LateContext::qpath_res`. But the docs of `LateContext::qpath_res` promise no ICE. So this updates the `LateContext` method to keep its promises, and removes the util function. Related: rust-lang/rust-clippy#4545 CC ````@eddyb```` since you've done related work CC ````@flip1995```` FYI

Improve safety of `LateContext::qpath_res` This is my first rustc code change, inspired by hacking on clippy! The first change is to clear cached `TypeckResults` from `LateContext` when visiting a nested item. I took a hint from [here](https://github.com/rust-lang/rust/blob/5e91c4ecc09312d8b63d250a432b0f3ef83f1df7/compiler/rustc_privacy/src/lib.rs#L1300). Clippy has a `qpath_res` util function to avoid a possible ICE in `LateContext::qpath_res`. But the docs of `LateContext::qpath_res` promise no ICE. So this updates the `LateContext` method to keep its promises, and removes the util function. Related: rust-lang/rust-clippy#4545 CC `````@eddyb````` since you've done related work CC `````@flip1995````` FYI

Improve safety of `LateContext::qpath_res` This is my first rustc code change, inspired by hacking on clippy! The first change is to clear cached `TypeckResults` from `LateContext` when visiting a nested item. I took a hint from [here](https://github.com/rust-lang/rust/blob/5e91c4ecc09312d8b63d250a432b0f3ef83f1df7/compiler/rustc_privacy/src/lib.rs#L1300). Clippy has a `qpath_res` util function to avoid a possible ICE in `LateContext::qpath_res`. But the docs of `LateContext::qpath_res` promise no ICE. So this updates the `LateContext` method to keep its promises, and removes the util function. Related: rust-lang/rust-clippy#4545 CC ``````@eddyb`````` since you've done related work CC ``````@flip1995`````` FYI

Improve safety of `LateContext::qpath_res` This is my first rustc code change, inspired by hacking on clippy! The first change is to clear cached `TypeckResults` from `LateContext` when visiting a nested item. I took a hint from [here](https://github.com/rust-lang/rust/blob/5e91c4ecc09312d8b63d250a432b0f3ef83f1df7/compiler/rustc_privacy/src/lib.rs#L1300). Clippy has a `qpath_res` util function to avoid a possible ICE in `LateContext::qpath_res`. But the docs of `LateContext::qpath_res` promise no ICE. So this updates the `LateContext` method to keep its promises, and removes the util function. Related: rust-lang/rust-clippy#4545 CC ```````````@eddyb``````````` since you've done related work CC ```````````@flip1995``````````` FYI

Rollup of 10 pull requests Successful merges: - rust-lang#79570 (rustc: Stabilize `-Zrun-dsymutil` as `-Csplit-debuginfo`) - rust-lang#79819 (Add `SEMICOLON_IN_EXPRESSIONS_FROM_MACROS` lint) - rust-lang#79991 (rustdoc: Render HRTB correctly for bare functions) - rust-lang#80215 (Use -target when linking binaries for Mac Catalyst) - rust-lang#81158 (Point to span of upvar making closure FnMut) - rust-lang#81176 (Improve safety of `LateContext::qpath_res`) - rust-lang#81287 (Split rustdoc JSON types into separately versioned crate) - rust-lang#81306 (Fuse inner iterator in FlattenCompat and improve related tests) - rust-lang#81333 (clean up some const error reporting around promoteds) - rust-lang#81459 (Fix rustdoc text selection for page titles) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

Improve safety of `LateContext::qpath_res` This is my first rustc code change, inspired by hacking on clippy! The first change is to clear cached `TypeckResults` from `LateContext` when visiting a nested item. I took a hint from [here](https://github.com/rust-lang/rust/blob/5e91c4ecc09312d8b63d250a432b0f3ef83f1df7/compiler/rustc_privacy/src/lib.rs#L1300). Clippy has a `qpath_res` util function to avoid a possible ICE in `LateContext::qpath_res`. But the docs of `LateContext::qpath_res` promise no ICE. So this updates the `LateContext` method to keep its promises, and removes the util function. Related: rust-lang/rust-clippy#4545 CC ````````````@eddyb```````````` since you've done related work CC ````````````@flip1995```````````` FYI

camsteffen added 3 commits January 18, 2021 13:38

Query for TypeckResults in LateContext::qpath_res

21fb586

Actually fulfills the documented guarantees.

Remove qpath_res util function

eaba3da

rust-highfive assigned Mark-Simulacrum Jan 18, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 18, 2021

flip1995 approved these changes Jan 19, 2021

View reviewed changes

rust-highfive assigned oli-obk and unassigned Mark-Simulacrum Jan 24, 2021

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 Jan 25, 2021

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 25, 2021

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 Jan 25, 2021

This was referenced Jan 25, 2021

Rollup of 11 pull requests #81390

Closed

Rollup of 11 pull requests #81392

Closed

Rollup of 12 pull requests #81394

Closed

JohnTitor mentioned this pull request Jan 26, 2021

Rollup of 11 pull requests #81406

Closed

Dylan-DPC-zz mentioned this pull request Jan 27, 2021

Rollup of 9 pull requests #81423

Closed

Dylan-DPC-zz mentioned this pull request Jan 27, 2021

Rollup of 11 pull requests #81424

Closed

This was referenced Jan 27, 2021

Rollup of 15 pull requests #81435

Closed

Rollup of 15 pull requests #81437

Closed

This was referenced Jan 27, 2021

Rollup of 19 pull requests #81449

Closed

Rollup of 18 pull requests #81450

Closed

Dylan-DPC-zz mentioned this pull request Jan 28, 2021

Rollup of 8 pull requests #81475

Closed

JohnTitor mentioned this pull request Jan 29, 2021

Rollup of 10 pull requests #81493

Merged

bors merged commit 0c5fcce into rust-lang:master Jan 29, 2021

rustbot added this to the 1.51.0 milestone Jan 29, 2021

flip1995 mentioned this pull request Jan 29, 2021

Rustup rust-lang/rust-clippy#6647

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve safety of `LateContext::qpath_res` #81176

Improve safety of `LateContext::qpath_res` #81176

camsteffen commented Jan 18, 2021 •

edited

Loading

rust-highfive commented Jan 18, 2021

flip1995 left a comment

Mark-Simulacrum commented Jan 24, 2021

oli-obk commented Jan 25, 2021

bors commented Jan 25, 2021

bors commented Jan 25, 2021

rust-log-analyzer commented Jan 25, 2021

bors commented Jan 25, 2021

m-ou-se commented Jan 25, 2021

Improve safety of LateContext::qpath_res #81176

Improve safety of LateContext::qpath_res #81176

Conversation

camsteffen commented Jan 18, 2021 • edited Loading

rust-highfive commented Jan 18, 2021

flip1995 left a comment

Choose a reason for hiding this comment

Mark-Simulacrum commented Jan 24, 2021

oli-obk commented Jan 25, 2021

bors commented Jan 25, 2021

bors commented Jan 25, 2021

rust-log-analyzer commented Jan 25, 2021

bors commented Jan 25, 2021

m-ou-se commented Jan 25, 2021

Improve safety of `LateContext::qpath_res` #81176

Improve safety of `LateContext::qpath_res` #81176

camsteffen commented Jan 18, 2021 •

edited

Loading