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

Sema: fix wording in error message #21944

Merged
merged 1 commit into from
Nov 9, 2024
Merged

Conversation

wooster0
Copy link
Contributor

@wooster0 wooster0 commented Nov 9, 2024

I think omitting the .zig file extension here is bad for several reasons,
but one is that if you have a directory with two empty files called "x.zig" and
"x.zig.zig" and then compile x.zig.zig, it says this:

~/lib/std/start.zig:604:46: error: root struct of file 'x.zig' has no member named 'main'
    const ReturnType = @typeinfo(@typeof(root.main)).@"fn".return_type.?;
                                         ~~~~^~~~~
x.zig.zig:1:1: note: struct declared here

^

It talks about x.zig in the message, but then x.zig.zig in the note below.
That, even though it never talks about the actual x.zig which is in the same
directory. This is confusing.

This is just the file name and I considered making this the full path to the file
but I think for this the testing infrastructure for compile errors has to be
improved because if it's the full path it might contain random temporary files in the
error message which is untestable. Might not be a problem though.
The other reason is that the full path would already be shown in the "struct declared here"
note which should follow below.

@mlugg
Copy link
Member

mlugg commented Nov 9, 2024

This is worse than status quo, because this brings us to an awkward middle-ground of "real file path" and "filename-based FQN". If you trigger this error message for a file in a subdirectory of your root source dir, you can see that it's using the "FQN", i.e. slashes are replaced with dots, e.g.:

main.zig:2:32: error: root struct of file 'test.foo' has no member named 'bar'
test/foo.zig:1:1: note: struct declared here

So, with this patch, that first line would say error: root struct of file 'test.foo.zig' has no member named 'bar', which is clearly bad.

We should go one way or the other on this. Personally, I prefer status quo and do not find your argument compelling (you shouldn't be naming files foo.zig.zig in the first place!), but wouldn't lose sleep if we went with filenames.

EDIT: I do think that under status quo we should probably reword the error message to file root struct 'x' has no member named 'main', since yeah, the existing message does strongly imply that it's giving you a real file path there.

@wooster0
Copy link
Contributor Author

wooster0 commented Nov 9, 2024

Well I meant this to be a real file name (just the name not the full path or some FQN-based thing).
Do you think using std.fs.path.basename(pt.root_mod.root_src_path) would work well? Or is there a better way?

It's an FQN, not an actual file name.
@wooster0
Copy link
Contributor Author

wooster0 commented Nov 9, 2024

I'm fine with your version though

@wooster0 wooster0 changed the title Sema: add missing file extension in error message Sema: fix wording in error message Nov 9, 2024
@mlugg
Copy link
Member

mlugg commented Nov 9, 2024

Okay, yeah, this update to just use the alternative wording is an improvement. Thanks!

@mlugg mlugg enabled auto-merge (rebase) November 9, 2024 16:56
@mlugg mlugg merged commit 35201e9 into ziglang:master Nov 9, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants