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

Crash with switch inside inline for #3136

Closed
Sahnvour opened this issue Aug 28, 2019 · 8 comments
Closed

Crash with switch inside inline for #3136

Sahnvour opened this issue Aug 28, 2019 · 8 comments
Labels
bug Observed behavior contradicts documented or intended behavior stage1 The process of building from source via WebAssembly and the C backend.
Milestone

Comments

@Sahnvour
Copy link
Contributor

Segfaults during LLVM codegen.

Reduced repro:

pub fn float() f32 {
    return 0.2;
}

pub fn foo(comptime args: var) void {
    inline for (args) |a| {
        switch (float()) {
            0.0...100.0 => {},
            else => {},
        }
    }
}

test "foo" {
    foo([_]u32{ 20, 30, 57 });
}

This worked some time ago, does on 0.4.0 and still does with the trunk version available on godbolt.

@Sahnvour Sahnvour added the bug Observed behavior contradicts documented or intended behavior label Aug 28, 2019
@Sahnvour Sahnvour added this to the 0.5.0 milestone Aug 28, 2019
@Sahnvour Sahnvour changed the title Crash with switch on float Crash with switch on float inside inline for Aug 28, 2019
@FireFox317
Copy link
Contributor

I think it shouldn't be possible to switch on a float, because internally a float is always an approximation. Also the C standard prohibits the use of a float inside a switch expression.

Thus I think this should add a compile error when a float is used inside a switch expression.

@SamTebbs33
Copy link
Contributor

SamTebbs33 commented Aug 29, 2019

This should be a compiler error for floats, but the issue is reproduced even with an i32.

pub fn float() i32 {
    return 2;
}

pub fn foo(comptime args: var) void {
    inline for (args) |a| {
        switch (float()) {
            0...1 => {},
            else => {},
        }
    }
}

test "foo" {
    foo([_]u32{ 20, 30, 57 });
}
#0  0x0000000004afbb50 in llvm::BasicBlock::getTerminator() const ()
#1  0x0000000004b62073 in llvm::DomTreeBuilder::SemiNCAInfo<llvm::DominatorTreeBase<llvm::BasicBlock, true> >::ChildrenGetter<false>::Get(llvm::BasicBlock*, std::integral_constant<bool, false>) [clone .isra.343] ()
#2  0x0000000004b687cc in llvm::DomTreeBuilder::SemiNCAInfo<llvm::DominatorTreeBase<llvm::BasicBlock, false> >::ChildrenGetter<false>::Get(llvm::BasicBlock*, llvm::DomTreeBuilder::SemiNCAInfo<llvm::DominatorTreeBase<llvm::BasicBlock, false> >::BatchUpdateInfo*) ()
#3  0x0000000004b6b962 in unsigned int llvm::DomTreeBuilder::SemiNCAInfo<llvm::DominatorTreeBase<llvm::BasicBlock, false> >::runDFS<false, bool (*)(llvm::BasicBlock*, llvm::BasicBlock*)>(llvm::BasicBlock*, unsigned int, bool (*)(llvm::BasicBlock*, llvm::BasicBlock*), unsigned int) [clone .constprop.397] ()
#4  0x0000000004b6bb88 in llvm::DomTreeBuilder::SemiNCAInfo<llvm::DominatorTreeBase<llvm::BasicBlock, false> >::CalculateFromScratch(llvm::DominatorTreeBase<llvm::BasicBlock, false>&, llvm::DomTreeBuilder::SemiNCAInfo<llvm::DominatorTreeBase<llvm::BasicBlock, false> >::BatchUpdateInfo*) ()

@raulgrell
Copy link
Contributor

raulgrell commented Aug 29, 2019

Switching on floats came up in an issue regarding switch range syntax #359. I don't think any consensus was reached.

@andrewrk andrewrk modified the milestones: 0.5.0, 0.6.0 Sep 26, 2019
@momumi
Copy link
Contributor

momumi commented Jan 9, 2020

This program also segfaults the compiler:

const std = @import("std");

pub fn main() anyerror!void {
    var bar: f32 = 1.0;

    const my_bool = switch (bar) {
        1 => true,
        0 => false,
        else => false,
    };
    std.debug.warn("{}\n", .{my_bool});
}

@LemonBoy
Copy link
Contributor

Your problem is similar to #4074, some custom kind of lowering must be implemented during the codegen phase as LLVM can only switch on integer types.
But I think that switching on a float should be forbidden at the Zig level too.

@andrewrk andrewrk added the stage1 The process of building from source via WebAssembly and the C backend. label Mar 4, 2020
@andrewrk andrewrk modified the milestones: 0.6.0, 0.7.0 Mar 4, 2020
@andrewrk andrewrk modified the milestones: 0.7.0, 0.8.0 Aug 13, 2020
@andrewrk andrewrk modified the milestones: 0.8.0, 0.9.0 Nov 6, 2020
@andrewrk andrewrk modified the milestones: 0.9.0, 0.10.0 May 19, 2021
@frmdstryr
Copy link
Contributor

frmdstryr commented Jun 27, 2021

Since llvm doesn't work with switching on non-ints (from scanning issues) would it make sense to have zig just convert switch on floats to an if / else if / else chain?

@Vexu Vexu changed the title Crash with switch on float inside inline for Crash with switch inside inline for Jun 27, 2021
@frmdstryr
Copy link
Contributor

To address the tolerance issue, maybe switching on a float should always require a range? If no range is given, compile throw a error.

Or the lazy me would want to add a tolerance to the switch, eg switch(v, 1e-3) or whatever.

@Vexu
Copy link
Member

Vexu commented Dec 28, 2022

Switching on floats has been disabled.

@Vexu Vexu closed this as completed Dec 28, 2022
@andrewrk andrewrk modified the milestones: 0.12.0, 0.11.0 Dec 28, 2022
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 stage1 The process of building from source via WebAssembly and the C backend.
Projects
None yet
Development

No branches or pull requests

9 participants