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

GenericPath dirname and dirname_str should be removed #10035

Closed
metajack opened this issue Oct 23, 2013 · 3 comments · Fixed by #21759
Closed

GenericPath dirname and dirname_str should be removed #10035

metajack opened this issue Oct 23, 2013 · 3 comments · Fixed by #21759

Comments

@metajack
Copy link
Contributor

GenericPath defines dirname which returns a byte vector (why?) instead of a path, and a dirname_str which returns a str. Returning a str seems inconsistent with not returning a str very specifically in Path and having to go through Display. Why doesn't dirname just return a Path? That would allow us to remove dir_path.

Alternatively we could keep dir_path and remove dirname and dirname_str.

This module still needs some love.

@emberian
Copy link
Member

dirname returns a slice because doing otherwise would require allocating for the path. Similarly for dirname_str, which can probably just be replaced by an from_utf8.

@emberian
Copy link
Member

There's a general problem of balancing allocation and convenience. AFAIK we're leaning towards "don't allocate if you don't have to". And maybe there's a usecase for that, but I'm not sure that usecase is in path handling. Then again, whoever does care about it is going to be really annoyed that simple path operations are allocating all over the place.

cc @aturon @kballard @alexcrichton

@lilyball
Copy link
Contributor

@cmr has it spot on. I wrote the current incarnation of the Path module, and I'll readily admit that most uses of paths don't really care much about allocation, but the problem is that this is such an important module (being the fundamental representation of paths for all path-related APIs) that if it doesn't allow you to avoid allocation then it will be very problematic for the small number of people that genuinely do care about path allocation.

alexcrichton added a commit to alexcrichton/rust that referenced this issue Feb 4, 2015
This PR implements [path reform](rust-lang/rfcs#474), and motivation and details for the change can be found there.

For convenience, the old path API is being kept as `old_path` for the time being. Updating after this PR is just a matter of changing imports to `old_path` (which is likely not needed, since the prelude entries still export the old path API).

This initial PR does not include additional normalization or platform-specific path extensions. These will be done in follow up commits or PRs.

[breaking-change]

Closes rust-lang#20034
Closes rust-lang#12056
Closes rust-lang#11594
Closes rust-lang#14028
Closes rust-lang#14049
Closes rust-lang#10035
bors pushed a commit to rust-lang-ci/rust that referenced this issue Jul 25, 2022
When extracting a field expression, if RA was unable to resolve the type of the
field, we would previously fall back to using "var_name" as the variable name.

Now, when the `Expr` being extracted matches a `FieldExpr`, we can use the
`NameRef`'s ident token as a fallback option.

fixes rust-lang#10035
bors added a commit to rust-lang-ci/rust that referenced this issue Jul 25, 2022
…eld-name, r=jonas-schievink

fix: Improve suggested names for extracted variables

When extracting a field expression, if RA was unable to resolve the type of the
field, we would previously fall back to using "var_name" as the variable name.

Now, when the `Expr` being extracted matches a `FieldExpr`, we can use the
`NameRef`'s ident token as a fallback option.

fixes rust-lang#10035
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 a pull request may close this issue.

3 participants