Skip to content
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

Switch case with an inline if never evaluates to false #10349

Closed
musiKk opened this issue Dec 17, 2021 · 3 comments
Closed

Switch case with an inline if never evaluates to false #10349

musiKk opened this issue Dec 17, 2021 · 3 comments
Labels
bug Observed behavior contradicts documented or intended behavior miscompilation The compiler reports success but produces semantically incorrect code. stage1 The process of building from source via WebAssembly and the C backend.
Milestone

Comments

@musiKk
Copy link

musiKk commented Dec 17, 2021

Zig Version

0.9.0-dev.2004+1fa5a1959

Steps to Reproduce

Create a project with the following code in src/main.zig, then run with zig run src/main.zig.

const std = @import("std");

pub fn main() !void {
    var n: u8 = 0;
    while (n < 3) : (n += 1) {
        const r = switch (n) {
            0 => @as(u8, if (one() > 2) 10 else 20),
            1 => blk: {
                break :blk @as(u8, if (one() > 2) 10 else 20);
            },
            2 => blk: {
                const v = @as(u8, if (one() > 2) 10 else 20);
                break :blk v;
            },
            else => unreachable
        };
        std.debug.print("{}\n", .{r});
    }

}

fn one() u8 {
    return 1;
}

Expected Behavior

I expect the output

20
20
20

Actual Behavior

The program prints instead

10
10
20

This only happens if n is var. If n is a const value, it works as expected. Also the if expression needs to be evaluated at runtime, that's why there is a one() function.

The third case is my workaround. I don't know if there is anything better.

@musiKk musiKk added the bug Observed behavior contradicts documented or intended behavior label Dec 17, 2021
@SpexGuy SpexGuy added miscompilation The compiler reports success but produces semantically incorrect code. stage1 The process of building from source via WebAssembly and the C backend. labels Dec 17, 2021
@SpexGuy
Copy link
Contributor

SpexGuy commented Dec 17, 2021

It looks like the compiler generates the correct branch, but then both paths jump to a basic block (EndIf) that overwrites the value with 10.

define void @foo() #1 !dbg !2680 {
Entry:
  %n = alloca i8, align 1
  %v = alloca i8, align 1
  %r = alloca i8, align 1
  store i8 0, i8* %n, align 1, !dbg !2688
  br label %WhileCond, !dbg !2689

WhileCond:                                        ; preds = %OverflowOk, %Entry
  %0 = load i8, i8* %n, align 1, !dbg !2690
  %1 = icmp ult i8 %0, 3, !dbg !2691
  br i1 %1, label %WhileBody, label %WhileEnd, !dbg !2691

WhileBody:                                        ; preds = %WhileCond
  %2 = load i8, i8* %n, align 1, !dbg !2692
  switch i8 %2, label %SwitchElse [
    i8 0, label %SwitchProng
    i8 1, label %SwitchProng1
    i8 2, label %SwitchProng2
  ], !dbg !2692

SwitchElse:                                       ; preds = %WhileBody
  call fastcc void @panic(%"[]u8"* @71, %std.builtin.StackTrace* null), !dbg !2693
  unreachable, !dbg !2693

SwitchProng:                                      ; preds = %WhileBody
  %3 = call fastcc i8 @one(), !dbg !2694
  %4 = icmp ugt i8 %3, 2, !dbg !2695
  br i1 %4, label %Then3, label %Else8, !dbg !2696

SwitchProng1:                                     ; preds = %WhileBody
  %5 = call fastcc i8 @one(), !dbg !2697
  %6 = icmp ugt i8 %5, 2, !dbg !2699
  br i1 %6, label %Then4, label %Else9, !dbg !2700

SwitchProng2:                                     ; preds = %WhileBody
  %7 = call fastcc i8 @one(), !dbg !2701
  %8 = icmp ugt i8 %7, 2, !dbg !2702
  br i1 %8, label %Then, label %Else, !dbg !2703

Then:                                             ; preds = %SwitchProng2
  store i8 10, i8* %v, align 1, !dbg !2704
  br label %EndIf6, !dbg !2703

Else:                                             ; preds = %SwitchProng2
  store i8 20, i8* %v, align 1, !dbg !2705
  br label %EndIf6, !dbg !2703

Then3:                                            ; preds = %SwitchProng
  store i8 10, i8* %r, align 1, !dbg !2706
  br label %EndIf, !dbg !2696

EndIf:                                            ; preds = %Else8, %Then3
  store i8 10, i8* %r, align 1, !dbg !2707 ; bug here! This follows after Else8
  br label %SwitchEnd, !dbg !2692

Then4:                                            ; preds = %SwitchProng1
  store i8 10, i8* %r, align 1, !dbg !2708
  br label %EndIf5, !dbg !2700

EndIf5:                                           ; preds = %Else9, %Then4
  store i8 10, i8* %r, align 1, !dbg !2709
  br label %BlockEnd, !dbg !2710

BlockEnd:                                         ; preds = %EndIf5
  store i8 10, i8* %r, align 1, !dbg !2711
  br label %SwitchEnd, !dbg !2692

EndIf6:                                           ; preds = %Else, %Then
  call void @llvm.dbg.declare(metadata i8* %v, metadata !2684, metadata !DIExpression()), !dbg !2712
  %9 = load i8, i8* %v, align 1, !dbg !2713
  store i8 %9, i8* %r, align 1, !dbg !2713
  br label %BlockEnd7, !dbg !2714

BlockEnd7:                                        ; preds = %EndIf6
  store i8 %9, i8* %r, align 1, !dbg !2715
  br label %SwitchEnd, !dbg !2692

SwitchEnd:                                        ; preds = %BlockEnd7, %BlockEnd, %EndIf
  call void @llvm.dbg.declare(metadata i8* %r, metadata !2687, metadata !DIExpression()), !dbg !2716
  %10 = load i8, i8* %r, align 1, !dbg !2717
  call void @clobber(i8 %10), !dbg !2718
  %11 = load i8, i8* %n, align 1, !dbg !2719
  %12 = call { i8, i1 } @llvm.uadd.with.overflow.i8(i8 %11, i8 1), !dbg !2720
  %13 = extractvalue { i8, i1 } %12, 0, !dbg !2720
  %14 = extractvalue { i8, i1 } %12, 1, !dbg !2720
  br i1 %14, label %OverflowFail, label %OverflowOk, !dbg !2720

WhileEnd:                                         ; preds = %WhileCond
  ret void, !dbg !2721

Else8:                                            ; preds = %SwitchProng
  store i8 20, i8* %r, align 1, !dbg !2722
  br label %EndIf, !dbg !2696

Else9:                                            ; preds = %SwitchProng1
  store i8 20, i8* %r, align 1, !dbg !2723
  br label %EndIf5, !dbg !2700

OverflowFail:                                     ; preds = %SwitchEnd
  call fastcc void @panic(%"[]u8"* @73, %std.builtin.StackTrace* null), !dbg !2720
  unreachable, !dbg !2720

OverflowOk:                                       ; preds = %SwitchEnd
  store i8 %13, i8* %n, align 1, !dbg !2720
  br label %WhileCond, !dbg !2689
}

@andrewrk andrewrk added this to the 0.11.0 milestone Dec 18, 2021
@FiniteNick
Copy link

Zig version
0.10.0-dev.390+0866fa9d1

I've found another case for this when creating anonymous struct literals:

const std = @import("std");

pub fn main() anyerror!void {}

fn one() u8 {
    return 1;
}

test "basic test" {
    const a = @as(u32, if (one() > 2) 1 else 2);

    const b = .{
        .val = @as(u32, if (one() > 2) 1 else 2),
    };

    const c = .{
        .val = blk: {
            const ret = @as(u32, if (one() > 2) 1 else 2);
            break :blk ret;
        },
    };

    try std.testing.expectEqual(a, 2);
    try std.testing.expectEqual(b.val, 2); // *********** Fails ***********
    try std.testing.expectEqual(c.val, 2);
}

@Vexu
Copy link
Member

Vexu commented Feb 2, 2023

Works in self-hosted, tested in test/behavior/eval.zig:test "if inside a switch" {-

@Vexu Vexu closed this as completed Feb 2, 2023
@andrewrk andrewrk modified the milestones: 0.13.0, 0.11.0 Feb 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Observed behavior contradicts documented or intended behavior miscompilation The compiler reports success but produces semantically incorrect code. stage1 The process of building from source via WebAssembly and the C backend.
Projects
None yet
Development

No branches or pull requests

5 participants