MIR: support user-given type annotations on fns, structs, and enums #53225
Conversation
This file contains 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
rust-highfive
added
the
S-waiting-on-review
27 tasks
nikomatsakis
force-pushed
the
nll-type-annot
branch
from
2c90a18
to
3575f96
Compare
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
nikomatsakis
force-pushed
the
nll-type-annot
branch
from
e36ff25
to
d4f5d1a
Compare
|
Hmm, it occurs to me that this code might not be handling normalizations very well. I'm inclined to land it anyway and open an issue for follow-up. It'll require some tweaks to the subtyping code that NLL is using to get right, and I'd sort of like to see if we can get away without it and instead push all the way to lazy normalizaton =) |
pnkfelix
reviewed
Aug 12, 2018
src/librustc/ty/context.rs
Outdated
| @@ -371,6 +371,18 @@ pub struct TypeckTables<'tcx> { | |||
| /// other items. | |||
| node_substs: ItemLocalMap<&'tcx Substs<'tcx>>, | |||
|
|
|||
| /// Stores the substitutions that the user explicit gave (if any) | |||
pnkfelix
reviewed
Aug 12, 2018
src/libsyntax/feature_gate.rs
Outdated
| @@ -858,6 +858,12 @@ pub const BUILTIN_ATTRIBUTES: &'static [(&'static str, AttributeType, AttributeG | |||
| is just used for rustc unit tests \ | |||
| and will never be stable", | |||
| cfg_fn!(rustc_attrs))), | |||
| ("rustc_dump_user_substs", Whitelisted, Gated(Stability::Unstable, | |||
| "rustc_attrs", | |||
| "the `#[rustc_error]` attribute \ | |||
There was a problem hiding this comment.
you should be saying #[rustc_dump_user_substs] here, not #[rustc_error].
pnkfelix
reviewed
Aug 12, 2018
| #![feature(nll)] | ||
| #![feature(rustc_attrs)] | ||
|
|
||
| trait Bazoom<T> { |
There was a problem hiding this comment.
You might want to just comment here that the names T and U are significant in the sense that the comments for each test reference them to identify how the user-written type is yielding the particular components of the user substs that is printed by the test.
(or at least, when I first started reading the comments, I asked "wait, what is T? And U???" Because I hadn't even registered the names in my head when I read the trait and impl definitions here.
pnkfelix
changed the title
pnkfelix
reviewed
Aug 12, 2018
| @@ -344,7 +344,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { | |||
| func: Operand::Constant(box Constant { | |||
| span: test.span, | |||
| ty: mty, | |||
| literal: method | |||
| user_ty: None, // FIXME | |||
There was a problem hiding this comment.
can you elaborate (perhaps in this FIXME comment, or perhaps just in a response to me here) on which kind of input expressions you expect need to reflect a user-type annotation? If I understand this part of the source correctly, this is where we are generating code to compare a pattern literal to the correspond part of the match input; does Rust currently have a way to express a user-written type annotation on such a pattern literal? Or are you just anticipating rust-lang/rfcs#354 or rust-lang/rfcs#2522 ?
There was a problem hiding this comment.
TBH I don't know, but it was a constant that derived from user input. Can we use e.g. associated constants in match? If so, then this might apply, since I guess you could do something like <T as Trait<'a>>::BAR (also, I'm not entirely how we should handle that, either. But that's for the next PR. =)
pnkfelix
reviewed
Aug 12, 2018
| let a = 22; | ||
| let b = 44; | ||
| let c = 66; | ||
| <_ as Bazoom<_>>::method::<&'static u32>(&a, b, &c); //~ ERROR |
There was a problem hiding this comment.
hmm. For some reason, the use of the acronum "UFCS" on this test name made me think that it was going to (at least) be testing instances where the user type annotation was being injected at various points in the <_ as Bazoom<_>>:: part of the code you show here. (Isn't method::<&'static u32> and such covered pretty well elsewhere? Not that you shouldn't test it; just surprised it got priority.)
There was a problem hiding this comment.
ah never mind, your followup commits address that there are three different positions to cover in the UFCS form <One<'a> as Two<'b>>::method::<Three<'c>>(...) (and it looks like you both rename this test to reflect that numbering and add the cases for positions One and Two).
|
r=me once you address minor points above and resolve the merge conflict |
pnkfelix
added
S-waiting-on-author
nikomatsakis
force-pushed
the
nll-type-annot
branch
from
d4f5d1a
to
88faf5f
Compare
|
@bors r=pnkfelix |
|
📌 Commit 88faf5fff5e45526bbcf2c00607f6cb1605983cb has been approved by |
bors
added
S-waiting-on-bors
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
⌛ Testing commit 88faf5fff5e45526bbcf2c00607f6cb1605983cb with merge 161b7f51df6c198ad83a8704f3c1ccaf16660861... |
|
💔 Test failed - status-travis |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
bors
added
S-waiting-on-review
|
Uh... Oh, maybe due to the previous failure and a cancellation or something. |
|
@nikomatsakis I think this is travis-ci/travis-ci#4924, I've seen @kennytm make PRs that linked to this in the past. |
bors
added
S-waiting-on-bors
|
The UI test failure is likely legit though. Could you reproduce when merged with master? ---- [ui] ui/nll/user-annotations/dump-fn-method.rs stdout ----
diff of stderr:
1 error: user substs: Canonical { variables: [], value: [u32] }
- --> $DIR/dump-fn-method.rs:35:13
+ --> $DIR/dump-fn-method.rs:36:13
3 |
4 LL | let x = foo::<u32>; //~ ERROR [u32]
5 | ^^^^^^^^^^
6
7 error: user substs: Canonical { variables: [CanonicalVarInfo { kind: Ty(General) }, CanonicalVarInfo { kind: Ty(General) }], value: [?0, u32, ?1] }
- --> $DIR/dump-fn-method.rs:39:13
+ --> $DIR/dump-fn-method.rs:42:13
9 |
10 LL | let x = <_ as Bazoom<u32>>::method::<_>; //~ ERROR [?0, u32, ?1]
11 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
12
13 error: user substs: Canonical { variables: [], value: [u8, u16, u32] }
- --> $DIR/dump-fn-method.rs:43:13
+ --> $DIR/dump-fn-method.rs:46:13
15 |
16 LL | let x = <u8 as Bazoom<u16>>::method::<u32>; //~ ERROR [u8, u16, u32]
17 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
18
19 error: user substs: Canonical { variables: [CanonicalVarInfo { kind: Ty(General) }, CanonicalVarInfo { kind: Ty(General) }], value: [?0, ?1, u32] }
- --> $DIR/dump-fn-method.rs:49:5
+ --> $DIR/dump-fn-method.rs:54:5
21 |
22 LL | y.method::<u32>(44, 66); //~ ERROR [?0, ?1, u32]
23 | ^^^^^^^^^^^^^^^^^^^^^^^ |
the 3 is because the type arguments are in the 3rd position
nikomatsakis
force-pushed
the
nll-type-annot
branch
from
88faf5f
to
ed73a32
Compare
|
@bors r=pnkfelix |
|
📌 Commit ed73a32 has been approved by |
bors
added a commit
that referenced
this pull request
Aug 24, 2018
MIR: support user-given type annotations on fns, structs, and enums This branch adds tooling to track user-given type annotations on functions, structs, and enum variant expressions. The user-given types are passed onto NLL which then enforces them. cc #47184 — not a complete fix, as there are more cases to cover r? @eddyb cc @rust-lang/wg-compiler-nll
|
☀️ Test successful - status-appveyor, status-travis |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This branch adds tooling to track user-given type annotations on functions, structs, and enum variant expressions. The user-given types are passed onto NLL which then enforces them.
cc #47184 — not a complete fix, as there are more cases to cover
r? @eddyb
cc @rust-lang/wg-compiler-nll