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

Make gccrs recognize negative_impls #3002

Merged
merged 1 commit into from
May 24, 2024

Conversation

badumbatish
Copy link
Contributor

@badumbatish badumbatish commented May 15, 2024

just a draft pr for #2962

@badumbatish
Copy link
Contributor Author

badumbatish commented May 15, 2024

I think i got gccrs to recognize the feature negative_impl but I'm not sure how to make it so that when user do sth like this

trait ExampleTrait {}

impl !ExampleTrait for i32 {}

it fails to compile instead of continuing with compilation because we haven't implemented them or if the user didn't do #![feature(negative_impls)] in the beginning of the .rs file

@P-E-P any pointers?

@P-E-P
Copy link
Member

P-E-P commented May 16, 2024

it fails to compile instead of continuing with compilation because we haven't implemented them or if the user didn't do #![feature(negative_impls)] in the beginning of the .rs file

@badumbatish You are in the right direction! There is a visitor in gcc/rust/checks/error/rust-feature-gate.cc which match the AST against available features. There is a member as_exclam in TraitImpl, if you meet one in the visitor, you need to error out and that's it.

Do not forget to add some tests! At least one with the feature enabled and one without it.

@badumbatish
Copy link
Contributor Author

hmm i think i might be cooking something

@badumbatish badumbatish force-pushed the gating_neg_impl branch 2 times, most recently from a7940b2 to 12535b7 Compare May 17, 2024 03:35
@badumbatish
Copy link
Contributor Author

badumbatish commented May 17, 2024

hmm i got down the case where if a user attempts to do impl !ExampleTrait for i32 {} we error out with // { dg-error "negative_impls are not yet implemented" } but I don't think the rustc compiler is doing this https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=b63a98a5146d438ded89b9dac762be48

Also with the case that we did use #![feature(negative_impls)], we should also error out sth? https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=1f3c2bd54f30a5931c02a2a4abcac08b

I'm also not sure how this is relate to core mentioned in #2962, there seems to not have an enum state with "nightly" or sth for me to use on this

@CohenArthur
Copy link
Member

hmm i got down the case where if a user attempts to do impl !ExampleTrait for i32 {} we error out with // { dg-error "negative_impls are not yet implemented" } but I don't think the rustc compiler is doing this https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=b63a98a5146d438ded89b9dac762be48

the error is not exactly the same but that's okay - it erros out and that's what matters :D

Also with the case that we did use #![feature(negative_impls)], we should also error out sth? https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=1f3c2bd54f30a5931c02a2a4abcac08b

we don't have a nightly and stable versions of the compiler at the moment, so you can just assume that everything is "nightly". so we handle having #![feature] at all times, and you don't need to do any particular things regarding nightly vs stable. so the right behavior is to not error out if the feature gate was enabled, like rustc does when run in nightly

I'm also not sure how this is relate to core mentioned in #2962, there seems to not have an enum state with "nightly" or sth for me to use on this

Copy link
Member

@CohenArthur CohenArthur left a comment

Choose a reason for hiding this comment

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

nice job!

gcc/testsuite/rust/compile/negative_impls.rs Outdated Show resolved Hide resolved
gcc/testsuite/rust/compile/negative_impls_2.rs Outdated Show resolved Hide resolved
gcc/rust/checks/errors/rust-feature-gate.cc Outdated Show resolved Hide resolved
gcc/rust/checks/errors/rust-feature.cc Outdated Show resolved Hide resolved
@badumbatish
Copy link
Contributor Author

Ill squash everything down to 1 commit after this rebase from master passes

@P-E-P
Copy link
Member

P-E-P commented May 21, 2024

@badumbatish Could you rewrite your commit message please ?

gcc/rust/ChangeLog:

	* checks/errors/rust-feature-gate.cc (FeatureGate::visit): make
	gccrs recognize negative_impls
	* checks/errors/rust-feature-gate.h: likewise.
	* checks/errors/rust-feature.cc (Feature::create): likewise.
	* checks/errors/rust-feature.h: likewise.

gcc/testsuite/ChangeLog:

	* rust/compile/negative_impls.rs: New test.
	* rust/compile/negative_impls_2.rs: New test.
Copy link
Member

@CohenArthur CohenArthur left a comment

Choose a reason for hiding this comment

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

looks great! well done!

@CohenArthur CohenArthur added this pull request to the merge queue May 24, 2024
Merged via the queue into Rust-GCC:master with commit 84666c6 May 24, 2024
9 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.

None yet

3 participants