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

translate-c generates zig code with unnecessary _ = foo; statements for unused local variables and parameters #9205

Closed
Tracked by #9191
andrewrk opened this issue Jun 23, 2021 · 10 comments · Fixed by #9306
Labels
contributor friendly This issue is limited in scope and/or knowledge of Zig internals. enhancement Solving this issue will likely involve adding new logic or components to the codebase. translate-c C to Zig source translation feature (@cImport)
Milestone

Comments

@andrewrk
Copy link
Member

andrewrk commented Jun 23, 2021

void foo(void) {
    int x;
}

This code translates to:

pub export fn foo() void {
    var x: c_int = undefined;
}

If you try to compile it with self-hosted:

$ ./zig build-obj test.zig
test.zig:2:9: error: unused local variable
    var x: c_int = undefined;
        ^

Instead it should generate:

pub export fn foo() void {
    var x: c_int = undefined;
    _ = x;
}

However ideally it would keep track of which variables and parameters are used and not generate unnecessary _ = foo; statements.

This is blocking #9191.

@andrewrk andrewrk added enhancement Solving this issue will likely involve adding new logic or components to the codebase. contributor friendly This issue is limited in scope and/or knowledge of Zig internals. translate-c C to Zig source translation feature (@cImport) labels Jun 23, 2021
@andrewrk andrewrk added this to the 0.9.0 milestone Jun 23, 2021
@ehaas
Copy link
Sponsor Contributor

ehaas commented Jun 23, 2021

I think the _ = x; solution would be pretty quick to implement as an unblocker, then maybe we could circle back and make it smarter about not emitting those for used variables?

@andrewrk
Copy link
Member Author

Sounds good to me!

@ehaas
Copy link
Sponsor Contributor

ehaas commented Jun 23, 2021

Cool, can have that to you later tonight

@ehaas
Copy link
Sponsor Contributor

ehaas commented Jun 23, 2021

@andrewrk mostly done; however __USER_LABEL_PREFIX__ is causing a problem on this branch. It gets injected automatically by clang https://gcc.gnu.org/onlinedocs/cpp/Common-Predefined-Macros.html

It translates to: pub const __USER_LABEL_PREFIX__ = _;

Which causes

test.zig:273:35: error: '_' may not be used as an identifier
pub const __USER_LABEL_PREFIX__ = _;
                                  ^

Even when the symbol is not otherwise referred to.

Seems like I could either special case that particular macro (skip it) or look for ones that have a single _ identifier as the definition.

@ehaas
Copy link
Sponsor Contributor

ehaas commented Jun 23, 2021

Actually, one other issue which is the removal of packed/extern enums. I have a PR for that already - #9164 - should I bring that in as well?

@ehaas
Copy link
Sponsor Contributor

ehaas commented Jun 23, 2021

OK, PR for this one here: #9209

Assuming it looks good, I can rebase #9164 onto stage1-astcheck after this one gets merged.

@andrewrk
Copy link
Member Author

Seems like I could either special case that particular macro (skip it) or look for ones that have a single _ identifier as the definition.

The intended solution for this is to use @"_" to refer to an identifier named _. Let me know if you hit any bugs related to that.

@andrewrk andrewrk changed the title translate-c generates zig code with unused local variables and parameters translate-c generates zig code with unnecessary _ = foo; statements for unused local variables and parameters Jun 23, 2021
@andrewrk andrewrk modified the milestones: 0.9.0, 0.10.0 Jun 23, 2021
@ehaas
Copy link
Sponsor Contributor

ehaas commented Jun 24, 2021

The intended solution for this is to use @"_" to refer to an identifier named _. Let me know if you hit any bugs related to that.

Is that expected to work now? I get the same error for pub const FOO = @"_";

test.zig:1:17: error: '_' may not be used as an identifier
pub const FOO = @"_";
                ^

@andrewrk
Copy link
Member Author

andrewrk commented Jul 5, 2021

Is that expected to work now? I get the same error for pub const FOO = @"_";

Oops, forgot to answer this question. I believe I fixed it in abfee12.

@ehaas
Copy link
Sponsor Contributor

ehaas commented Jul 6, 2021

Is that expected to work now? I get the same error for pub const FOO = @"_";

Oops, forgot to answer this question. I believe I fixed it in abfee12.

I'm able to declare a public symbol with that commit (eg. pub const @"_": c_int = 5; works) however I'm not able to declare a variable:

pub export fn main() void {
    var @"_": c_int = 5;
    _ = @"_";
}
./test.zig:2:5: error: `_` is not a declarable symbol
    var @"_": c_int = 5;
    ^
./test.zig:3:9: error: `_` may only be used to assign things to
    _ = @"_";
        ^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor friendly This issue is limited in scope and/or knowledge of Zig internals. enhancement Solving this issue will likely involve adding new logic or components to the codebase. translate-c C to Zig source translation feature (@cImport)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants