remove builtin @minValue and @maxValue and replace with stdlib implementation; #1476

kristate · 2018-09-05T12:45:11Z

Remove builtin @minValue and @maxValue from stage1
Replace all instances of @minValue and @maxValue with std.math.minInt and std.math.maxInt
update / include tests

Hejsil · 2018-09-05T13:03:19Z

std/math/index.zig

+            i64 => -9223372036854775808,
+            i128 => -170141183460469231731687303715884105728,
+            else => {
+                //Calculate for Integers that we do not have cached


I don't think there is any reason for this explicit caching since the compiler already does this.

What does "the compiler already does this" mean here? we are in comptime, so obviously we are not caching at runtime, but I thought that even at comptime, having a switch for common types could speed-up compile times.

Well, each instantiation of min/maxInt only gets its body comptime evaluated once and then cached. This is how types such as ArrayList(u8) is the same type each time it is called. Isn't the point of comptime stuff that we can avoid hardcoding our comptime generated tables and such? :)

@Hejsil I didn't realize that all return values of comptime were cached. Is that defined somewhere in the documentation? I was aware of using comptime to create lookup tables, etc for crc32 [0].

I did a test calling std.math.maxInt(u32) repeatedly and it did cache, so I will remove the switch.

Hejsil · 2018-09-05T13:06:10Z

std/math/index.zig

+                return switch (@typeId(T)) {
+                  // - (1 << (T.bit_count - 1))
+                  TypeId.Int, TypeId.ComptimeInt => if (T.is_signed) -(1 << (T.bit_count - 1)) else 0,
+                  else => @compileError("minValue not implemented for " ++ @typeName(T)),


Should this function also find the max value of an enum?
Use case: User made enum arrays

I'm thinking it should probably just be std.maxInt and std.minInt. max/min values of enums can be separate functions.

Okay, I will change the function name.

Hejsil · 2018-09-05T13:07:54Z

std/math/index.zig

@@ -380,7 +456,7 @@ pub fn absInt(x: var) !@typeOf(x) {
    comptime assert(@typeId(T) == builtin.TypeId.Int); // must pass an integer to absInt
    comptime assert(T.is_signed); // must pass a signed integer to absInt

-    if (x == @minValue(@typeOf(x))) {
+    if (x == std.math.minValue(@typeOf(x))) {


We are already in the math namespace here, so no need for std.math

Hejsil · 2018-09-05T13:09:20Z

test/cases/misc.zig

-    assert(@maxValue(u16) == 65535);
-    assert(@maxValue(u32) == 4294967295);
-    assert(@maxValue(u64) == 18446744073709551615);
+test "std.math.minValue and std.math.maxValue" {


This should probably exist in std.math. I'm pretty sure these tests test language features.

I thought so too, but I am glad that everyone agrees -- I will move it over to std.math

Hejsil · 2018-09-05T13:10:19Z

test/compile_errors.zig

@@ -1,6 +1,24 @@
 const tests = @import("tests.zig");

 pub fn addCases(cases: *tests.CompileErrorContext) void {
+    cases.add(


Pretty sure this tests compiler errors for compiler features, so these should not be here anymore.

std.math.minValue and friends emits a @compileError for using types that are not an Integer -- this was supposed to test that, but where would be the better place?

I don't think std tests for compiler errors for any of its functions. We probably just have to wait for some feature that will allow for this (I feel like I've seen an issue for it somewhere, but have only found #1356 and #567)

Hejsil · 2018-09-05T15:40:51Z

std/math/index.zig

                };
            },
        };
    }
 }

+test "std.math.minInt and std.math.maxInt" {
+    assert(std.math.maxInt(u1) == 1);


You missed removing std.math here when copy-pasting :)

kristate · 2018-09-06T02:51:09Z

Removed cached values and rebased.

tiehuis · 2018-09-06T05:25:35Z

Could you rebase all the renames across files into one commit? It helps when going through commit logs where I think its better to see that the commit intent is highlighted rather than the individual files, which we have the diff for anyway. Thanks.

kristate · 2018-09-06T07:20:33Z

@tiehuis collapsed into 640105a

tgschultz · 2018-09-07T16:13:06Z

std/math/index.zig

@@ -246,30 +246,10 @@ test "math.max" {
 /// The result is a compile time constant.
 pub fn minInt(comptime T: type) @typeOf(42) {


@typeOf(42) can just be comptime_int now.

kristate · 2018-09-15T07:43:55Z

Going to rebase this after 0.3.0 lands.

andrewrk

Squash commits please

ref ziglang#1466

… have cached;

… or max value for a non-integer;

Hejsil reviewed Sep 5, 2018

View reviewed changes

andrewrk added this to the 0.4.0 milestone Sep 5, 2018

Hejsil reviewed Sep 5, 2018

View reviewed changes

kristate force-pushed the minmax-tostdlib-issue1466 branch 2 times, most recently from b5e4a9d to 01f5133 Compare September 6, 2018 02:50

kristate force-pushed the minmax-tostdlib-issue1466 branch from 01f5133 to ee28327 Compare September 6, 2018 07:19

tgschultz reviewed Sep 7, 2018

View reviewed changes

kristate force-pushed the minmax-tostdlib-issue1466 branch 4 times, most recently from be25229 to b40ebc4 Compare September 13, 2018 15:36

andrewrk requested changes Sep 24, 2018

View reviewed changes

kristopher tate added 14 commits October 10, 2018 19:01

std/math/index.zig: @minValue implementation in stdlib;

ebd5b28

ref ziglang#1466

std/math/index.zig: @MaxValue implementation in stdlib;

3252bcb

ref ziglang#1466

src/all_types.hpp: remove @minValue and @MaxValue; ref ziglang#1466;

83c8775

src/codegen.cpp: remove @minValue and @MaxValue; ref ziglang#1466;

45f8fc4

src/ir.cpp: remove @minValue and @MaxValue; ref ziglang#1466;

dc9fc9a

src/ir_print.cpp: remove @minValue and @MaxValue; ref ziglang#1466;

7ea6f93

src/ir.cpp: remove ir_analyze_min_max;

a935d81

std/math/index.zig: minValue and maxValue for Integers that we do not…

463f637

… have cached;

std: builtin @{min,max}Value to std.math.{min,max}Value;

e8e1fff

std/math/index.zig: fix typo in minValue;

7bc9f82

test/compile_errors.zig: add test cases for attempting to get the min…

eea90e6

… or max value for a non-integer;

std/math/index.zig: change definition of {min,max}Value to {min,max}Int;

95191a7

std/math/index.zig: already inside of std.math namespace;

c33f7b4

{min,max}Value to {min,max}Int;

20fa3c5

kristopher tate added 6 commits October 10, 2018 19:31

tests: {min,max}Int tests from test/cases/misc.zig to std.math;

6086ddf

std/math/index.zig: remove cached values because comptime caches for us;

caf7fc1

std/math/index.zig: @typeof(42) to comptime_int;

f452652

doc/langref.html.in: remove @minValue and @MaxValue from docs;

e894685

doc/langref.html.in: builtin @{min,max}Value to std.math.{min,max}Int;

50e7de8

std.rand: builtin @{min,max}Value to std.math.{min,max}Value;

5302839

kristate force-pushed the minmax-tostdlib-issue1466 branch from b40ebc4 to 5302839 Compare October 10, 2018 10:39

test/cases/cast.zig: builtin @{min,max}Value to std.math.{min,max}Value;

ef1ed41

andrewrk closed this in 2b395d4 Oct 26, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

remove builtin @minValue and @maxValue and replace with stdlib implementation; #1476

remove builtin @minValue and @maxValue and replace with stdlib implementation; #1476

kristate commented Sep 5, 2018 •

edited

Loading

Hejsil Sep 5, 2018

kristate Sep 5, 2018 •

edited

Loading

Hejsil Sep 5, 2018

kristate Sep 6, 2018

Hejsil Sep 5, 2018

andrewrk Sep 5, 2018

kristate Sep 5, 2018

Hejsil Sep 5, 2018

Hejsil Sep 5, 2018

kristate Sep 5, 2018

Hejsil Sep 5, 2018

kristate Sep 5, 2018

Hejsil Sep 5, 2018

Hejsil Sep 5, 2018

kristate commented Sep 6, 2018

tiehuis commented Sep 6, 2018

kristate commented Sep 6, 2018

tgschultz Sep 7, 2018

kristate Sep 7, 2018

kristate commented Sep 15, 2018

andrewrk left a comment

		@@ -246,30 +246,10 @@ test "math.max" {
		/// The result is a compile time constant.
		pub fn minInt(comptime T: type) @typeOf(42) {

remove builtin @minValue and @maxValue and replace with stdlib implementation; #1476

remove builtin @minValue and @maxValue and replace with stdlib implementation; #1476

Conversation

kristate commented Sep 5, 2018 • edited Loading

Choose a reason for hiding this comment

kristate Sep 5, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kristate commented Sep 6, 2018

tiehuis commented Sep 6, 2018

kristate commented Sep 6, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kristate commented Sep 15, 2018

andrewrk left a comment

Choose a reason for hiding this comment

kristate commented Sep 5, 2018 •

edited

Loading

kristate Sep 5, 2018 •

edited

Loading