Skip to content

Conversation

Urgau
Copy link
Member

@Urgau Urgau commented Dec 13, 2023

This PR adds an exclusion for well known names from showing in suggestions of check-cfg/unexpected_cfgs.

Follow-up to #118213 and fixes #118213 (comment).

r? @petrochenkov

@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 Dec 13, 2023
@bors

This comment was marked as resolved.

@Urgau Urgau force-pushed the check-cfg-exclude-well-known-from-diag branch from 0883bfb to f23c3f4 Compare December 31, 2023 00:08
@petrochenkov
Copy link
Contributor

I really don't want to merge it like this.
The list should either be filled automatically when we are collecting built-in cfg names, or we should live with these help notes.
@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 Jan 12, 2024
@Urgau Urgau force-pushed the check-cfg-exclude-well-known-from-diag branch from f23c3f4 to 29afbbd Compare January 12, 2024 17:50
@Urgau
Copy link
Member Author

Urgau commented Jan 12, 2024

I've updated the PR to not rely anymore on a hard-coded list and instead have an FxHashSet<Symbol> being dynamically populated from CheckCfg:::fill_well_known. This should resolve your concern about the "list not being filled automatically".

@rustbot ready

@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 Jan 12, 2024
@petrochenkov
Copy link
Contributor

Much better!
@bors r+

@bors
Copy link
Collaborator

bors commented Jan 12, 2024

📌 Commit 29afbbd 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 Jan 12, 2024
@bors
Copy link
Collaborator

bors commented Jan 13, 2024

⌛ Testing commit 29afbbd with merge f1f8687...

@bors
Copy link
Collaborator

bors commented Jan 13, 2024

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing f1f8687 to master...

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (f1f8687): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

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

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

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 665.928s -> 666.315s (0.06%)
Artifact size: 308.12 MiB -> 308.15 MiB (0.01%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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.

5 participants