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

compile error for modification of comptime data inside runtime block #1470

Closed
andrewrk opened this issue Sep 4, 2018 · 8 comments
Closed
Labels
accepted This proposal is planned. contributor friendly This issue is limited in scope and/or knowledge of Zig internals. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@andrewrk
Copy link
Member

andrewrk commented Sep 4, 2018

Inspired by #906

const std = @import("std");
const builtin = @import("builtin");
const debug = std.debug;

fn isSliceType(comptime T: type) bool {
    if (@typeInfo(T) == .Pointer) {
        if (@typeInfo(T).Pointer.size == .Slice) {
            return true;
        }
    }
    return false;
}

fn countSliceMembers(inp: anytype) usize {
    comptime var count = @as(usize, 0);
    const type_info = @typeInfo(@TypeOf(inp));

    inline for (type_info.Struct.fields) |field_info| {
        const FieldType = field_info.field_type;
        if (isSliceType(FieldType)) count += 1;
    }
    return count;
}

test "incorrectly caching... something" {
    const String = []const u8;
    const Number = u32;

    const S = struct {
        string: String,
        number: Number,
    };

    debug.assert(isSliceType(String) == true);
    debug.assert(isSliceType(Number) == false);

    var s = S{
        .string = "",
        .number = 10,
    };
    debug.assert(countSliceMembers(s) == 1);
}

count += 1 is modification of comptime data inside of a runtime block. It can have surprising results, as demonstrated by this test case. With this proposal, Zig would detect that count was declared outside the runtime block scope, and therefore this is illegal modification of comptime data.

@andrewrk andrewrk added the proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. label Sep 4, 2018
@andrewrk andrewrk added this to the 0.4.0 milestone Sep 4, 2018
@andrewrk andrewrk modified the milestones: 0.4.0, 0.5.0 Feb 15, 2019
@andrewrk andrewrk modified the milestones: 0.5.0, 0.6.0 Aug 28, 2019
@andrewrk andrewrk modified the milestones: 0.6.0, 0.7.0 Feb 10, 2020
@andrewrk andrewrk modified the milestones: 0.7.0, 0.8.0 Oct 10, 2020
@andrewrk andrewrk added the accepted This proposal is planned. label Dec 31, 2020
@SpexGuy
Copy link
Contributor

SpexGuy commented Jan 5, 2021

The specific decision here is that comptime vars should become immutable when inside a runtime branch. So for example:

fn foo() {
  comptime var x = 0; // x is mutable

  if (runtime_value) {
    comptime var y = 0;
    // y is mutable, x is immutable
  } else {
    // x is immutable
  }
  
  for (some_slice) |item, i| {
    // runtime branch, x is immutable
  }
  
  while (some_condition) {
    // runtime branch, x is immutable
  }
  
  switch (runtime_value) {
    else => {}, // runtime branch, x is immutable
  }
  
  if (comptime_value) {
    // comptime branch, x is mutable
  } else {
    // comptime branch, x is mutable
  }
  
  inline for (some_array_or_tuple) |item, i| {
    // not a runtime branch, x is mutable
  }
  
  inline while (some_comptime_condition) {
    //  not a runtime branch, x is mutable
  }
  
  switch (comptime_value) {
    else => {}, // not a runtime branch, x is mutable
  }
}

This is similar to how comptime vars become immutable when they go out of scope.

@daurnimator
Copy link
Contributor

when inside a runtime branch.

I don't think we've defined this before in the language: what is a "runtime branch"?

@SpexGuy
Copy link
Contributor

SpexGuy commented Jan 5, 2021

A scope which is compiled but may or may not be executed, or may be executed multiple times, depending on values which are not comptime known.

@andrewrk
Copy link
Member Author

andy@ark ~/tmp [1]> zig test test.zig
test.zig:20:43: error: store to comptime variable depends on runtime condition
        if (isSliceType(FieldType)) count += 1;
                                    ~~~~~~^~~~
test.zig:20:24: note: runtime condition here
        if (isSliceType(FieldType)) count += 1;
            ~~~~~~~~~~~^~~~~~~~~~~

this is implemented now; just need to verify that we have test coverage before closing

@nektro
Copy link
Contributor

nektro commented Aug 24, 2022

@andrewrk andrewrk modified the milestones: 0.10.1, 0.11.0 Jan 10, 2023
@Vexu
Copy link
Member

Vexu commented Jan 23, 2023

$ git grep "store to comptime variable depends on runtime condition" test
test/cases/compile_errors/comptime_store_in_comptime_switch_in_runtime_if.zig:// :13:25: error: store to comptime variable depends on runtime condition
test/cases/x86_64-linux/comptime_var.0.zig:// :4:19: error: store to comptime variable depends on runtime condition
test/cases/x86_64-linux/comptime_var.1.zig:// :6:19: error: store to comptime variable depends on runtime condition
test/cases/x86_64-macos/comptime_var.0.zig:// :4:19: error: store to comptime variable depends on runtime condition
test/cases/x86_64-macos/comptime_var.1.zig:// :6:19: error: store to comptime variable depends on runtime condition

@Vexu Vexu closed this as completed Jan 23, 2023
@Pyrolistical
Copy link
Contributor

Pyrolistical commented Mar 24, 2024

FYI if anybody runs into "error: store to comptime variable depends on runtime condition", but your runtime condition can be calculated at comptime, then wrap that section in a comptime block.

// BEFORE
inline for (type_info.Struct.fields) |field| {
    const FieldType = field.type;
    if (isSliceType(FieldType)) count += 1; // isSliceType is runtime condition
}

// AFTER
comptime {
    for (type_info.Struct.fields) |field| {
        const FieldType = field.type;
        if (isSliceType(FieldType)) count += 1; // now comptime
    }
}

In this example, it would be been simpler to inline isSliceType to make it comptime to begin with, but existing runtime std fns like std.mem.eql can be used purely in comptime if both params are comptime known.

@nektro
Copy link
Contributor

nektro commented Mar 24, 2024

you can also use comptime surgically instead of wrapping the whole block

 inline for (type_info.Struct.fields) |field| {
     const FieldType = field.type;
-    if (isSliceType(FieldType)) count += 1; // isSliceType is runtime condition
+    if (comptime isSliceType(FieldType)) count += 1; // isSliceType is runtime condition
 }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted This proposal is planned. contributor friendly This issue is limited in scope and/or knowledge of Zig internals. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Projects
None yet
Development

No branches or pull requests

6 participants