Skip to content

Restrict EII declarations to functions at lowering time#152384

Open
enthropy7 wants to merge 1 commit intorust-lang:mainfrom
enthropy7:fix-eii-root-resolution-clean
Open

Restrict EII declarations to functions at lowering time#152384
enthropy7 wants to merge 1 commit intorust-lang:mainfrom
enthropy7:fix-eii-root-resolution-clean

Conversation

@enthropy7
Copy link
Copy Markdown
Contributor

@enthropy7 enthropy7 commented Feb 9, 2026

We tighten EII declaration resolution so that lower_path_simple_eii only accepts function‑like items (Fn, AssocFn, Ctor(_, Fn)) as valid EII targets. If name resolution points at something else (like a const with the same name), we now emit a direct error (“externally implementable items must refer to a function”) at the declaration site, which prevents bad DefIds from ever reaching compare_eii_function_types and turning into an ICE

this is more robust and root-cause oriented fix to #152337 issue and an alternate of my more simple PR #152365

test included

@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 Feb 9, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Feb 9, 2026

r? @petrochenkov

rustbot has assigned @petrochenkov.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler
  • compiler expanded to 21 candidates
  • Random selection from 13 candidates

@rust-log-analyzer

This comment has been minimized.

@enthropy7 enthropy7 force-pushed the fix-eii-root-resolution-clean branch from 0ba99f1 to d2f08a3 Compare February 9, 2026 14:27
@enthropy7
Copy link
Copy Markdown
Contributor Author

previously I tightened lower_path_simple_eii for all EII paths, which also affected macro targets (eii_macro_path) and broke a number of existing EII tests. this revision restores lower_path_simple_eii to its original behavior and moves the function‑kind check into a dedicated lower_eii_decl_target used only for EII declarations, so we still reject non‑function declarations without changing the behavior of macro-based EII implementations and existing tests.

@petrochenkov
Copy link
Copy Markdown
Contributor

This looks marginally better than #152365 to me.
Although the right place for reporting errors like "expected definition X, found definition Y" is rustc_resolve, see e.g. PathSource::is_expected.
@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 9, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Feb 9, 2026

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@jdonszelmann
Copy link
Copy Markdown
Contributor

r? me I'll look at this tomorrow morning but don't merge anything similar to this pr until I did

@rustbot rustbot assigned jdonszelmann and unassigned petrochenkov Feb 9, 2026
@jdonszelmann
Copy link
Copy Markdown
Contributor

I agree with petrochenkov, this has to be solved using PathSource @enthropy7

@rustbot author

@enthropy7 enthropy7 force-pushed the fix-eii-root-resolution-clean branch from d2f08a3 to 2f52ca9 Compare March 5, 2026 13:11
@enthropy7
Copy link
Copy Markdown
Contributor Author

Moved EII declaration target validation from ast_lowering to resolve via PathSource (ExternItemImpl), so diagnostics now come from resolver

also updated tests

@enthropy7 enthropy7 force-pushed the fix-eii-root-resolution-clean branch from 2f52ca9 to 6e6d79e Compare April 7, 2026 14:59
@rustbot

This comment has been minimized.

@enthropy7 enthropy7 force-pushed the fix-eii-root-resolution-clean branch from 6e6d79e to 76d908b Compare April 7, 2026 15:01
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 7, 2026

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@enthropy7
Copy link
Copy Markdown
Contributor Author

enthropy7 commented Apr 7, 2026

@rustbot ready

rebased onto latest main. updated ExternItemImpl::is_expected to also accept Ctor(..) to preserve clean E0428 diagnostics for constructor name collisions.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 7, 2026
@rust-log-analyzer

This comment has been minimized.

@enthropy7 enthropy7 force-pushed the fix-eii-root-resolution-clean branch from 76d908b to f939ce3 Compare April 7, 2026 18:20
@rust-log-analyzer

This comment has been minimized.

@enthropy7 enthropy7 force-pushed the fix-eii-root-resolution-clean branch from f939ce3 to 5961ba1 Compare April 7, 2026 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants