Make output type in ast::FnDecl optional #21099
Conversation
|
(rust_highfive has picked a reviewer for you, use r? to override) |
| if let ast::Return(ref ty) = decl.output { | ||
| match ty.node { | ||
| ast::TyTup(ref tys) if tys.is_empty() => { | ||
| return self.maybe_print_comment(ty.span.lo); |
There was a problem hiding this comment.
Just making sure, but was it intentional to lose the maybe_print_comment? (not that I know what it does...)
There was a problem hiding this comment.
(same in two spots above as well)
There was a problem hiding this comment.
I think that prints comments that may appear at that spot in the source.
There was a problem hiding this comment.
It was intentional. As far as I can tell, this does not break anything. For example, fn f() /*1*/ {} still prints correctly.
|
I checked callers of FunctionRetTy::span, and the only problem I could come up with was this: trait T {}
fn main() { let _: T = (|| loop {})(); }which causes: The error is that the return type of the closure is unsized. After this patch, rustc will highlight the beginning of the crate instead of (incorrect) |
|
Thanks for investigating that! Perhaps instead of |
|
What span should I attach? The whole point is there is no correct span. |
|
It looks like the parser just currently uses something along the lines of |
sanxiyn
force-pushed
the
opt-return-ty
branch
from
bfaa8dd to
3f0cc80
Compare
|
Completely re-done with a custom enum. |
|
⌛ Testing commit 3f0cc80 with merge e5ae40f... |
|
💔 Test failed - auto-mac-64-nopt-t |
|
@bors: retry |
This avoids having ast::Ty nodes which have no counterpart in the source.
| @@ -297,11 +297,8 @@ fn is_test_fn(cx: &TestCtxt, i: &ast::Item) -> bool { | |||
| match &i.node { | |||
| &ast::ItemFn(ref decl, _, _, ref generics, _) => { | |||
| let no_output = match decl.output { | |||
| ast::Return(ref ret_ty) => match ret_ty.node { | |||
There was a problem hiding this comment.
Removing this variant seems to have broken quickcheck, because the function returned by item_fn seems to use ast::Return, not ast::DefaultReturn. Is this something that needs to be fixed on quickcheck's end, or was the switch from ast::Return(..) => true to => false unintentional?
There was a problem hiding this comment.
It was intentional, but that is an unfortunate side effect. It can be fixed on quickcheck's end. Whether it needs to be I am less sure, rustc certainly could be more accepting here. Probably both should be fixed.
|
rustc side will be fixed by #21504. |
This avoids having ast::Ty nodes which have no counterpart in the source.