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

Avoid using unexpected cfgs in user code #1667

Merged
merged 3 commits into from
Nov 7, 2024

Conversation

Urgau
Copy link
Member

@Urgau Urgau commented Nov 5, 2024

This PR adds:

  • the ability to declare a feature without cfg checking
  • and mark every feature with missing corresponding target feature cfgs as such

This is a prerequisites to rust-lang/rust#132577 which wants to enable reporting of the unexpected_cfgs lint in external macros and is unfortunately triggering on the macros call generated by stdarch, like is_x86_feature_detected!, ...

I have computed the list of missing target feature cfgs by comparing:

  • rg "[ ]+@feature: .*: \"(.*)\";" -r '$1' --no-filename crates/std_detect/src/detect/ | sort | uniq
  • with tests/ui/check-cfg/well-known-values.stderr1 (from the main Rust repo)

(actual.txt, referenced.txt, diff-actual-referenced.txt)

Footnotes

  1. https://github.com/rust-lang/rust/blob/e8c698bb3bdc121ac7f65919bd16d22f6567a3f1/tests/ui/check-cfg/well-known-values.stderr#L177

@rustbot
Copy link
Collaborator

rustbot commented Nov 5, 2024

r? @Amanieu

rustbot has assigned @Amanieu.
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

@Amanieu
Copy link
Member

Amanieu commented Nov 6, 2024

Could we have some way of checking, in stdarch CI, that when rustc adds support for a feature we don't forget to re-enable the flag here?

@Urgau Urgau force-pushed the check-cfg-target_features branch from aad0ff1 to 682534f Compare November 7, 2024 17:16
@Urgau
Copy link
Member Author

Urgau commented Nov 7, 2024

Could we have some way of checking, in stdarch CI, that when rustc adds support for a feature we don't forget to re-enable the flag here?

Sure, I just pushed (with the small fix) a commit that add a test function as part of the feature! macro. That automatically generated test function denies both the unexpected_cfgs, and unfulfilled_lint_expectations lint such as the for the known target feature cfgs it will trigger the unexpected_cfgs lint if it isn't marked with without cfg check: true and if it's an unexpected cfg it will trigger the lint which is expected with an #[expect] attribute.

I have successfully tested it locally with the modification from upstream.

Urgau added 3 commits November 7, 2024 20:33
This is necessary to avoid `unexpected_cfgs` warnings for unexpected/
missing target features, in user code.
Computed by diffing of:
$ rg "[ ]+@feature: .*: \"(.*)\";" -r '$1' --no-filename \
  crates/std_detect/src/detect/ | sort | uniq

With (from the main Rust repo[^1]):
$ rg "target_feature" tests/ui/check-cfg/well-known-values.stderr

[^1]: https://github.com/rust-lang/rust/blob/e8c698bb3bdc121ac7f65919bd16d22f6567a3f1/tests/ui/check-cfg/well-known-values.stderr#L177
@Urgau Urgau force-pushed the check-cfg-target_features branch from 682534f to ccff009 Compare November 7, 2024 19:33
@Amanieu Amanieu merged commit e5e00aa into rust-lang:master Nov 7, 2024
58 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants