-
Notifications
You must be signed in to change notification settings - Fork 3
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
Decision: Separator choice #1
Comments
Ugh. No great choices here. I'm currently leaning toward something new and weird to avoid ambiguities with existing code; we'll get used to the way it looks. But I'm open to being convinced otherwise, especially by people on the Cargo, compiler, and lang teams. |
@carols10cents an important point brought up by @withoutboats was that something that "converts to an underscore" would not require rustc changes, and they prefer a solution that is self contained in cargo/crates. |
From your example: [package]
name = "using-foo"
[dependencies]
"foo" = "1"
"foo/std" = { version = "1", optional = true }
[features]
# Does this enable crate "foo/std", or feature "std" of crate "foo"?
default = ["foo/std"] What features does [dependencies]
"using-foo" = { version = "1", no-default-features = true }
[features]
# Does this enable feature "foo/std" of crate "using-foo", or feature "std" of crate "foo" of "using-foo"?
default = ["using-foo/foo/std"] As you can see, this will quickly blow up and will be a bit inefficient for cargo to resolve. Recently namespace features in cargo added syntax as [package]
name = "using-foo"
[dependencies]
"foo" = "1"
"foo:std" = { version = "1", optional = true }
"foo:bar" = { version = "1", optional = true }
[features]
# This enables crate "foo:std" & feature "std" of crate "foo" & "foo:bar"
default = ["foo:std", "foo/std", "crate:foo:bar"]
baz = [] Now the [dependencies]
"using-foo" = { version = "1", no-default-features = true }
[features]
# enable feature "foo:std" of crate "using-foo"
default = ["using-foo/foo:std"]
# enable feature "std" of crate "foo" of "using-foo"
zzz = ["using-foo/foo/std"] |
@pksunkara I don't see how this will "quickly blow up" since at worst there are never more than two options. Discussions about feature syntax should go in #4 , I've proposed some fixes there. |
Ah, I see. Would we map |
We would map and that is being discussed in #2, right? I wanted to keep the context here to just the separator. |
Correct, but it's important to note if you want it to have a mapping in the first place or be in-source. |
One issue that might come up with slashes which I haven't seen mentioned elsewhere is that it's commonly used as a path separator. This could specifically make it more difficult for CLI tools to know what people are trying to do. An interesting example is |
Edit: I just saw that Manish pointed out here that colons are already being using by Cargo for features. Feel free to disregard my comment in that case. Personally, I'm fond of using I'd also like to second the concern of |
@Manishearth I believe we could unambiguously allow The crate named If And conversely, if you want to import There are plenty of disambiguators we could use as well, such as We already pervasively use |
@joshtriplett so yes, I think this can work, but I kinda disagree with this point:
There is a very valid use case for this: If the crate I don't think this is a deal breaker, but it's worth flagging. With your lang team hat, if you think this would not be a huge deal to make happen, I'm quite amenable to this solution. It's what I originally preferred, but there seemed to be a lot of push at the time (granted, this was ages ago) to avoid changing rustc. |
To be clear: it's worth flagging because the exact way we handle such an ambiguity still needs to be selected, with tooling choices that make it not confusing. Perhaps it's okay for us to not offer any in-source disambiguation path (and rely on Cargo renaming if people need it) |
Importantly, it would be very bad if rustc considered this an error. This kind of thing is already annoying in, e.g., the "conflicting From impls" coherence case where 99% of the time the conflicting impls are the exact same code. One understands why the coherence error works that way, but there's no need for us to replicate that experience here 😄 |
I don't think we should consider it an error; I think we should consider it unambiguously a crate name if declared in Cargo.toml. With my lang hat on, I don't see a reason we couldn't do this, and it seems reasonable to me. There may be an implementation reason unknown to me why this would be an issue, but I don't see a language issue with it. |
So to be clear, the issue here is that we want
(If the feature had existed when unic was being worked on, the unic crate would be empty and just exist as a namespace for the package-as-module tree. I even considered killing the package forest and replacing it with a feature forest instead, since the cost of maintaining the package forest was part of the reason work slowed to a halt. As such, I'm strongly positive for making The most name clashing possible would then be // extern crate foo { mod bar; };
// extern crate foo::bar;
mod foo { mod bar; }
// ambiguity error [E0659]
use foo::bar;
// use local mod
use self::foo::bar;
// this needs to deambiguate between (crate foo::bar) and (crate foo)::(mod bar)
use ::foo::bar; It's important to note that if the crate literally does To re(ab)use existing path syntax, we could use // (crate foo)::(mod bar)
use ::<foo>::bar;
// (crate foo::bar)
use ::<foo::bar>; If we require the path to start with a leading And to repeat, this only would cause a hard error when
TL;DR: we already do with E0659 and the module/crate ambiguity. Do the same thing for crate-module/crate ambiguity, but it should happen even less frequently. Footnotes
|
Two separate but related things that has to be brought up:
[package]
name = "foo::bar"
version = "1.0.0"
[lib]
name = "spam" And the consumer has [dependencies]
"foo::bar" = "1.0.0" then what do they get? This applies equally to It's reasonable to model setting
[dependencies.foo::bar]
package = "foo-bar"
version = "1.0.0" (This IIUC doesn't apply to
[package]
name = "evil-icu-hijack"
version = "1.0.0"
[lib]
name = "icu::ucd" This feels like it should definitely be disallowed, since the point of this RFC is that |
I think As an example: mapping My proposal would be to use almost anything but a slash based on my bad experiences with this in the past. |
I was going to propose the same too. The ambiguity of the name clash when doing
As I mentioned in my previous comment, requiring a leading |
Shouldn't crates that want to do this just be able to avoid a situation where they need the disambiguation? Keep in mind that because the owner of One could be constructed via malicious renames, but... you can already break such macros via renaming and even trigger E0659 on such macros' output. Plus, the case where this'd be problematic is rather specific: you have to have the root crate in scope via The proper solution for those proc macros is for them to have some way to properly get a If it's not a proc macro, it should almost certainly be using Footnotes
|
Ah, that's reasonable. I do like this approach. |
@Manishearth I do as well, conditional on that being easy to detect. I don't think the proposal should depend on it; if it turns out to be hard to detect, it seems perfectly fine to just always disambiguate in favor of a subcrate if one exists. |
@joshtriplett I believe what @CAD97 is saying is that this detection exists for globs already. But yes, I agree. If i get a chance I might draft language for this and update my RFC (at least as a separate "alternative" though I'd like to promote it to the main solution once we're sure we have some consensus -- let's wait some more days). I'm also happy to accept PRs doing the same! |
While it may exist for globs, that's not what I'm referencing. Specifically: //extern crate regex;
mod regex { pub use ::regex::*; }
use regex::Regex; produces
but extern crate regex;
use regex::Regex; works fine, even though the import is nominally ambiguous between The same exact handling should in theory be able to apply unchanged to |
Ah, makes sense! |
Any mechanism to refer to crates within namespaced packages on the Rust code side must not include any symbols that aren't allowed in a filename. This, due to existence of Today there are two and a half of a way to provide a crate to I wonder if it would make sense to figure out the two problems separately. For now require that crate name specified in package's manifest ( As a side note: I believe invoking |
@nagisa hmm, so my general idea for this was going to be:
Overall I consider it okay if the But I would like to see what ideas other people have here |
IMO that's not really ergonomic because it needs the end users to always provide
You are right. I just realised that you are mentioning the leading |
How does that work for cases like |
2c: This talk about Granted, Java coalesces this at runtime in a single tree, but maybe prior art could be taken from the language and approach that Java takes to explain this, to help with writing guides. Personally, while I absolutely love the use of This is not only relevant in the language used to understand and work with it, but also for explicit/implicit notation of importing such a crate. Rust so far has always erred to the side of explicitness when it comes to showing/requiring a thing, it does not do ambiguous things implicitly - at least, this is my feeling with the language so far, this feels like a breakage of that notion. I quite liked the suggestion of If not for the primary syntax, I heavily recommend it for the “explicit” syntax, to disambiguate and work with cases where both |
How would the former work with multiple crates named e.g.
I'm not other people, but I think punting the implementation of [dependencies]
serde = { version = "1", features = ["derive"] } For this approach [dependencies]
derive = { package = "serde::derive", version = "1", optional = true } and then within the crate implementation #[cfg(feature="derive")] // or whatever the RFC decides on...
pub use derive; Presume we wanted to enable an equivalent experience with users of [dependencies]
serde = "1"
"serde::derive" = "1" # rustc gets a valid crate identifier as specified in `lib.name` As long as This would be sufficient for the ecosystem to experiment with this idea and also explore alternatives, while giving the end users of these namespaced package families largely a good enough experience. The two downsides: it'd mean more complexity for package authors for the time being; and that in the scenario I should probably have written this comment on the RFC itself, but then the context would be lost... |
So I think the problem there is that for many users the "root" crate won't actually be in use: e.g. Perhaps the answer to that in your scheme is that namespaced crates must be renamed, which works i guess. |
While requiring an identifier |
Fixed in Manishearth/rfcs@1986c3d, we have switched to |
Related: #2
The current RFC lists slashes as the separator choice. They have some drawbacks
There are many other alternatives we could pick:
The text was updated successfully, but these errors were encountered: