Skip to content

Commit

Permalink
stage1: allow (?T == ?T) and combinations
Browse files Browse the repository at this point in the history 
 * unwrap operand(s) to { BinOpTypeCmpEq, BinOpTypeCmpNotEq }:
   - when not comparing with null
   - when either lhs or rhs or both are optional
 * repeat until neither operand was unwrapped
  • Loading branch information
@mikdusan
mikdusan committed Apr 19, 2020
1 parent 895f2d9 commit baf161e
Show file tree
Hide file tree
Showing 3 changed files with 168 additions and 0 deletions.
3 changes: 3 additions & 0 deletions CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,9 @@ if("${CMAKE_BUILD_TYPE}" STREQUAL "Debug")
set(EXE_CFLAGS "${EXE_CFLAGS} /w")
else()
set(EXE_CFLAGS "${EXE_CFLAGS} -Werror -Wall -Werror=implicit-fallthrough")
set(EXE_CFLAGS "${EXE_CFLAGS} -fcolor-diagnostics")
set(EXE_CFLAGS "${EXE_CFLAGS} -Wno-error=unused-variable")
set(EXE_CFLAGS "${EXE_CFLAGS} -Wno-error=unused-label")
endif()
endif()

Expand Down
83 changes: 83 additions & 0 deletions src/ir.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,9 @@ static IrInstGen *ir_analyze_struct_value_field_value(IrAnalyze *ira, IrInst* so
IrInstGen *struct_operand, TypeStructField *field);
static bool value_cmp_numeric_val_any(ZigValue *left, Cmp predicate, ZigValue *right);
static bool value_cmp_numeric_val_all(ZigValue *left, Cmp predicate, ZigValue *right);
static IrInstGen *ir_analyze_optional_value_payload_value(IrAnalyze *ira, IrInst* source_instr,
IrInstGen *optional_operand, bool safety_check_on);


static void destroy_instruction_src(IrInstSrc *inst) {
switch (inst->id) {
Expand Down Expand Up @@ -16394,6 +16397,86 @@ static IrInstGen *ir_analyze_bin_op_cmp(IrAnalyze *ira, IrInstSrcBinOp *bin_op_i

IrBinOp op_id = bin_op_instruction->op_id;
bool is_equality_cmp = (op_id == IrBinOpCmpEq || op_id == IrBinOpCmpNotEq);

// unwrap_equality_operands: (?T == ?T) and combinations
if (is_equality_cmp &&
op1->value->type->id != ZigTypeIdNull &&
op2->value->type->id != ZigTypeIdNull)
{
if (instr_is_comptime(op1) && instr_is_comptime(op2)) for (;;) {
bool repeat = false;

const bool is_op1_optional = op1->value->type->id == ZigTypeIdOptional;
const bool is_op2_optional = op2->value->type->id == ZigTypeIdOptional;

const bool is_op1_null = is_op1_optional ? optional_value_is_null(op1->value) : false;
const bool is_op2_null = is_op2_optional ? optional_value_is_null(op2->value) : false;

if (is_op1_optional) {
if (is_op1_null) {
const bool answer = is_op2_null ? true : false;
return ir_const_bool(ira, &bin_op_instruction->base.base,
op_id == IrBinOpCmpEq ? answer : !answer);
}

if (op1->value->type->data.maybe.child_type->id == ZigTypeIdPointer) {
op1 = ir_analyze_optional_value_payload_value(ira, &bin_op_instruction->base.base,
op1, bin_op_instruction->safety_check_on);
if (type_is_invalid(op1->value->type))
return ira->codegen->invalid_inst_gen;
} else {
op1->value = op1->value->data.x_optional;
}

repeat = true;
}

if (is_op2_optional) {
if (is_op2_null)
return ir_const_bool(ira, &bin_op_instruction->base.base, op_id == IrBinOpCmpEq ? false : true);

if (op2->value->type->data.maybe.child_type->id == ZigTypeIdPointer) {
op2 = ir_analyze_optional_value_payload_value(ira, &bin_op_instruction->base.base,
op2, bin_op_instruction->safety_check_on);
if (type_is_invalid(op2->value->type))
return ira->codegen->invalid_inst_gen;
} else {
op2->value = op2->value->data.x_optional;
}

repeat = true;
}

if (!repeat) break;
} else for (;;) {
bool repeat = false;

if (op1->value->type->id == ZigTypeIdOptional) {
IrInstGen *op1_ref = ir_get_ref(ira, &op1->base, op1, true, false);
ZigType *op1_child_ptr_type = get_pointer_to_type_extra(ira->codegen,
op1->value->type->data.maybe.child_type, true, false,
PtrLenSingle, 0, 0, 0, false);
IrInstGen *op1_unwrap_ptr = ir_build_optional_unwrap_ptr_gen(ira, &op1->base, op1_ref,
false, false, op1_child_ptr_type);
op1 = ir_get_deref(ira, &op1->base, op1_unwrap_ptr, nullptr);
repeat = true;
}

if (op2->value->type->id == ZigTypeIdOptional) {
IrInstGen *op2_ref = ir_get_ref(ira, &op2->base, op2, true, false);
ZigType *op2_child_ptr_type = get_pointer_to_type_extra(ira->codegen,
op2->value->type->data.maybe.child_type, true, false,
PtrLenSingle, 0, 0, 0, false);
IrInstGen *op2_unwrap_ptr = ir_build_optional_unwrap_ptr_gen(ira, &op2->base, op2_ref,
false, false, op2_child_ptr_type);
op2 = ir_get_deref(ira, &op2->base, op2_unwrap_ptr, nullptr);
repeat = true;
}

if (!repeat) break;
}
}

if (is_equality_cmp && op1->value->type->id == ZigTypeIdNull && op2->value->type->id == ZigTypeIdNull) {
return ir_const_bool(ira, &bin_op_instruction->base.base, (op_id == IrBinOpCmpEq));
} else if (is_equality_cmp &&
Expand Down
82 changes: 82 additions & 0 deletions test/stage1/behavior/optional.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -198,3 +198,85 @@ test "0-bit child type coerced to optional" {
S.doTheTest();
comptime S.doTheTest();
}

test "unwrap equality operands" {
const S = struct {
fn valueTest() void {
var i: u32 = 11;
var oi: ?u32 = 11;
var ooi: ??u32 = 11;

var j: u32 = 11;
var oj: ?u32 = 11;
var ooj: ??u32 = 11;

var k: u32 = 22;
var ok: ?u32 = 22;
var ook: ??u32 = 22;

expect(i == oj);
expect(i == ooj);
expect(oj == i);
expect(ooj == i);
expect(oi == oj);
expect(oi == ooj);
expect(ooi == ooj);

expect(i != ok);
expect(i != ook);
expect(ok != i);
expect(ook != i);
expect(oi != ok);
expect(oi != ook);
expect(ooi != ook);

var oin: ?u32 = null;
var ooin: ??u32 = null;
var ojn: ?u32 = null;
var oojn: ??u32 = null;

expect(oi != ojn);
expect(oin == ojn);
expect(oj != oin);
expect(ojn == oin);

expect(i == 11);
expect(11 == i);
expect(oi == 11);
expect(11 == oi);
expect(ooi == 11);
expect(11 == ooi);

expect(i != 22);
expect(22 != i);
expect(oi != 22);
expect(22 != oi);
expect(ooi != 22);
expect(22 != ooi);
}

fn pointerTest() void {
var storage_a: u32 = 1;
var storage_b: u32 = 2;

var a: *u32 = &storage_a;
var oa: ?*u32 = &storage_a;
var oan: ?*u32 = null;

var b: *u32 = &storage_b;
var ob: ?*u32 = &storage_b;
var obn: ?*u32 = null;

expect(a == oa);
expect(a != ob);
expect(oa == a);
expect(ob != a);
expect(oa != ob);
expect(ob != oa);
}
};
S.valueTest();
S.pointerTest();
comptime S.valueTest();
comptime S.pointerTest();
}

2 comments on commit baf161e

@foobles
Copy link

@foobles foobles commented on baf161e Apr 19, 2020 •
edited

Choose a reason for hiding this comment

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

Right here:

            if (op1->value->type->id == ZigTypeIdOptional) {
                IrInstGen *op1_ref = ir_get_ref(ira, &op1->base, op1, true, false);
                ZigType *op1_child_ptr_type = get_pointer_to_type_extra(ira->codegen,
                        op1->value->type->data.maybe.child_type, true, false,
                        PtrLenSingle, 0, 0, 0, false);
                IrInstGen *op1_unwrap_ptr = ir_build_optional_unwrap_ptr_gen(ira, &op1->base, op1_ref,
                    false, false, op1_child_ptr_type);
                op1 = ir_get_deref(ira, &op1->base, op1_unwrap_ptr, nullptr);
                repeat = true;
            }

I think this is a mistake. You are unwrapping without checking if a value actually exists. It's necessary to generate some control flow around this for null checking, which after talking to andrew would be best accomplished by creating a new type of IrInstGen just for comparing optionals. I'm not sure why this passes, but it is definitely incorrect. I think the error you were getting with ir_analyze_optional_value_payload_value was actually telling you the right thing.

@mikdusan
Copy link
Owner Author

@mikdusan mikdusan commented on baf161e Apr 19, 2020 •
edited

Choose a reason for hiding this comment

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

thanks for catching this! I had a verbose-ir dump of desired behaviour showing need for runtime branch before but somehow lost it.

Please sign in to comment.