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

Consider if @src() in an inline function should refer to the caller's location #21906

Open
matklad opened this issue Nov 4, 2024 · 11 comments
Open
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.

Comments

@matklad
Copy link
Contributor

matklad commented Nov 4, 2024

Currently, @src() returns source it's own location, even in inline functions:

const std = @import("std");

inline 
fn f(a: std.builtin.SourceLocation) void {
    const b = @src();
    inline for (.{ a, b }, .{ "a", "b" }) |location, name| {
        std.debug.print("{s} = {s}:{}:{}\n", .{
            name,
            location.file,
            location.line,
            location.column,
        });
    }
}

pub fn main() void {
    f(@src());
}
a = main.zig:16:7
b = main.zig:4:15

Given that inline functions are inlined semantically, it is theoretically possible to also provide SourceLocation of the call-site. There are several ways to handle this:

  • Just change @src() to always be caller's location in an inline function
  • Provide @callerSrc() which is a syntax error if called outside of inline fn
  • Add caller: ?*SourceLocation field to std.builtin.SourceLocation which is non-null iff @src() is inside a function. This allows for having several levels of ancestral @src() if an inline function calls another inline function.
  • Maybe something else?

If implemented, this feature could enable several patterns. For example, there's a Rust GUI framework that uses equivalent feature to implement Flutter-like memorization: https://github.com/anp/moxie/blob/4f71b6f28340b2db263f07d0883394fa0b233de0/topo/src/lib.rs#L140-L192

Specific use-case I am interested in is this:

pub inline fn assert(condition: bool) {
    const src = @callerSrc();
    if (!condition) {
        std.debug.panic("assertion failure: {s}:{}:{}", .{src.file, src.line, src.column})
    }
}

That is, I want to write an assert function which prints the location of the failure even if the code is compiled with Release, and all debug info is stripped.

@BratishkaErik
Copy link
Contributor

Sounds like alternative to #12093 (comment) and gwenzek#5

@mlugg
Copy link
Member

mlugg commented Nov 4, 2024

Note that this is a slight inconvenience for incremental, since we would have to further complicate the dependency mechanism to track dependencies on source location, whereas with status-quo behavior this is avoided by lumping this information into the ZIR source hash when @src() is used.

To be honest, I also prefer status quo here from a language design perspective. I think having to pass @src() as a parameter isn't a big deal but is notably clearer. Also, I suspect this proposal would encourage even more misuse of inline fn than we see today (most Zig users should simply never be writing inline fn!).

@mlugg mlugg added the proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. label Nov 4, 2024
@matklad
Copy link
Contributor Author

matklad commented Nov 4, 2024

I think having to pass @src() as a parameter isn't a big deal but is notably clearer

I am of two minds here! I definitely don't mind passing @src once in a while for things like snapshot tests here:

https://github.com/tigerbeetle/tigerbeetle/blob/ac571a653197a68ac6c7138900d88ed39da977c2/src/scripts/cfo.zig#L724-L739

At the same time, passing @src() to every assert in here would feel like a lot of needless duplication:

https://github.com/tigerbeetle/tigerbeetle/blob/ac571a653197a68ac6c7138900d88ed39da977c2/src/vsr/replica.zig#L4379-L4399

@accelbread
Copy link
Contributor

This would also be useful for logging. In C codebases I work with, my logging macros print file+line info of the logging site, but my Zig projects lack that since I don't want to pass @src for every logging line.

@zxcv05
Copy link

zxcv05 commented Nov 10, 2024

I think having @callerSrc() would be useful even outside of inline functions (for error handling and logging) and that if added shouldn't be exclusive to inline functions

@alexrp
Copy link
Member

alexrp commented Nov 10, 2024

I think having @callerSrc() would be useful even outside of inline functions (for error handling and logging) and that if added shouldn't be exclusive to inline functions

How would that work for a non-inline function?

@zxcv05
Copy link

zxcv05 commented Nov 10, 2024

setting @callerSrc() to @src() before entering a function (in theory atleast)

@alexrp
Copy link
Member

alexrp commented Nov 10, 2024

Having the language emit a hidden thread-local variable is a non-starter, especially for freestanding.

@zxcv05
Copy link

zxcv05 commented Nov 10, 2024

I dont imagine this would happen at runtime but rather be tracked during compile time

@alexrp
Copy link
Member

alexrp commented Nov 10, 2024

A non-inline function can be called from multiple places. You have to pass the caller location information to it somehow.

@accelbread
Copy link
Contributor

I don't think support for non-inline function's makes sense. If caller info is wanted for an assert/logging function that you dont want to inline, that could be done with a non-inlined implementation taking a src info parameter and an inline wrapper that passes the caller src info, as follows:

inline fn assertEq(expected: anytype, value: @TypeOf(expected)) void {
    assertEqImpl(@callerSrc(), expected, value);
}

fn assertEqImpl(
    src_info: @TypeOf(@src()),
    expected: anytype,
    value: @TypeOf(expected),
) void {
    // do assert stuff
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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