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

Suggest defining type parameter when appropriate #68447

Merged
merged 1 commit into from Jan 27, 2020

Conversation

@estebank
Copy link
Contributor

estebank commented Jan 22, 2020

error[E0412]: cannot find type `T` in this scope
 --> file.rs:3:12
  |
3 | impl Trait<T> for Struct {}
  |     -      ^ not found in this scope
  |     |
  |     help: you might be missing a type parameter: `<T>`

Fix #64298.

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Jan 22, 2020

r? @varkor

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

src/librustc_resolve/late.rs Outdated Show resolved Hide resolved
src/librustc_resolve/late.rs Outdated Show resolved Hide resolved
src/librustc_resolve/late.rs Show resolved Hide resolved
src/librustc_resolve/late.rs Outdated Show resolved Hide resolved
@petrochenkov

This comment has been minimized.

Copy link
Contributor

petrochenkov commented Jan 22, 2020

@rust-highfive

This comment was marked as resolved.

Copy link
Collaborator

rust-highfive commented Jan 23, 2020

The job x86_64-gnu-llvm-7 of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
2020-01-23T03:03:13.5157595Z ========================== Starting Command Output ===========================
2020-01-23T03:03:13.5215146Z [command]/bin/bash --noprofile --norc /home/vsts/work/_temp/b81baf3f-9bd2-49ad-acbc-7e99ab6bf6ae.sh
2020-01-23T03:03:13.5215426Z 
2020-01-23T03:03:13.5218776Z ##[section]Finishing: Disable git automatic line ending conversion
2020-01-23T03:03:13.5226306Z ##[section]Starting: Checkout rust-lang/rust@refs/pull/68447/merge to s
2020-01-23T03:03:13.5228463Z Task         : Get sources
2020-01-23T03:03:13.5228560Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
2020-01-23T03:03:13.5228604Z Version      : 1.0.0
2020-01-23T03:03:13.5228646Z Author       : Microsoft
---
2020-01-23T03:03:14.5454245Z ##[command]git remote add origin https://github.com/rust-lang/rust
2020-01-23T03:03:14.5464966Z ##[command]git config gc.auto 0
2020-01-23T03:03:14.5467540Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2020-01-23T03:03:14.5469645Z ##[command]git config --get-all http.proxy
2020-01-23T03:03:14.5476236Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/68447/merge:refs/remotes/pull/68447/merge

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@estebank

This comment has been minimized.

Copy link
Contributor Author

estebank commented Jan 23, 2020

I believe all the points have been addressed.

@@ -1573,7 +1583,9 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
let def_id = this.parent_scope.module.normal_ancestor_id;
let node_id = this.r.definitions.as_local_node_id(def_id).unwrap();
let better = res.is_some();
this.r.use_injections.push(UseError { err, candidates, node_id, better });
let suggestion =
if res.is_none() { this.report_missing_type_error(path) } else { None };

This comment has been minimized.

Copy link
@petrochenkov

petrochenkov Jan 24, 2020

Contributor

Aha, it's now in smart_resolve_path, I like it much better.
This is what I though about in #68447 (comment) basically, except the reporting condition is different.
As long as it's contained inside diagnostics.rs I don't care too much about the precise conditions though.

@petrochenkov

This comment has been minimized.

Copy link
Contributor

petrochenkov commented Jan 25, 2020

#68447 (comment) still needs resolving.

src/librustc_resolve/late.rs Outdated Show resolved Hide resolved
src/librustc_resolve/late.rs Outdated Show resolved Hide resolved
src/librustc_resolve/late.rs Outdated Show resolved Hide resolved
src/librustc_resolve/late.rs Outdated Show resolved Hide resolved
src/librustc_resolve/late.rs Outdated Show resolved Hide resolved
@petrochenkov

This comment has been minimized.

Copy link
Contributor

petrochenkov commented Jan 26, 2020

r=me after removing the crate visibilities and squashing the commits.

```
error[E0412]: cannot find type `T` in this scope
 --> file.rs:3:12
  |
3 | impl Trait<T> for Struct {}
  |     -      ^ not found in this scope
  |     |
  |     help: you might be missing a type parameter: `<T>`
```

Fix #64298.
@estebank estebank force-pushed the estebank:sugg-type-param branch from 310dfa5 to 697fdc5 Jan 26, 2020
@estebank

This comment has been minimized.

Copy link
Contributor Author

estebank commented Jan 27, 2020

@bors r=petrochenkov

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Jan 27, 2020

📌 Commit 697fdc5 has been approved by petrochenkov

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Jan 27, 2020

⌛️ Testing commit 697fdc5 with merge 320ada6...

bors added a commit that referenced this pull request Jan 27, 2020
Suggest defining type parameter when appropriate

```
error[E0412]: cannot find type `T` in this scope
 --> file.rs:3:12
  |
3 | impl Trait<T> for Struct {}
  |     -      ^ not found in this scope
  |     |
  |     help: you might be missing a type parameter: `<T>`
```

Fix #64298.
@bors

This comment has been minimized.

Copy link
Contributor

bors commented Jan 27, 2020

☀️ Test successful - checks-azure
Approved by: petrochenkov
Pushing 320ada6 to master...

@bors bors added the merged-by-bors label Jan 27, 2020
@bors bors merged commit 697fdc5 into rust-lang:master Jan 27, 2020
5 checks passed
5 checks passed
homu Test successful
Details
pr Build #20200126.22 succeeded
Details
pr (Linux mingw-check) Linux mingw-check succeeded
Details
pr (Linux x86_64-gnu-llvm-7) Linux x86_64-gnu-llvm-7 succeeded
Details
pr (Linux x86_64-gnu-tools) Linux x86_64-gnu-tools succeeded
Details
@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Jan 27, 2020

📣 Toolstate changed by #68447!

Tested on commit 320ada6.
Direct link to PR: #68447

💔 rustc-guide on linux: test-pass → test-fail (cc @JohnTitor @amanjeev @spastorino @mark-i-m, @rust-lang/infra).

rust-highfive added a commit to rust-lang-nursery/rust-toolstate that referenced this pull request Jan 27, 2020
Tested on commit rust-lang/rust@320ada6.
Direct link to PR: <rust-lang/rust#68447>

💔 rustc-guide on linux: test-pass → test-fail (cc @JohnTitor @amanjeev @spastorino @mark-i-m, @rust-lang/infra).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

5 participants
You can’t perform that action at this time.