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

Camel case warning fires on X86_64 #34633

Closed
nagisa opened this issue Jul 3, 2016 · 3 comments · Fixed by #46907
Closed

Camel case warning fires on X86_64 #34633

nagisa opened this issue Jul 3, 2016 · 3 comments · Fixed by #46907
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue.

Comments

@nagisa
Copy link
Member

nagisa commented Jul 3, 2016

I feel like the warning fires wrongly on this variant name in particular.

src/lib.rs:24:5: 24:11 warning: variant `X86_64` should have a camel case name such as `X8664`, #[warn(non_camel_case_types)] on by default
src/lib.rs:24     X86_64,
                  ^~~~~~
@Thiez
Copy link
Contributor

Thiez commented Jul 3, 2016

It looks like the warning fires whenever the name contains a _ that is not leading or trailing. Would it be reasonable to make an exception for _ if the characters directly before and after are digits?

@nagisa
Copy link
Member Author

nagisa commented Jul 3, 2016

Adding an exception for digits only will lead to to an issue later about some other non-letter case. What should be done instead, IMO, is check that the character before and after _ are letters and only warn then.

/me shrugs. Very bikeshedy thing to discuss.

@brson
Copy link
Contributor

brson commented Jul 8, 2016

I understand wanting this to just work, but it's getting uncomfortably clever. Requiring a suppression here isn't so bad.

Is this a case that comes up other than for "X86_64"?

@brson brson added A-compiler C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Jul 8, 2016
@steveklabnik steveklabnik added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-tools and removed A-compiler labels Mar 24, 2017
@alexcrichton alexcrichton added T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. and removed T-tools labels May 22, 2017
kennytm added a commit to kennytm/rust that referenced this issue Jan 4, 2018
Allow non-alphabetic underscores in camel case

Certain identifiers, such as `X86_64`, cannot currently be unambiguously represented in camel case (`X8664`, `X86_64`, `X8_664`, etc. are all transformed to the same identifier). This change relaxes the rules so that underscores are permitted between two non-alphabetic characters under `#[forbid(non_camel_case_types)]`. Fixes rust-lang#34633 and fixes rust-lang#41621.
bors added a commit that referenced this issue Jan 5, 2018
Allow non-alphabetic underscores in camel case

Certain identifiers, such as `X86_64`, cannot currently be unambiguously represented in camel case (`X8664`, `X86_64`, `X8_664`, etc. are all transformed to the same identifier). This change relaxes the rules so that underscores are permitted between two non-alphabetic characters under `#[forbid(non_camel_case_types)]`. Fixes #34633 and fixes #41621.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants