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

@src.file file name collisions #20963

Closed
leecannon opened this issue Aug 6, 2024 · 7 comments · Fixed by #20979
Closed

@src.file file name collisions #20963

leecannon opened this issue Aug 6, 2024 · 7 comments · Fixed by #20979
Labels
regression It worked in a previous version of Zig, but stopped working. use case Describes a real use case that is difficult or impossible, but does not propose a solution.
Milestone

Comments

@leecannon
Copy link
Contributor

leecannon commented Aug 6, 2024

Zig Version

0.14.0-dev.850+ddcb7b1c1

Steps to Reproduce and Observed Behavior

Files from different modules with the same relative path names from their respective root files have the same value for @src().file and therefore cannot be distinguished.

// build.zig
pub fn build(b: *std.Build) void {
    const exe = b.addExecutable(.{
        .name = "fff",
        .root_source_file = b.path("src/main.zig"),
        .target = b.standardTargetOptions(.{}),
        .optimize = b.standardOptimizeOption(.{}),
    });

    exe.root_module.addImport("dep1", b.createModule(.{
        .root_source_file = b.path("dep1/main.zig"),
    }));
    exe.root_module.addImport("dep2", b.createModule(.{
        .root_source_file = b.path("dep2/main.zig"),
    }));

    const run_cmd = b.addRunArtifact(exe);
    const run_step = b.step("run", "Run the app");
    run_step.dependOn(&run_cmd.step);
}

// src/main.zig
pub fn main() void {
    std.debug.print("{s}\n", .{@src().file});
    @import("dep1").printSrcFile();
    @import("dep2").printSrcFile();
}

// both dep1/main.zig and dep2/main are identical
pub fn printSrcFile() void {
    std.debug.print("{s}\n", .{@src().file});
}
$ zig build run
main.zig
main.zig
main.zig

Expected Behavior

In the context of a single compilation each file should have a unique @src().file.

@leecannon leecannon added the bug Observed behavior contradicts documented or intended behavior label Aug 6, 2024
@rohlem
Copy link
Contributor

rohlem commented Aug 6, 2024

Related discussion in #19570 , somewhat related also #20864 and #20941.
We could add an additional .module field to disambiguate, although iirc module names are currently local to the depender, and not unique across the build graph. Maybe the hash could work.
I think for every module that cares about this (so on an opt-in basis), a userland solution (defining a "module name" constant to include in a userland Src struct) should be possible.
I assume you have a use case for wanting to differentiate files across modules? Could you elaborate on your goal for compilation-unique @src values?

@andrewrk andrewrk removed the bug Observed behavior contradicts documented or intended behavior label Aug 7, 2024
@leecannon
Copy link
Contributor Author

leecannon commented Aug 7, 2024

One of the use cases of @src is for things like tracy.

With different files receiving the same path not only would tracy conflate traces from different source files it also has no way of locating the file from that path alone making it useless.

Although the zig compiler does not encounter an issue as it does not have imports of other modules with overlapping relative path names, its tracy bindings are susceptible to this issue:

.file = src.file.ptr,

@VisenDev
Copy link

VisenDev commented Aug 7, 2024

I believe this conflict was discussed on the discord. Especially in the case of projects like dvui that us @src for getting unique src locations. I believe andrew agreed to expand the capabilities of @src to include module information

Screenshot 2024-08-07 at 10 45 41 AM

@mnemnion
Copy link

mnemnion commented Aug 7, 2024

I've made some comments in #20864 about this issue as well, which I'm linking to mostly because @andrewrk said the same thing there as in the screenshot.

To state what ohsnap needs in a goal-oriented way, it's the ability to use the output of @src() to find and open the source file. Any approach to that functionality works for me.

@zkburke
Copy link

zkburke commented Aug 7, 2024

I believe this conflict was discussed on the discord. Especially in the case of projects like dvui that us @src for getting unique src locations. I believe andrew agreed to expand the capabilities of @src to include module information
Screenshot 2024-08-07 at 10 45 41 AM

Just commenting here to say I was the one who initiated this discussion on the discord as I also have a use case for this. Any instrumentation library will need this (like tracy) and most immediate mode uis will use this as well. The usefulness of @src() is significantly neutered by allowing these name conflicts to occur. I'd be totally happy with a module field in SourceLocation as a minimum, but I don't want to repeat myself as I've mentioned this in the discord discussion. Just leaving my thoughts here for posterity.

@andrewrk andrewrk added this to the 0.14.0 milestone Aug 7, 2024
@andrewrk andrewrk added use case Describes a real use case that is difficult or impossible, but does not propose a solution. regression It worked in a previous version of Zig, but stopped working. labels Aug 7, 2024
andrewrk added a commit that referenced this issue Aug 7, 2024
@andrewrk
Copy link
Member

andrewrk commented Aug 8, 2024

Play with that a little and let me know what you think.

@mnemnion
Copy link

mnemnion commented Aug 8, 2024

Will do, having some trouble with the build (not a request for help, working the Discord angle there), but I'll try it out as soon as I can.

igor84 pushed a commit to igor84/zig that referenced this issue Aug 11, 2024
SammyJames pushed a commit to SammyJames/zig that referenced this issue Aug 13, 2024
Rexicon226 pushed a commit to Rexicon226/zig that referenced this issue Aug 13, 2024
richerfu pushed a commit to richerfu/zig that referenced this issue Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression It worked in a previous version of Zig, but stopped working. use case Describes a real use case that is difficult or impossible, but does not propose a solution.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants