Skip to content

Make output type in ast::FnDecl optional #21099

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

Merged
merged 1 commit into from
Jan 19, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/librustc/middle/infer/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1177,6 +1177,7 @@ impl<'a, 'tcx> Rebuilder<'a, 'tcx> {
ast::Return(ref ret_ty) => ast::Return(
self.rebuild_arg_ty_or_output(&**ret_ty, lifetime, anon_nums, region_names)
),
ast::DefaultReturn(span) => ast::DefaultReturn(span),
ast::NoReturn(span) => ast::NoReturn(span)
}
}
Expand Down
21 changes: 9 additions & 12 deletions src/librustc_trans/trans/debuginfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1450,18 +1450,15 @@ pub fn create_function_debug_context<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>,
let mut signature = Vec::with_capacity(fn_decl.inputs.len() + 1);

// Return type -- llvm::DIBuilder wants this at index 0
match fn_decl.output {
ast::Return(ref ret_ty) if ret_ty.node == ast::TyTup(vec![]) =>
signature.push(ptr::null_mut()),
_ => {
assert_type_for_node_id(cx, fn_ast_id, error_reporting_span);

let return_type = ty::node_id_to_type(cx.tcx(), fn_ast_id);
let return_type = monomorphize::apply_param_substs(cx.tcx(),
param_substs,
&return_type);
signature.push(type_metadata(cx, return_type, codemap::DUMMY_SP));
}
assert_type_for_node_id(cx, fn_ast_id, error_reporting_span);
let return_type = ty::node_id_to_type(cx.tcx(), fn_ast_id);
let return_type = monomorphize::apply_param_substs(cx.tcx(),
param_substs,
&return_type);
if ty::type_is_nil(return_type) {
signature.push(ptr::null_mut())
} else {
signature.push(type_metadata(cx, return_type, codemap::DUMMY_SP));
}

// Arguments types
Expand Down
5 changes: 2 additions & 3 deletions src/librustc_trans/trans/foreign.rs
Original file line number Diff line number Diff line change
Expand Up @@ -445,9 +445,8 @@ fn gate_simd_ffi(tcx: &ty::ctxt, decl: &ast::FnDecl, ty: &ty::BareFnTy) {
for (input, ty) in decl.inputs.iter().zip(sig.inputs.iter()) {
check(&*input.ty, *ty)
}
match decl.output {
ast::NoReturn(_) => {}
ast::Return(ref ty) => check(&**ty, sig.output.unwrap())
if let ast::Return(ref ty) = decl.output {
check(&**ty, sig.output.unwrap())
}
}
}
Expand Down
18 changes: 13 additions & 5 deletions src/librustc_typeck/astconv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1359,7 +1359,8 @@ fn ty_of_method_or_bare_fn<'a, 'tcx>(this: &AstConv<'tcx>,
implied_output_region,
lifetimes_for_params,
&**output)),
ast::NoReturn(_) => ty::FnDiverging
ast::DefaultReturn(..) => ty::FnConverging(ty::mk_nil(this.tcx())),
ast::NoReturn(..) => ty::FnDiverging
};

