Skip to content

Disallow forbidden usage of non-ascii identifiers. #72259

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 17, 2020
Merged

Disallow forbidden usage of non-ascii identifiers. #72259

merged 1 commit into from
May 17, 2020

Conversation

crlf0710
Copy link
Member

Part of RFC2457, this tightens allowed identifiers back to ascii only in two situations.

r? @petrochenkov

@rust-highfive
Copy link
Contributor

Some changes occurred in diagnostic error codes

cc @GuillaumeGomez

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 16, 2020
@crlf0710 crlf0710 mentioned this pull request May 16, 2020
Closed
23 tasks
@rust-highfive

This comment has been minimized.

Sign in to view
@crlf0710 crlf0710 force-pushed the ascii_only_check branch from 3675bfe to d401518 Compare May 16, 2020 10:13
@petrochenkov
Copy link
Contributor

Could you move both checks into ast_validation.rs?

@petrochenkov
Copy link
Contributor

There's one more place where identifier can be used for searching filesystem locations - crate names.
That includes both extern crate my_crate and arbitrary paths my_crate::foo.

We cannot prohibit non-ascii identifiers in arbitrary paths, so this needs to be done more precisely in the crate loader.
Could you add this to the TODO list in #55467?

@petrochenkov petrochenkov 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 May 16, 2020
@crlf0710
Copy link
Member Author

@petrochenkov I will address the implementations changes. But i don't have the permission to update the TODO list though...

@crlf0710 crlf0710 force-pushed the ascii_only_check branch from d401518 to 640e722 Compare May 16, 2020 14:26
@crlf0710
Copy link
Member Author

Updated the implementation! Ready for review again.

@crlf0710 crlf0710 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 May 16, 2020
@crlf0710 crlf0710 requested a review from petrochenkov May 16, 2020 14:29
@petrochenkov
Copy link
Contributor

r=me with the remaining comments addressed.

@petrochenkov petrochenkov 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 May 16, 2020
@crlf0710 crlf0710 force-pushed the ascii_only_check branch from e4a80fc to de24dda Compare May 16, 2020 17:31
@crlf0710 crlf0710 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 May 16, 2020
@crlf0710
Copy link
Member Author

Addressed the remaining comments, now just need to wait for CI.

@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented May 16, 2020

📌 Commit de24dda has been approved by petrochenkov

@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 May 16, 2020
@RalfJung RalfJung mentioned this pull request May 17, 2020
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request May 17, 2020
@bors
Auto merge of rust-lang#72295 - RalfJung:rollup-icmhbs7, r=RalfJung
d79f1bd 
Rollup of 3 pull requests

Successful merges:

 - rust-lang#72259 (Disallow forbidden usage of non-ascii identifiers.)
 - rust-lang#72261 (Break out early on empty span when generate_fn_span)
 - rust-lang#72291 (bootstrap: fix typo)

Failed merges:

r? @ghost
@bors bors merged commit 49b81ca into rust-lang:master May 17, 2020
@crlf0710 crlf0710 added the F-non_ascii_idents `#![feature(non_ascii_idents)]` label May 28, 2020
@doctorn doctorn mentioned this pull request Jun 19, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@petrochenkov petrochenkov petrochenkov left review comments

Assignees

@petrochenkov petrochenkov

Labels
F-non_ascii_idents `#![feature(non_ascii_idents)]` S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@crlf0710 @rust-highfive @petrochenkov @bors