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

Make call suggestions more general and more accurate #101100

Merged
merged 7 commits into from
Aug 31, 2022

Conversation

compiler-errors
Copy link
Member

@compiler-errors compiler-errors commented Aug 27, 2022

Cleans up some suggestions that have to do with adding () to make typeck happy.

  1. Drive-by rename of expr_t to base_ty since it's the type of the base_expr
  2. Autoderef until we get to a callable type in suggest_fn_call.
  3. Don't erroneously suggest calling constructor when a method/field does not exist on it.
  4. Suggest calling a method receiver if its function output has a method (e.g. fn.method() => fn().method())
  5. Extend call suggestions to type parameters, fn pointers, trait objects where possible
  6. Suggest calling in operators too (fixes Suggest calling size_of when it is missing parens in a comparison #101054)
  7. Use /* {ty} */ as argument placeholder instead of just _, which is confusing and makes suggestions look less like if let syntax.

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

r? @petrochenkov

(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 27, 2022
@@ -1,16 +1,28 @@
// This test case checks the behavior of typeck::check::method::suggest::is_fn on Ty::Error.

struct Foo;
Copy link
Member Author

Choose a reason for hiding this comment

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

Had to fix this test since we no longer say "perhaps you wish to call it" on a method that doesn't exist.

@@ -6,7 +6,7 @@ enum Foo{
Tup()
}
impl Foo {
fn foo() { }
fn foo(&self) { }
Copy link
Member Author

Choose a reason for hiding this comment

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

Had to fix this test because the suggestion makes no sense without it

@rust-log-analyzer

This comment has been minimized.

self.flags
.treat_err_as_bug
.map_or(false, |c| self.err_count() + self.lint_err_count >= c.get())
self.flags.treat_err_as_bug.map_or(false, |c| {
Copy link
Member Author

@compiler-errors compiler-errors Aug 28, 2022

Choose a reason for hiding this comment

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

Well, I didn't mean to include this, but this is a fix to -Ztreat-err-as-bug which doesn't count delayed bugs correctly. I can remove it and put it into another PR if needed.

--

detailed explanation: If we emit an error A, delay a bug B, then emit error C. Then:

  1. running -Ztreat-err-as-bug=1 will make it ICE on A.
  2. running -Ztreat-err-as-bug=2 will make it ICE on B.
  3. running -Ztreat-err-as-bug=3 will skip over B and will make rust skip over C.

So there's literally no value N such that -Ztreat-err-as-bug=N will properly work to catch C. We add the delayed good path and span bugs into the count here so this works properly now.

@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Aug 30, 2022

📌 Commit 1256530 has been approved by petrochenkov

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 30, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 31, 2022
…ggestions, r=petrochenkov

Make call suggestions more general and more accurate

Cleans up some suggestions that have to do with adding `()` to make typeck happy.

1. Drive-by rename of `expr_t` to `base_ty` since it's the type of the `base_expr`
1. Autoderef until we get to a callable type in `suggest_fn_call`.
1. Don't erroneously suggest calling constructor when a method/field does not exist on it.
1. Suggest calling a method receiver if its function output has a method (e.g. `fn.method()` => `fn().method()`)
1. Extend call suggestions to type parameters, fn pointers, trait objects where possible
1. Suggest calling in operators too (fixes rust-lang#101054)
1. Use `/* {ty} */` as argument placeholder instead of just `_`, which is confusing and makes suggestions look less like `if let` syntax.
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 31, 2022
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#100970 (Allow deriving multipart suggestions)
 - rust-lang#100984 (Reinstate preloading of some dll imports)
 - rust-lang#101011 (Use getentropy when possible on all Apple platforms)
 - rust-lang#101025 (Add tier-3 support for powerpc64 and riscv64 openbsd)
 - rust-lang#101049 (Remove span fatal from ast lowering)
 - rust-lang#101100 (Make call suggestions more general and more accurate)
 - rust-lang#101171 (Fix UB from misalignment and provenance widening in `std::sys::windows`)
 - rust-lang#101185 (Tweak `WellFormedLoc`s a bit)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit b8b2f88 into rust-lang:master Aug 31, 2022
@rustbot rustbot added this to the 1.65.0 milestone Aug 31, 2022
@compiler-errors compiler-errors deleted the generalize-call-suggestions branch November 2, 2022 03:00
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.

Suggest calling size_of when it is missing parens in a comparison
6 participants