(ty::BareFnTy {
Expand Down Expand Up @@ -1486,14 +1487,21 @@ pub fn ty_of_closure<'tcx>(

let expected_ret_ty = expected_sig.map(|e| e.output);

let is_infer = match decl.output {
ast::Return(ref output) if output.node == ast::TyInfer => true,
ast::DefaultReturn(..) => true,
_ => false
};

let output_ty = match decl.output {
ast::Return(ref output) if output.node == ast::TyInfer && expected_ret_ty.is_some() =>
_ if is_infer && expected_ret_ty.is_some() =>
expected_ret_ty.unwrap(),
ast::Return(ref output) if output.node == ast::TyInfer =>
ty::FnConverging(this.ty_infer(output.span)),
_ if is_infer =>
ty::FnConverging(this.ty_infer(decl.output.span())),
ast::Return(ref output) =>
ty::FnConverging(ast_ty_to_ty(this, &rb, &**output)),
ast::NoReturn(_) => ty::FnDiverging
ast::DefaultReturn(..) => unreachable!(),
ast::NoReturn(..) => ty::FnDiverging
};

debug!("ty_of_closure: input_tys={}", input_tys.repr(this.tcx()));
Expand Down
4 changes: 3 additions & 1 deletion src/librustc_typeck/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1488,7 +1488,9 @@ fn ty_of_foreign_fn_decl<'a, 'tcx>(ccx: &CollectCtxt<'a, 'tcx>,
let output = match decl.output {
ast::Return(ref ty) =>
ty::FnConverging(ast_ty_to_ty(ccx, &rb, &**ty)),
ast::NoReturn(_) =>
ast::DefaultReturn(..) =>
ty::FnConverging(ty::mk_nil(ccx.tcx)),
ast::NoReturn(..) =>
ty::FnDiverging
};

Expand Down
4 changes: 3 additions & 1 deletion src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1141,14 +1141,16 @@ impl Clean<Argument> for ast::Arg {
#[derive(Clone, RustcEncodable, RustcDecodable, PartialEq, Show)]
pub enum FunctionRetTy {
Return(Type),
DefaultReturn,
NoReturn
}

impl Clean<FunctionRetTy> for ast::FunctionRetTy {
fn clean(&self, cx: &DocContext) -> FunctionRetTy {
match *self {
ast::Return(ref typ) => Return(typ.clean(cx)),
ast::NoReturn(_) => NoReturn
ast::DefaultReturn(..) => DefaultReturn,
ast::NoReturn(..) => NoReturn
}
}
}
Expand Down
1 change: 1 addition & 0 deletions src/librustdoc/html/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,7 @@ impl fmt::String for clean::FunctionRetTy {
match *self {
clean::Return(clean::Tuple(ref tys)) if tys.is_empty() => Ok(()),
clean::Return(ref ty) => write!(f, " -&gt; {}", ty),
clean::DefaultReturn => Ok(()),
clean::NoReturn => write!(f, " -&gt; !")
}
}
Expand Down
5 changes: 5 additions & 0 deletions src/libsyntax/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1390,6 +1390,10 @@ pub enum FunctionRetTy {
/// Functions with return type ! that always
/// raise an error or exit (i.e. never return to the caller)
NoReturn(Span),
/// Return type is not specified. Functions default to () and
/// closures default to inference. Span points to where return
/// type would be inserted.
DefaultReturn(Span),
/// Everything else
Return(P<Ty>),
}
Expand All @@ -1398,6 +1402,7 @@ impl FunctionRetTy {
pub fn span(&self) -> Span {
match *self {
NoReturn(span) => span,
DefaultReturn(span) => span,
Return(ref ty) => ty.span
}
}
Expand Down
10 changes: 2 additions & 8 deletions src/libsyntax/fold.rs
Original file line number Diff line number Diff line change
Expand Up @@ -726,6 +726,7 @@ pub fn noop_fold_fn_decl<T: Folder>(decl: P<FnDecl>, fld: &mut T) -> P<FnDecl> {
inputs: inputs.move_map(|x| fld.fold_arg(x)),
output: match output {
Return(ty) => Return(fld.fold_ty(ty)),
DefaultReturn(span) => DefaultReturn(span),
NoReturn(span) => NoReturn(span)
},
variadic: variadic
Expand Down Expand Up @@ -1189,14 +1190,7 @@ pub fn noop_fold_foreign_item<T: Folder>(ni: P<ForeignItem>, folder: &mut T) ->
attrs: attrs.move_map(|x| folder.fold_attribute(x)),
node: match node {
ForeignItemFn(fdec, generics) => {
ForeignItemFn(fdec.map(|FnDecl {inputs, output, variadic}| FnDecl {
inputs: inputs.move_map(|a| folder.fold_arg(a)),
output: match output {
Return(ty) => Return(folder.fold_ty(ty)),
NoReturn(span) => NoReturn(span)
},
variadic: variadic
}), folder.fold_generics(generics))
ForeignItemFn(folder.fold_fn_decl(fdec), folder.fold_generics(generics))
}
ForeignItemStatic(t, m) => {
ForeignItemStatic(folder.fold_ty(t), m)
Expand Down
4 changes: 1 addition & 3 deletions src/libsyntax/parse/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1066,9 +1066,7 @@ mod test {
}),
id: ast::DUMMY_NODE_ID
}),
output: ast::Return(P(ast::Ty{id: ast::DUMMY_NODE_ID,
node: ast::TyTup(vec![]),
span:sp(15,15)})), // not sure
output: ast::DefaultReturn(sp(15, 15)),
variadic: false
}),
ast::Unsafety::Normal,
Expand Down
29 changes: 5 additions & 24 deletions src/libsyntax/parse/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ use ast::{Mod, BiAdd, Arg, Arm, Attribute, BindByRef, BindByValue};
use ast::{BiBitAnd, BiBitOr, BiBitXor, BiRem, BiLt, BiGt, Block};
use ast::{BlockCheckMode, CaptureByRef, CaptureByValue, CaptureClause};
use ast::{Crate, CrateConfig, Decl, DeclItem};
use ast::{DeclLocal, DefaultBlock, UnDeref, BiDiv, EMPTY_CTXT, EnumDef, ExplicitSelf};
use ast::{DeclLocal, DefaultBlock, DefaultReturn};
use ast::{UnDeref, BiDiv, EMPTY_CTXT, EnumDef, ExplicitSelf};
use ast::{Expr, Expr_, ExprAddrOf, ExprMatch, ExprAgain};
use ast::{ExprAssign, ExprAssignOp, ExprBinary, ExprBlock, ExprBox};
use ast::{ExprBreak, ExprCall, ExprCast};
Expand Down Expand Up @@ -1426,11 +1427,7 @@ impl<'a> Parser<'a> {
}
} else {
let pos = self.span.lo;
Return(P(Ty {
id: ast::DUMMY_NODE_ID,
node: TyTup(vec![]),
span: mk_sp(pos, pos),
}))
DefaultReturn(mk_sp(pos, pos))
}
}

Expand Down Expand Up @@ -4548,15 +4545,7 @@ impl<'a> Parser<'a> {
(optional_unboxed_closure_kind, args)
}
};
let output = if self.check(&token::RArrow) {
self.parse_ret_ty()
} else {
Return(P(Ty {
id: ast::DUMMY_NODE_ID,
node: TyInfer,
span: self.span,
}))
};
let output = self.parse_ret_ty();

