Uh oh!
There was an error while loading. Please reload this page.
Implement conversion traits for usize/isize together with a portability lint - #37423
Implement conversion traits for usize/isize together with a portability lint#37423petrochenkov wants to merge 3 commits into
Conversation
A lint The lints can also be improved to catch some generic cases like |
@aturon Could you give some early feedback, what is the current general sentiment of libs team regarding these conversions on the scale "We want this now!" -> "We'll merge this with modifications" -> "We won't merge it at this time no matter what."? |
aturon
commented
Nov 3, 2016
Oy, this has disappeared in my inbox during Rust Belt Rust -- I'm very sorry for the delay! I'll get you feedback ASAP. |
ollie27
commented
Nov 4, 2016
Would it not make more sense for the lints to only cover one pointer size each? We could have |
petrochenkov
commented
Nov 4, 2016
@ollie27 |
nikomatsakis
commented
Nov 4, 2016
Huh, this disappeared out of my inbox too somehow. =) Very cool! I'll take a look. |
bors
commented
Nov 6, 2016
☔ The latest upstream changes (presumably #37597) made this pull request unmergeable. Please resolve the merge conflicts. |
aturon
commented
Nov 7, 2016
cc @rust-lang/libs @rust-lang/lang @petrochenkov Again, sorry for the delay from Rust Belt Rust. I've had a chance to take a look, and I'm personally very excited about this. In general, I think lints (or lint-like systems) are a great way to handle these various portability tradeoffs, and potentially also to approach things like implicit widening. This PR also closely aligns with the ongoing libs team discussion about scenarios. I'm CCing the relevant teams, but I suspect given previous discussions that people are going to be on board with something very much like this PR. I do agree with @ollie27's suggestion; I found the mention of two sized in the lints a bit confusing. |
alexcrichton
commented
Nov 7, 2016
I'd personally prefer to hold off on this until we figure out where scenarios are going. In that world this lint would actually default to a hard error and also cover cases like: fnfoo<T:Into<u32>>(t:T){/* ... */}foo(1usize);// not always portableIn general though sounds like a great case for scenarios! |
alexcrichton
commented
Nov 10, 2016
@petrochenkov do you have thoughts about the case I mentioned above where it looks like we wouldn't emit a warning? |
petrochenkov
commented
Nov 10, 2016
I mentioned it in a message above. |
@alexcrichton I'm certainly sympathetic to both these aspects. I imagine with scenarios the user code will look something like this. Only 64-bit platforms are targeted Only 16-bit and 32-bit platforms are targeted My main concern with scenarios is schedule. They look like a huge work (especially in requirements gathering department) which is very unlikely to be done by "community" and therefore done at all unless specifically planned. |
alexcrichton
commented
Nov 10, 2016
@petrochenkov ah ok, sorry I missed that! But yeah I personally prefer this to be done with scenarios. I'd prefer to not have this as a lint as those can be opted out of (and they're ignored in dependencies), whereas scenarios are intended to provide more static guarantees that can't be opted out of. It's true yeah the scenarios are still aways out, but this doesn't seem particularly too pressing to justify a second system perhaps (at least to me personally) |
alexcrichton
commented
Nov 10, 2016
@petrochenkov oh also to clarify, I think what you posted for scenario organization is roughly what I was thinking (I don't have false/true in my mind, though). I would imagine:
That way most uses of |
alexcrichton
commented
Nov 10, 2016
(either that or we'd have nothing on by default but you could opt-in as needed) |
petrochenkov
commented
Nov 11, 2016
Another problem with scenarios is that conversion impls are in libcore, so libcore needs to be recompiled on scenario changes to apply new |
sfackler
commented
Nov 11, 2016
Scenario changes would not involve recompiling standard libraries. |
Huh? I was under impression that scenarios is simply a "preprocessor" feature. How are you going to check scenarios for trait impls? It'll require deep integration with type checker. Basically, every time the knowledge that |
alexcrichton
commented
Nov 11, 2016
Ah sorry for the misunderstanding!
My personal assumption has been something like:
I was hoping that was easy enough to implement while also being 100% robust in theory. |
eddyb
commented
Nov 11, 2016
As long as you don't need to make choices inside types, then you can be clever like that with definitions, and effectively have them "feature-gated" and potentially unusable (i.e. ungating requiring rebuilding the dependency, but this should be avoided where possible). |
petrochenkov
commented
Nov 13, 2016
This is where I disagree. Lack of these statically guaranteed lossless conversions is a pressing issue, it's not good that they are blocked by a feature in hand-waving stage of development, especially that they are trivially implementable themselves. |
brson
commented
Nov 14, 2016
I'm not excited about adding more things to std that are not available on some platforms before we understand where scenarios are going. |
alexcrichton
commented
Nov 15, 2016
The libs team discussed this a bit during triage today and some conclusions were:
|
petrochenkov
commented
Nov 15, 2016
Personally, I don't need this today, I needed this 1-2 years ago when I worked with integer heavy code. |
I need this today as much as I needed it several months ago, i.e. I find Rust unpleasant to work with when non- If scenarios are coming in the near future I don't mind waiting. If they're a pie in the sky, a partial solution would be better than no solution (although I suspect type ascriptions might be required as well to make |
dhardy
commented
Nov 16, 2016
I think I agree with brson to wait until scenarios are available. I don't personally need another solution today. |
An implementation for https://internals.rust-lang.org/t/lang-team-minutes-integer-conversion-and-portability-writ-large/4170
All lossless
From/Intoconversions are implemented, but conversions not portable between 64-bit and 32-bit platforms are reported by a special warn-by-default lint. Conversions not portable between more exotic platform pairs can also be reported by two other allow-by-default lints.cc @nikomatsakis
r? @aturon