Commit c6605cba8375871a145896f9cf1090e9c823214a | zig

author	Josh Wolfe <thejoshwolfe@gmail.com> 2017-04-23 16:59:55-07:00
committer	Josh Wolfe <thejoshwolfe@gmail.com> 2017-04-23 21:15:15-07:00
log	c6605cba8375871a145896f9cf1090e9c823214a
tree	f6309057804dc9fd6d5fa10feafb9d8f6a44a713
parent	6de33ded81981554ccffc3ecbfcd5b0f628cf502

blocks check that their statements are void

closes #291

This changes the error message "return value ignored" to "expression value is ignored".
This is because this error also applies to {1;}, which has no function calls.

Also fix ignored expression values in std and test.
This caught a bug in debug.readAllocBytes where an early Eof error would have been missed.
See #219.

11 files changed, 80 insertions(+), 22 deletions(-)

src
std
test
- cases
  - switch_prong_err_enum.zig
- compile_errors.zig

src/all_types.hpp+7

 @@ -1738,6 +1738,7 @@ enum IrInstructionId {
     IrInstructionIdIntToErr,
     IrInstructionIdErrToInt,
     IrInstructionIdCheckSwitchProngs,
+    IrInstructionIdCheckStatementIsVoid,
     IrInstructionIdTestType,
     IrInstructionIdTypeName,
     IrInstructionIdCanImplicitCast,
 @@ -2435,6 +2436,12 @@ struct IrInstructionCheckSwitchProngs {
     size_t range_count;
 };
+struct IrInstructionCheckStatementIsVoid {
+    IrInstruction base;
+    IrInstruction *statement_value;
+};
 struct IrInstructionTestType {
     IrInstruction base;

src/codegen.cpp+1

...	@@ -2885,6 +2885,7 @@ static LLVMValueRef ir_render_instruction(CodeGen g, IrExecutable executable,	...	@@ -2885,6 +2885,7 @@ static LLVMValueRef ir_render_instruction(CodeGen g, IrExecutable executable,
2885	case IrInstructionIdTestComptime:	2885	case IrInstructionIdTestComptime:
2886	case IrInstructionIdGeneratedCode:	2886	case IrInstructionIdGeneratedCode:
2887	case IrInstructionIdCheckSwitchProngs:	2887	case IrInstructionIdCheckSwitchProngs:
		2888	case IrInstructionIdCheckStatementIsVoid:
2888	case IrInstructionIdTestType:	2889	case IrInstructionIdTestType:
2889	case IrInstructionIdTypeName:	2890	case IrInstructionIdTypeName:
2890	case IrInstructionIdCanImplicitCast:	2891	case IrInstructionIdCanImplicitCast:

src/ir.cpp+48-13

 @@ -510,6 +510,10 @@ static constexpr IrInstructionId ir_instruction_id(IrInstructionCheckSwitchProng
     return IrInstructionIdCheckSwitchProngs;
 }
+static constexpr IrInstructionId ir_instruction_id(IrInstructionCheckStatementIsVoid *) {
+    return IrInstructionIdCheckStatementIsVoid;
+}
 static constexpr IrInstructionId ir_instruction_id(IrInstructionTestType *) {
     return IrInstructionIdTestType;
 }
 @@ -2803,6 +2807,15 @@ static IrInstruction *ir_instruction_checkswitchprongs_get_dep(IrInstructionChec
     return nullptr;
 }
+static IrInstruction *ir_instruction_checkstatementisvoid_get_dep(IrInstructionCheckStatementIsVoid *instruction,
+        size_t index)
+{
+    switch (index) {
+        case 0: return instruction->statement_value;
+        default: return nullptr;
+    }
+}
 static IrInstruction *ir_instruction_testtype_get_dep(IrInstructionTestType *instruction, size_t index) {
     switch (index) {
         case 0: return instruction->type_value;
 @@ -3060,6 +3073,8 @@ static IrInstruction *ir_instruction_get_dep(IrInstruction *instruction, size_t
             return ir_instruction_errtoint_get_dep((IrInstructionErrToInt *) instruction, index);
         case IrInstructionIdCheckSwitchProngs:
             return ir_instruction_checkswitchprongs_get_dep((IrInstructionCheckSwitchProngs *) instruction, index);
+        case IrInstructionIdCheckStatementIsVoid:
+            return ir_instruction_checkstatementisvoid_get_dep((IrInstructionCheckStatementIsVoid *) instruction, index);
         case IrInstructionIdTestType:
             return ir_instruction_testtype_get_dep((IrInstructionTestType *) instruction, index);
         case IrInstructionIdTypeName:
 @@ -3333,6 +3348,18 @@ static ScopeBlock *find_block_scope(IrExecutable *exec, Scope *scope) {
     return nullptr;
 }
+static IrInstruction *ir_build_check_statement_is_void(IrBuilder *irb, Scope *scope, AstNode *source_node,
+    IrInstruction* statement_value)
+{
+    IrInstructionCheckStatementIsVoid *instruction = ir_build_instruction<IrInstructionCheckStatementIsVoid>(
+            irb, scope, source_node);
+    instruction->statement_value = statement_value;
+    ir_ref_instruction(statement_value, irb->current_basic_block);
+    return &instruction->base;
+}
 static IrInstruction *ir_gen_block(IrBuilder *irb, Scope *parent_scope, AstNode *block_node) {
     assert(block_node->type == NodeTypeBlock);
 @@ -3410,12 +3437,8 @@ static IrInstruction *ir_gen_block(IrBuilder *irb, Scope *parent_scope, AstNode
                 return_value = statement_value;
             } else {
                 // there are more statements ahead of this one. this statement's value must be void
-                TypeTableEntry *instruction_type = statement_value->value.type;
+                if (statement_value != irb->codegen->invalid_instruction) {
-                if (instruction_type &&
+                    ir_mark_gen(ir_build_check_statement_is_void(irb, child_scope, statement_node, statement_value));
-                    instruction_type->id != TypeTableEntryIdInvalid &&
-                    instruction_type->id != TypeTableEntryIdVoid &&
-                    instruction_type->id != TypeTableEntryIdUnreachable) {
-                    add_node_error(irb->codegen, statement_node, buf_sprintf("expression valued ignored"));
                 }
             }
         }
 @@ -12640,6 +12663,22 @@ static TypeTableEntry *ir_analyze_instruction_check_switch_prongs(IrAnalyze *ira
     return ira->codegen->builtin_types.entry_void;
 }
+static TypeTableEntry *ir_analyze_instruction_check_statement_is_void(IrAnalyze *ira,
+        IrInstructionCheckStatementIsVoid *instruction)
+{
+    IrInstruction *statement_value = instruction->statement_value;
+    TypeTableEntry *statement_type = statement_value->value.type;
+    if (type_is_invalid(statement_type))
+        return ira->codegen->builtin_types.entry_invalid;
+    if (statement_type->id != TypeTableEntryIdVoid) {
+        ir_add_error(ira, &instruction->base, buf_sprintf("expression value is ignored"));
+    }
+    ir_build_const_from(ira, &instruction->base);
+    return ira->codegen->builtin_types.entry_void;
+}
 static TypeTableEntry *ir_analyze_instruction_test_type(IrAnalyze *ira, IrInstructionTestType *instruction) {
     IrInstruction *type_value = instruction->type_value->other;
     TypeTableEntry *type_entry = ir_resolve_type(ira, type_value);
 @@ -12987,6 +13026,8 @@ static TypeTableEntry *ir_analyze_instruction_nocast(IrAnalyze *ira, IrInstructi
             return ir_analyze_instruction_test_comptime(ira, (IrInstructionTestComptime *)instruction);
         case IrInstructionIdCheckSwitchProngs:
             return ir_analyze_instruction_check_switch_prongs(ira, (IrInstructionCheckSwitchProngs *)instruction);
+        case IrInstructionIdCheckStatementIsVoid:
+            return ir_analyze_instruction_check_statement_is_void(ira, (IrInstructionCheckStatementIsVoid *)instruction);
         case IrInstructionIdTestType:
             return ir_analyze_instruction_test_type(ira, (IrInstructionTestType *)instruction);
         case IrInstructionIdCanImplicitCast:
 @@ -13026,13 +13067,6 @@ static TypeTableEntry *ir_analyze_instruction(IrAnalyze *ira, IrInstruction *ins
                instruction_type->id == TypeTableEntryIdUnreachable);
         instruction->other = instruction;
     }
-    if (instruction_type->id != TypeTableEntryIdInvalid &&
-        instruction_type->id != TypeTableEntryIdVoid &&
-        instruction_type->id != TypeTableEntryIdUnreachable &&
-        instruction->ref_count == 0)
-    {
-        ir_add_error(ira, instruction, buf_sprintf("return value ignored"));
-    }
     return instruction_type;
 }
 @@ -13123,6 +13157,7 @@ bool ir_has_side_effects(IrInstruction *instruction) {
         case IrInstructionIdBreakpoint:
         case IrInstructionIdOverflowOp: // TODO when we support multiple returns this can be side effect free
         case IrInstructionIdCheckSwitchProngs:
+        case IrInstructionIdCheckStatementIsVoid:
         case IrInstructionIdSetGlobalAlign:
         case IrInstructionIdSetGlobalSection:
         case IrInstructionIdSetGlobalLinkage:

src/ir_print.cpp+9

 @@ -814,6 +814,12 @@ static void ir_print_check_switch_prongs(IrPrint *irp, IrInstructionCheckSwitchP
     fprintf(irp->f, ")");
 }
+static void ir_print_check_statement_is_void(IrPrint *irp, IrInstructionCheckStatementIsVoid *instruction) {
+    fprintf(irp->f, "@checkStatementIsVoid(");
+    ir_print_other_instruction(irp, instruction->statement_value);
+    fprintf(irp->f, ")");
+}
 static void ir_print_test_type(IrPrint *irp, IrInstructionTestType *instruction) {
     fprintf(irp->f, "testtype ");
     ir_print_other_instruction(irp, instruction->type_value);
 @@ -1149,6 +1155,9 @@ static void ir_print_instruction(IrPrint *irp, IrInstruction *instruction) {
         case IrInstructionIdCheckSwitchProngs:
             ir_print_check_switch_prongs(irp, (IrInstructionCheckSwitchProngs *)instruction);
             break;
+        case IrInstructionIdCheckStatementIsVoid:
+            ir_print_check_statement_is_void(irp, (IrInstructionCheckStatementIsVoid *)instruction);
+            break;
         case IrInstructionIdTestType:
             ir_print_test_type(irp, (IrInstructionTestType *)instruction);
             break;

std/buf_map.zig+2-2

...	@@ -31,14 +31,14 @@ pub const BufMap = struct {	...	@@ -31,14 +31,14 @@ pub const BufMap = struct {
31	test (self.hash_map.get(key)) \|entry\| {	31	test (self.hash_map.get(key)) \|entry\| {
32	const value_copy = %return self.copy(value);	32	const value_copy = %return self.copy(value);
33	%defer self.free(value_copy);	33	%defer self.free(value_copy);
34	%return self.hash_map.put(key, value_copy);	34	_ = %return self.hash_map.put(key, value_copy);
35	self.free(entry.value);	35	self.free(entry.value);
36	} else {	36	} else {
37	const key_copy = %return self.copy(key);	37	const key_copy = %return self.copy(key);
38	%defer self.free(key_copy);	38	%defer self.free(key_copy);
39	const value_copy = %return self.copy(value);	39	const value_copy = %return self.copy(value);
40	%defer self.free(value_copy);	40	%defer self.free(value_copy);
41	%return self.hash_map.put(key_copy, value_copy);	41	_ = %return self.hash_map.put(key_copy, value_copy);
42	}	42	}
43	}	43	}
44		44

std/buf_set.zig+1-1

...	@@ -28,7 +28,7 @@ pub const BufSet = struct {	...	@@ -28,7 +28,7 @@ pub const BufSet = struct {
28	if (self.hash_map.get(key) == null) {	28	if (self.hash_map.get(key) == null) {
29	const key_copy = %return self.copy(key);	29	const key_copy = %return self.copy(key);
30	%defer self.free(key_copy);	30	%defer self.free(key_copy);
31	%return self.hash_map.put(key_copy, {});	31	_ = %return self.hash_map.put(key_copy, {});
32	}	32	}
33	}	33	}
34		34

std/build.zig+5-2

 @@ -415,20 +415,23 @@ pub const Builder = struct {
             .value = UserValue.Scalar { value },
             .used = false,
         })) |*prev_value| {
+            // option already exists
             switch (prev_value.value) {
                 UserValue.Scalar => |s| {
+                    // turn it into a list
                     var list = List([]const u8).init(self.allocator);
                     %%list.append(s);
                     %%list.append(value);
-                    %%self.user_input_options.put(name, UserInputOption {
+                    _ = %%self.user_input_options.put(name, UserInputOption {
                         .name = name,
                         .value = UserValue.List { list },
                         .used = false,
                     });
                 },
                 UserValue.List => |*list| {
+                    // append to the list
                     %%list.append(value);
-                    %%self.user_input_options.put(name, UserInputOption {
+                    _ = %%self.user_input_options.put(name, UserInputOption {
                         .name = name,
                         .value = UserValue.List { *list },
                         .used = false,

std/debug.zig+1-1

...	@@ -217,7 +217,7 @@ fn getString(st: &ElfStackTrace, offset: u64) -> %[]u8 {	...	@@ -217,7 +217,7 @@ fn getString(st: &ElfStackTrace, offset: u64) -> %[]u8 {
217	fn readAllocBytes(in_stream: &io.InStream, size: usize) -> %[]u8 {	217	fn readAllocBytes(in_stream: &io.InStream, size: usize) -> %[]u8 {
218	const buf = %return global_allocator.alloc(u8, size);	218	const buf = %return global_allocator.alloc(u8, size);
219	%defer global_allocator.free(buf);	219	%defer global_allocator.free(buf);
220	%return in_stream.read(buf);	220	if (size < %return in_stream.read(buf)) return error.Eof;
221	return buf;	221	return buf;
222	}	222	}
223		223

std/fmt.zig+1-1

...	@@ -386,7 +386,7 @@ fn bufPrintIntToSlice(buf: []u8, value: var, base: u8, uppercase: bool, width: u	...	@@ -386,7 +386,7 @@ fn bufPrintIntToSlice(buf: []u8, value: var, base: u8, uppercase: bool, width: u
386	}	386	}
387		387
388	test "testParseU64DigitTooBig" {	388	test "testParseU64DigitTooBig" {
389	parseUnsigned(u64, "123a", 10) %% \|err\| {	389	_ = parseUnsigned(u64, "123a", 10) %% \|err\| {
390	if (err == error.InvalidChar) return;	390	if (err == error.InvalidChar) return;
391	unreachable;	391	unreachable;
392	};	392	};

test/cases/switch_prong_err_enum.zig+4-1

 @@ -22,6 +22,9 @@ fn doThing(form_id: u64) -> %FormValue {
 }
 test "switchProngReturnsErrorEnum" {
-    %%doThing(17);
+    switch (%%doThing(17)) {
+        FormValue.Address => |payload| { assert(payload == 1); },
+        else => unreachable,
+    }
     assert(read_count == 1);
 }

test/compile_errors.zig+1-1

...	@@ -1346,7 +1346,7 @@ pub fn addCases(cases: &tests.CompileErrorContext) {	...	@@ -1346,7 +1346,7 @@ pub fn addCases(cases: &tests.CompileErrorContext) {
1346	\\ bar();	1346	\\ bar();
1347	\\}	1347	\\}
1348	\\fn bar() -> i32 { 0 }	1348	\\fn bar() -> i32 { 0 }
1349	, ".tmp_source.zig:2:8: error: return value ignored");	1349	, ".tmp_source.zig:2:8: error: expression value is ignored");
1350		1350
1351	cases.add("integer literal on a non-comptime var",	1351	cases.add("integer literal on a non-comptime var",
1352	\\export fn foo() {	1352	\\export fn foo() {