Implement normalize_lexically by ChrisDenton · Pull Request #134696 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation15 Commits1 Checks6 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Implements #134694
This is, I think, the most straightforward implementation I could do, which will hopefully more easily allow experimentation if we decide to change the design here.
rustbot has assigned @workingjubilee.
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
Status: Awaiting review from the assignee but also interested parties.
Relevant to the library team, which will review and decide on the PR/issue.
labels
Comment on lines 3014 to 3021
| Component::RootDir | Component::Prefix(_) => return Err(NormalizeError), |
|---|
| Component::CurDir => continue, |
| Component::ParentDir => { |
| if lexical.as_os_str().len() == root { |
| return Err(NormalizeError); |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is also a "should never happen" case, isn't it?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I remember now. As per the RFC, we need to guard against popping from the path when there's nothing to pop. So a/../.. must error which is simple enough. But the complicating factor here is that base paths like C:\ are split by components as (Prefix, RootDir) whereas in reality you almost always want to treat C:\ as one unit. So it checks if popping would go above the root by just doing a length check. This handles whatever the "root" may be, e.g. C: (without the \) or whatever.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aha.
Hmm. This wasn't supposed to get frozen on those two questions, so I'm just going to rebase this and approve it. If I think we maybe should call unreachable!() instead of returning I can just PR that.
This comment has been minimized.
ugh, the move of the tests breaks them because they relied on a struct expr
Sorry for the delay here, I've kept getting distracted by other things. I've rebased, fixed the tests and squished.
This comment has been minimized.
📌 Commit c299e29 has been approved by workingjubilee
It is now in the queue for this repository.
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
and removed S-waiting-on-author
Status: This is awaiting some action (such as code changes or more information) from the author.
labels
jhpratt added a commit to jhpratt/rust that referenced this pull request
…=workingjubilee
Implement normalize_lexically
Implements rust-lang#134694
This is, I think, the most straightforward implementation I could do, which will hopefully more easily allow experimentation if we decide to change the design here.
bors added a commit that referenced this pull request
Rollup of 9 pull requests
Successful merges:
- #134696 (Implement
normalize_lexically) - #138744 (Add methods to TCP and UDP sockets to modify hop limit (refresh of #94678))
- #140539 (Simplify
attribute_groups) - #140863 ([rustdoc] Unify type aliases rendering with other ADT)
- #140936 (Clarify WTF-8 safety docs)
- #140952 (Specify that split_ascii_whitespace uses the same definition as is_ascii_whitespace)
- #141472 (Attempt to improve the
std::fs::create_dir_alldocs related to atomicity) - #141502 (ci: move PR job x86_64-gnu-tools to codebuild)
- #141559 (const-check: stop recommending the use of rustc_allow_const_fn_unstable)
r? @ghost
@rustbot modify labels: rollup
bors added a commit that referenced this pull request
Rollup of 8 pull requests
Successful merges:
- #134696 (Implement
normalize_lexically) - #140539 (Simplify
attribute_groups) - #140863 ([rustdoc] Unify type aliases rendering with other ADT)
- #140936 (Clarify WTF-8 safety docs)
- #140952 (Specify that split_ascii_whitespace uses the same definition as is_ascii_whitespace)
- #141472 (Attempt to improve the
std::fs::create_dir_alldocs related to atomicity) - #141502 (ci: move PR job x86_64-gnu-tools to codebuild)
- #141559 (const-check: stop recommending the use of rustc_allow_const_fn_unstable)
r? @ghost
@rustbot modify labels: rollup
rust-timer added a commit that referenced this pull request
Rollup merge of #134696 - ChrisDenton:normalize-lexically, r=workingjubilee
Implement normalize_lexically
Implements #134694
This is, I think, the most straightforward implementation I could do, which will hopefully more easily allow experimentation if we decide to change the design here.
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request
…=workingjubilee
Implement normalize_lexically
Implements rust-lang#134694
This is, I think, the most straightforward implementation I could do, which will hopefully more easily allow experimentation if we decide to change the design here.
Labels
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Relevant to the library team, which will review and decide on the PR/issue.