Skip to content

Use impl's def id when calculating type to specify in UFCS #104334

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Dec 16, 2022

Conversation

compiler-errors
Copy link
Member

Fixes #104327
Fixes #104328

Also addresses #102670 (comment)

@rustbot
Copy link
Collaborator

rustbot commented Nov 12, 2022

r? @estebank

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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 12, 2022
@@ -9,8 +9,8 @@ LL | let _f: base::Foo = base::HasNew::new();
|
help: use the fully-qualified path to the only available implementation
|
LL | let _f: base::Foo = base::<Foo as HasNew>::new();
| +++++++ +
LL | let _f: base::Foo = <Foo as base::HasNew>::new();
Copy link
Contributor

@jruderman jruderman Nov 12, 2022

Choose a reason for hiding this comment

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

This is an improvement, but for it to actually compile it needs a path to Foo and type params for HasNew:

let _f: base::Foo = <base::Foo as base::HasNew<_>>::new();

I can spin this off into a new issue (or two new issues) if you prefer.

Copy link
Member Author

Choose a reason for hiding this comment

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

You can make a new issue for this. Thanks for pointing it out.

Copy link
Contributor

Choose a reason for hiding this comment

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

Reported the missing path as #117623. The missing type params is probably okay, since the problem is in the original code rather than being introduced by the hint.

@estebank
Copy link
Contributor

r=me (in case you need to rebase)

@compiler-errors
Copy link
Member Author

@bors r=estebank

@bors
Copy link
Collaborator

bors commented Dec 14, 2022

📌 Commit 1225a65 has been approved by estebank

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 Dec 14, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 14, 2022
…f-id, r=estebank

Use impl's def id when calculating type to specify in UFCS

Fixes rust-lang#104327
Fixes rust-lang#104328

Also addresses rust-lang#102670 (comment)
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 14, 2022
…f-id, r=estebank

Use impl's def id when calculating type to specify in UFCS

Fixes rust-lang#104327
Fixes rust-lang#104328

Also addresses rust-lang#102670 (comment)
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 15, 2022
…f-id, r=estebank

Use impl's def id when calculating type to specify in UFCS

Fixes rust-lang#104327
Fixes rust-lang#104328

Also addresses rust-lang#102670 (comment)
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 15, 2022
…f-id, r=estebank

Use impl's def id when calculating type to specify in UFCS

Fixes rust-lang#104327
Fixes rust-lang#104328

Also addresses rust-lang#102670 (comment)
@bors
Copy link
Collaborator

bors commented Dec 15, 2022

⌛ Testing commit 1225a65 with merge 51f5fa23453106ecb83c6df6f2f3008bd8c79c85...

@bors
Copy link
Collaborator

bors commented Dec 15, 2022

💔 Test failed - checks-actions

@bors 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 Dec 15, 2022
@rust-log-analyzer
Copy link
Collaborator

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

Click to see the possible cause of the failure (guessed by this bot)
test [rustdoc] src/test/rustdoc/visibility.rs ... ok

failures:

---- [rustdoc] src/test/rustdoc/primitive/no_std.rs stdout ----
thread '[rustdoc] src/test/rustdoc/primitive/no_std.rs' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 22, kind: InvalidInput, message: "Invalid argument" }', src/tools/compiletest/src/runtest.rs:2348:34


failures:
    [rustdoc] src/test/rustdoc/primitive/no_std.rs

@compiler-errors
Copy link
Member Author

@bors retry unrelated apple tester failure

@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 Dec 16, 2022
@bors
Copy link
Collaborator

bors commented Dec 16, 2022

⌛ Testing commit 1225a65 with merge 39b2a41...

@bors
Copy link
Collaborator

bors commented Dec 16, 2022

☀️ Test successful - checks-actions
Approved by: estebank
Pushing 39b2a41 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 16, 2022
@bors bors merged commit 39b2a41 into rust-lang:master Dec 16, 2022
@rustbot rustbot added this to the 1.68.0 milestone Dec 16, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (39b2a41): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was 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)
-3.7% [-3.7%, -3.7%] 1
All ❌✅ (primary) - - 0

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)
3.0% [3.0%, 3.0%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.8% [-4.0%, -3.7%] 2
All ❌✅ (primary) 3.0% [3.0%, 3.0%] 1

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

Aaron1011 pushed a commit to Aaron1011/rust that referenced this pull request Jan 6, 2023
…id, r=estebank

Use impl's def id when calculating type to specify in UFCS

Fixes rust-lang#104327
Fixes rust-lang#104328

Also addresses rust-lang#102670 (comment)
@compiler-errors compiler-errors deleted the ufcs-sugg-wrong-def-id branch August 11, 2023 20:21
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
merged-by-bors This PR was explicitly merged by bors. 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.

ICE: DefId for Sized does not have a "type_of" ICE "compute_type_of_item: unexpected item type"
7 participants