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

Error for name is not marked 'pub' is misleading/incomplete #20461

Open
insalt-glitch opened this issue Jun 30, 2024 · 2 comments
Open

Error for name is not marked 'pub' is misleading/incomplete #20461

insalt-glitch opened this issue Jun 30, 2024 · 2 comments
Labels
error message This issue points out an error message that is unhelpful and should be improved.
Milestone

Comments

@insalt-glitch
Copy link

Zig Version

0.13.0

Steps to Reproduce and Observed Output

Consider the following combination scenario:
Main.zig:

const Point2D = @import("Point.zig").Point;
const PointCloud = @import("PointCloud.zig");

pub fn main() !void {
    _ = PointCloud{
        .points = .{
            .{ .x = 1.0, .y = 0.0 },
            .{ .x = 0.0, .y = 1.0 },
        }
    };
}

Point.zig:

pub const Point = struct { x: f64, y: f64 };

PointCloud.zig

const Point = @import("Main.zig").Point2D;

points: []Point

This produces the (in my opinion) misleading/incomplete error message PointCloud.zig:1:34: error: 'Point2D' is not marked 'pub'.

Expected Output

The reason that I think it is misleading, is that the compiler knows the location where Point is defined and should suggest importing from Point.zig. I realize that it may still be useful to report the fact that Point is imported into Main.zig but is not marked pub there, but it most situations you probably want to import from the original file.

In my opinion, the expected output could read something like:
PointCloud.zig:1:34: error: The imported variable 'Point2D' at 'Main.zig:1.1' is not marked 'pub'. Consider importing the original definition at 'Point.zig:1:1'.

@insalt-glitch insalt-glitch added the error message This issue points out an error message that is unhelpful and should be improved. label Jun 30, 2024
@rohlem
Copy link
Contributor

rohlem commented Jun 30, 2024

i[n] most situations you probably want to import from the original file.

I don't think I personally agree:
Every step along an alias trace may hold some significance, or even include some comptime selection logic. Directly importing from the original file might be the wrong choice in those cases.
I wouldn't mind the suggested alternative message for the given example, however this can quickly become less and less readable the longer / more complex the trace gets. Therefore I'm not sure it's preferable in the general case.
(I generally feel like my preferred tooling for tracing this chain back to the origin would be an IDE, and not a purely textual compile error message, though I realize having neither right now is even less ideal.
EDIT: compiler notes (note: entries) would be more fitting for this type of use case though, rather than expanding on the initial error message.)

@insalt-glitch
Copy link
Author

insalt-glitch commented Jul 4, 2024

I actually agree. Maybe it would be nice to just have an additional note: The original definition of 'Point2D' is located at 'Point.zig:1:1'. or even the complete trace? I think the latter might be too verbose. What do you think?

@Vexu Vexu added this to the 1.0.0 milestone Jul 4, 2024
@insalt-glitch insalt-glitch changed the title Error for name is not marked 'pub' is misleading/impcomplete Error for name is not marked 'pub' is misleading/incomplete Jul 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
error message This issue points out an error message that is unhelpful and should be improved.
Projects
None yet
Development

No branches or pull requests

3 participants