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.
ziglang · Apr 24, 2017 · c6605cb · andrewrk · Apr 24, 2017 · thejoshwolfe
1 parent 6de33de
commit c6605cb
Show file tree

Hide file tree

Showing 11 changed files with 80 additions and 22 deletions.
diff --git a/src/all_types.hpp b/src/all_types.hpp
@@ -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;
 

diff --git a/src/codegen.cpp b/src/codegen.cpp
@@ -2885,6 +2885,7 @@ static LLVMValueRef ir_render_instruction(CodeGen *g, IrExecutable *executable,
         case IrInstructionIdTestComptime:
         case IrInstructionIdGeneratedCode:
         case IrInstructionIdCheckSwitchProngs:
+        case IrInstructionIdCheckStatementIsVoid:
         case IrInstructionIdTestType:
         case IrInstructionIdTypeName:
         case IrInstructionIdCanImplicitCast:

diff --git a/src/ir.cpp b/src/ir.cpp
@@ -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 (instruction_type &&
-                    instruction_type->id != TypeTableEntryIdInvalid &&
-                    instruction_type->id != TypeTableEntryIdVoid &&
-                    instruction_type->id != TypeTableEntryIdUnreachable) {
-                    add_node_error(irb->codegen, statement_node, buf_sprintf("expression valued ignored"));
+                if (statement_value != irb->codegen->invalid_instruction) {
+                    ir_mark_gen(ir_build_check_statement_is_void(irb, child_scope, statement_node, statement_value));
                 }
             }
         }
@@ -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:

diff --git a/src/ir_print.cpp b/src/ir_print.cpp
@@ -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;

diff --git a/std/buf_map.zig b/std/buf_map.zig
@@ -31,14 +31,14 @@ pub const BufMap = struct {
         test (self.hash_map.get(key)) |entry| {
             const value_copy = %return self.copy(value);
             %defer self.free(value_copy);
-            %return self.hash_map.put(key, value_copy);
+            _ = %return self.hash_map.put(key, value_copy);
             self.free(entry.value);
         } else {
             const key_copy = %return self.copy(key);
             %defer self.free(key_copy);
             const value_copy = %return self.copy(value);
             %defer self.free(value_copy);
-            %return self.hash_map.put(key_copy, value_copy);
+            _ = %return self.hash_map.put(key_copy, value_copy);
         }
     }
 

diff --git a/std/buf_set.zig b/std/buf_set.zig
@@ -28,7 +28,7 @@ pub const BufSet = struct {
         if (self.hash_map.get(key) == null) {
             const key_copy = %return self.copy(key);
             %defer self.free(key_copy);
-            %return self.hash_map.put(key_copy, {});
+            _ = %return self.hash_map.put(key_copy, {});
         }
     }
 

diff --git a/std/build.zig b/std/build.zig
@@ -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,

diff --git a/std/debug.zig b/std/debug.zig
@@ -217,7 +217,7 @@ fn getString(st: &ElfStackTrace, offset: u64) -> %[]u8 {
 fn readAllocBytes(in_stream: &io.InStream, size: usize) -> %[]u8 {
     const buf = %return global_allocator.alloc(u8, size);
     %defer global_allocator.free(buf);
-    %return in_stream.read(buf);
+    if (size < %return in_stream.read(buf)) return error.Eof;
     return buf;
 }
 

diff --git a/std/fmt.zig b/std/fmt.zig
@@ -386,7 +386,7 @@ fn bufPrintIntToSlice(buf: []u8, value: var, base: u8, uppercase: bool, width: u
 }
 
 test "testParseU64DigitTooBig" {
-    parseUnsigned(u64, "123a", 10) %% |err| {
+    _ = parseUnsigned(u64, "123a", 10) %% |err| {
         if (err == error.InvalidChar) return;
         unreachable;
     };

diff --git a/test/cases/switch_prong_err_enum.zig b/test/cases/switch_prong_err_enum.zig
@@ -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);
 }
diff --git a/test/compile_errors.zig b/test/compile_errors.zig
@@ -1346,7 +1346,7 @@ pub fn addCases(cases: &tests.CompileErrorContext) {
         \\    bar();
         \\}
         \\fn bar() -> i32 { 0 }
-    , ".tmp_source.zig:2:8: error: return value ignored");
+    , ".tmp_source.zig:2:8: error: expression value is ignored");
 
     cases.add("integer literal on a non-comptime var",
         \\export fn foo() {