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

fix(cc): preprocessor doesn't emit bin #20740

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zhylmzr
Copy link
Contributor

@zhylmzr zhylmzr commented Jul 22, 2024

zig cc main.c -o /dev/null emit a.out, zig cc -E main.c emit main.o, this pr modifies the linking and preprocessing generation logic to be more consistent with clang's behavior.

When using clang -E main.c or clang -E main.c -o - outputs directly to standard output, if using clang -E main.c -o a it outputs to a file.

Fix the following issues:
fixed: #20739
fixed: #20689
fixed: #20359
fixed: #20709

@zhylmzr zhylmzr force-pushed the master branch 2 times, most recently from a48afe0 to cc9d19b Compare July 22, 2024 17:54
@markus-oberhumer
Copy link
Contributor

Can we get this merged, please?

src/main.zig Outdated
// otherwise our atomic rename into place will fail. This also
// makes Zig do less work, avoiding pointless file system operations.
if (mem.eql(u8, it.only_arg, "/dev/null")) {
if (mem.eql(u8, it.only_arg, "/dev/null") or mem.eql(u8, it.only_arg, "-")) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes zig cc a.c -o - behave differently from clang/gcc which will output to a file named - but I guess it's an improvement over the current behavior of writing the binary file to stdout.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have adjusted the logic for emit binary files, now the behavior of zig cc a.c -o - is consistent with clang.

src/main.zig Outdated
@@ -2518,7 +2518,7 @@ fn buildOutputType(
switch (c_out_mode orelse .link) {
.link => {
create_module.opts.output_mode = if (is_shared_lib) .Lib else .Exe;
emit_bin = if (out_path) |p| .{ .yes = p } else EmitBin.yes_a_out;
emit_bin = if (out_path) |p| .{ .yes = p } else emit_bin;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes_a_out looks unused after this and could be deleted.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I was on the wrong branch and was getting the old behavior.

Why was this changed?

Copy link
Member

@andrewrk andrewrk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Causes the following regression:

andy@bark ~/tmp> ~/src/zig/build-noassert/stage4/bin/zig cc hello.c
andy@bark ~/tmp> ./a.out
./a.out: command not found

Please test before sending patches.

@zhylmzr
Copy link
Contributor Author

zhylmzr commented Aug 28, 2024

fixed, sorry for missing this test case.

clang's behavior.

1. `zig cc main.c -o /dev/null` shouldn't emit a.out
2. `zig cc -E main.c` and `zig cc -E main -o -` should output to stdout
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants