From 133d36f452d42c5a1629e74850e4e73e0ecba5ad Mon Sep 17 00:00:00 2001 From: Marijn Haverbeke Date: Fri, 16 Sep 2011 16:57:54 +0200 Subject: [PATCH] Require body of else-less if expressions to be a value-less block For consistency with other constructs that could not possibly return a value (say, loops). --- src/comp/middle/resolve.rs | 2 +- src/comp/middle/trans.rs | 2 +- src/comp/syntax/ext/simplext.rs | 2 +- src/comp/syntax/parse/lexer.rs | 2 +- src/comp/syntax/parse/parser.rs | 14 +++++++++----- src/comp/syntax/print/pprust.rs | 6 +++--- src/test/compiletest/compiletest.rs | 4 ++-- src/test/run-pass/block-expr-precedence.rs | 6 +++--- 8 files changed, 21 insertions(+), 17 deletions(-) diff --git a/src/comp/middle/resolve.rs b/src/comp/middle/resolve.rs index b8f10a8f3e76..7018f8d1a858 100644 --- a/src/comp/middle/resolve.rs +++ b/src/comp/middle/resolve.rs @@ -677,7 +677,7 @@ fn lookup_in_scope(e: env, sc: scopes, sp: span, name: ident, ns: namespace) if ns == ns_value { alt lookup_in_pat(name, a.pats[0]) { some(did) { ret some(ast::def_binding(did)); } - _ { ret none } + _ { ret none; } } } } diff --git a/src/comp/middle/trans.rs b/src/comp/middle/trans.rs index 3f253b4920c8..83088f65df2c 100644 --- a/src/comp/middle/trans.rs +++ b/src/comp/middle/trans.rs @@ -5130,7 +5130,7 @@ fn trans_closure(bcx_maybe: option::t<@block_ctxt>, create_real_fn_pair(env.bcx, option::get(llfnty), llfndecl, env.ptr); if copying { - add_clean_temp(bcx, closure, node_id_type(cx.ccx, id)) + add_clean_temp(bcx, closure, node_id_type(cx.ccx, id)); } some({fn_pair: closure, bcx: env.bcx}) } diff --git a/src/comp/syntax/ext/simplext.rs b/src/comp/syntax/ext/simplext.rs index 516795ec9280..f81e101c17b6 100644 --- a/src/comp/syntax/ext/simplext.rs +++ b/src/comp/syntax/ext/simplext.rs @@ -359,7 +359,7 @@ fn transcribe_expr(cx: ext_ctxt, b: bindings, idx_path: @mutable [uint], expr_path(p) { // Don't substitute into qualified names. if vec::len(p.node.types) > 0u || vec::len(p.node.idents) != 1u { - e + e; } alt follow_for_trans(cx, b.find(p.node.idents[0]), idx_path) { some(match_ident(id)) { diff --git a/src/comp/syntax/parse/lexer.rs b/src/comp/syntax/parse/lexer.rs index 1d6a10744b56..e36b26c559e9 100644 --- a/src/comp/syntax/parse/lexer.rs +++ b/src/comp/syntax/parse/lexer.rs @@ -194,7 +194,7 @@ fn scan_dec_digits_with_prefix(rdr: reader) -> str { rdr.bump(); } let digits = scan_dec_digits(rdr); - if negative { str::unshift_char(digits, '-') } + if negative { str::unshift_char(digits, '-'); } ret digits; } diff --git a/src/comp/syntax/parse/parser.rs b/src/comp/syntax/parse/parser.rs index dffa10ff8315..1d84ed148b16 100644 --- a/src/comp/syntax/parse/parser.rs +++ b/src/comp/syntax/parse/parser.rs @@ -28,6 +28,7 @@ type parser = fn swap(token::token, uint, uint); fn look_ahead(uint) -> token::token; fn fatal(str) -> ! ; + fn span_fatal(span, str) -> ! ; fn warn(str); fn restrict(restriction); fn get_restriction() -> restriction; @@ -99,7 +100,10 @@ fn new_parser(sess: parse_sess, cfg: ast::crate_cfg, rdr: lexer::reader, ret buffer[distance - 1u].tok; } fn fatal(m: str) -> ! { - codemap::emit_error(some(self.get_span()), m, sess.cm); + self.span_fatal(self.get_span(), m); + } + fn span_fatal(sp: span, m: str) -> ! { + codemap::emit_error(some(sp), m, sess.cm); fail; } fn warn(m: str) { @@ -1270,6 +1274,9 @@ fn parse_if_expr_1(p: parser) -> let elexpr = parse_else_expr(p); els = some(elexpr); hi = elexpr.span.hi; + } else if !option::is_none(thn.node.expr) { + let sp = option::get(thn.node.expr).span; + p.span_fatal(sp, "if without else can not return a value"); } ret {cond: cond, then: thn, els: els, lo: lo, hi: hi}; } @@ -1658,10 +1665,7 @@ fn parse_block_no_value(p: parser) -> ast::blk { let blk = parse_block(p); if !option::is_none(blk.node.expr) { let sp = option::get(blk.node.expr).span; - codemap::emit_error(some(sp), - "this block must not return a value", - p.get_sess().cm); - fail; + p.span_fatal(sp, "this block must not return a value"); } ret blk; } diff --git a/src/comp/syntax/print/pprust.rs b/src/comp/syntax/print/pprust.rs index 487e262415ed..159a70509c4c 100644 --- a/src/comp/syntax/print/pprust.rs +++ b/src/comp/syntax/print/pprust.rs @@ -599,9 +599,9 @@ fn print_maybe_parens_discrim(s: ps, e: @ast::expr) { ast::expr_fail(none.) { true } _ { false } }; - if disambig { popen(s) } + if disambig { popen(s); } print_expr(s, e); - if disambig { pclose(s) } + if disambig { pclose(s); } } fn print_if(s: ps, test: @ast::expr, blk: ast::blk, @@ -1504,7 +1504,7 @@ fn print_comment(s: ps, cmnt: lexer::cmnt) { pp::STRING(s, _) { s == ";" } _ { false } }; - if is_semi || is_begin(s) || is_end(s) { hardbreak(s.s) } + if is_semi || is_begin(s) || is_end(s) { hardbreak(s.s); } hardbreak(s.s); } } diff --git a/src/test/compiletest/compiletest.rs b/src/test/compiletest/compiletest.rs index 0961191918b1..b75bb10bab39 100644 --- a/src/test/compiletest/compiletest.rs +++ b/src/test/compiletest/compiletest.rs @@ -151,11 +151,11 @@ fn is_test(config: config, testfile: str) -> bool { let valid = false; for ext in valid_extensions { - if str::ends_with(name, ext) { valid = true } + if str::ends_with(name, ext) { valid = true; } } for pre in invalid_prefixes { - if str::starts_with(name, pre) { valid = false } + if str::starts_with(name, pre) { valid = false; } } ret valid; diff --git a/src/test/run-pass/block-expr-precedence.rs b/src/test/run-pass/block-expr-precedence.rs index 571ddfe26b76..6c231f2fc877 100644 --- a/src/test/run-pass/block-expr-precedence.rs +++ b/src/test/run-pass/block-expr-precedence.rs @@ -51,7 +51,7 @@ fn main() { assert if (true) { 12 } else { 12 } - num == 0; assert 12 - if (true) { 12 } else { 12 } == 0; - if (true) { 12 } {-num}; - if (true) { 12 }; {-num}; - if (true) { 12 };;; -num; + if (true) { 12; } {-num}; + if (true) { 12; }; {-num}; + if (true) { 12; };;; -num; }