(P(FnDecl {
inputs: inputs_captures,
Expand All @@ -4573,15 +4562,7 @@ impl<'a> Parser<'a> {
seq_sep_trailing_allowed(token::Comma),
|p| p.parse_fn_block_arg());

let output = if self.check(&token::RArrow) {
self.parse_ret_ty()
} else {
Return(P(Ty {
id: ast::DUMMY_NODE_ID,
node: TyInfer,
span: self.span,
}))
};
let output = self.parse_ret_ty();

P(FnDecl {
inputs: inputs,
Expand Down
28 changes: 10 additions & 18 deletions src/libsyntax/print/pprust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2351,10 +2351,8 @@ impl<'a> State<'a> {
try!(self.print_fn_args(decl, None));
try!(word(&mut self.s, "|"));

if let ast::Return(ref ty) = decl.output {
if ty.node == ast::TyInfer {
return self.maybe_print_comment(ty.span.lo);
}
if let ast::DefaultReturn(..) = decl.output {
return Ok(());
}

try!(self.space_if_not_bol());
Expand All @@ -2364,6 +2362,7 @@ impl<'a> State<'a> {
try!(self.print_type(&**ty));
self.maybe_print_comment(ty.span.lo)
}
ast::DefaultReturn(..) => unreachable!(),
ast::NoReturn(span) => {
try!(self.word_nbsp("!"));
self.maybe_print_comment(span.lo)
Expand All @@ -2385,10 +2384,8 @@ impl<'a> State<'a> {
try!(self.print_fn_args(decl, None));
try!(word(&mut self.s, ")"));

if let ast::Return(ref ty) = decl.output {
if ty.node == ast::TyInfer {
return self.maybe_print_comment(ty.span.lo);
}
if let ast::DefaultReturn(..) = decl.output {
return Ok(());
}

try!(self.space_if_not_bol());
Expand All @@ -2398,6 +2395,7 @@ impl<'a> State<'a> {
try!(self.print_type(&**ty));
self.maybe_print_comment(ty.span.lo)
}
ast::DefaultReturn(..) => unreachable!(),
ast::NoReturn(span) => {
try!(self.word_nbsp("!"));
self.maybe_print_comment(span.lo)
Expand Down Expand Up @@ -2684,13 +2682,8 @@ impl<'a> State<'a> {
}

pub fn print_fn_output(&mut self, decl: &ast::FnDecl) -> IoResult<()> {
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);
}
_ => ()
}
if let ast::DefaultReturn(..) = decl.output {
return Ok(());
}

try!(self.space_if_not_bol());
Expand All @@ -2699,6 +2692,7 @@ impl<'a> State<'a> {
match decl.output {
ast::NoReturn(_) =>
try!(self.word_nbsp("!")),
ast::DefaultReturn(..) => unreachable!(),
ast::Return(ref ty) =>
try!(self.print_type(&**ty))
}
Expand Down Expand Up @@ -3071,9 +3065,7 @@ mod test {

let decl = ast::FnDecl {
inputs: Vec::new(),
output: ast::Return(P(ast::Ty {id: 0,
node: ast::TyTup(vec![]),
span: codemap::DUMMY_SP})),
output: ast::DefaultReturn(codemap::DUMMY_SP),
variadic: false
};
let generics = ast_util::empty_generics();
Expand Down
14 changes: 4 additions & 10 deletions src/libsyntax/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
ast::TyTup(ref tys) if tys.is_empty() => true,
_ => false,
},
ast::NoReturn(_) => false
ast::DefaultReturn(..) => true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

_ => false
};
if decl.inputs.is_empty()
&& no_output
Expand Down Expand Up @@ -336,11 +333,8 @@ fn is_bench_fn(cx: &TestCtxt, i: &ast::Item) -> bool {
ast::ItemFn(ref decl, _, _, ref generics, _) => {
let input_cnt = decl.inputs.len();
let no_output = match decl.output {
ast::Return(ref ret_ty) => match ret_ty.node {
ast::TyTup(ref tys) if tys.is_empty() => true,
_ => false,
},
ast::NoReturn(_) => false
ast::DefaultReturn(..) => true,
_ => false
};
let tparm_cnt = generics.ty_params.len();
// NB: inadequate check, but we're running
Expand Down
17 changes: 17 additions & 0 deletions src/test/pretty/fn-return.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// pp-exact

// Check that `fn f() -> () { }` does not print as `fn f() { }`.

fn f() -> () { }

fn main() { }