Emit error for unused target expression in glob delegations#157601
Open
aerooneqq wants to merge 4 commits into
Open
Emit error for unused target expression in glob delegations#157601aerooneqq wants to merge 4 commits into
aerooneqq wants to merge 4 commits into
Conversation
This comment has been minimized.
This comment has been minimized.
e6b6c5e to
c967907
Compare
aerooneqq
commented
Jun 8, 2026
petrochenkov
reviewed
Jun 9, 2026
Contributor
Author
|
@rustbot ready |
petrochenkov
reviewed
Jun 9, 2026
| generated_target_expr, | ||
| glob_id: { | ||
| let expn_id = self.tcx.expn_that_defined(self.owner.def_id); | ||
| if expn_id == ExpnId::root() { None } else { Some(expn_id) } |
Contributor
There was a problem hiding this comment.
Suggested change
| if expn_id == ExpnId::root() { None } else { Some(expn_id) } | |
| (delegation_source == DelegationSource::Glob).then(|| self.tcx.expn_that_defined(self.owner.def_id)) |
Right now we are still not checking that the delegation came from a glob, and not from some random other macro.
| let mut delegations_by_expansions = FxIndexMap::default(); | ||
|
|
||
| for &id in tcx.resolutions(()).delegation_infos.keys() { | ||
| if let Some(info) = tcx.hir_opt_delegation_info(id) |
Contributor
There was a problem hiding this comment.
Suggested change
| if let Some(info) = tcx.hir_opt_delegation_info(id) | |
| let info = tcx.hir_delegation_info(id); |
Can it ever fail here? We are only processing delegations.
| use rustc_span::Span; | ||
|
|
||
| pub fn check_glob_delegations_target_expr(tcx: TyCtxt<'_>) { | ||
| let mut delegations_by_expansions = FxIndexMap::default(); |
Contributor
There was a problem hiding this comment.
Suggested change
| let mut delegations_by_expansions = FxIndexMap::default(); | |
| let mut delegations_by_glob_id = FxIndexMap::default(); |
The glob id being and expansion is not a detail that is important here.
| } | ||
| } | ||
|
|
||
| for (_, state) in delegations_by_expansions { |
Contributor
There was a problem hiding this comment.
Suggested change
| for (_, state) in delegations_by_expansions { | |
| for (_, (removed_target_expr, span)) in delegations_by_expansions { |
| { | ||
| let entry = delegations_by_expansions.entry(expn_id); | ||
| let entry = entry.or_insert_with(|| (true, tcx.def_span(id))); | ||
| entry.0 &= !info.generated_target_expr; |
Contributor
There was a problem hiding this comment.
Style nit: all the 3 statements can be merged into a single one.
| pub self_ty_id: Option<HirId>, | ||
| pub propagate_self_ty: bool, | ||
| pub generated_target_expr: bool, | ||
| pub glob_id: Option<ExpnId>, |
Contributor
There was a problem hiding this comment.
Suggested change
| pub glob_id: Option<ExpnId>, | |
| pub glob_id: Option<(ExpnId, /*removed_target_expr*/ bool)>, |
The boolean flag is irrelevant for non-glob delegations.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR emits error if unuse target expression is specified for glob delegation. Second part of #156798.
Part of #118212.
r? @petrochenkov