Skip to content

zig run/cc: recognize "-x language" #13544

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

Merged
merged 1 commit into from
Jan 14, 2023
Merged

Conversation

motiejus
Copy link
Contributor

@motiejus motiejus commented Nov 14, 2022

This commit adds support for "-x language" for a couple of hand-picked supported languages. There is no reason the list of supported languages to not grow (e.g. add "c-header"), but I'd like to keep it small at the start.

Alternative 1

I first tried to add a new type "Language", and then add that to the CSourceFile. But oh boy what a change it turns out to be. So I am keeping myself tied to FileExt and see what you folks think.

Alternative 2

I tried adding Language: ?[]const u8 to CSourceFile. However, the language/ext, whatever we want to call it, still needs to be interpreted in the main loop: one kind of handling for source files, other kind of handling for everything else.

Test case

standalone.c

#include <iostream>

int main() {
    std::cout << "elho\n";
}

Compile and run:

$ ./zig run -x c++ -lc++ standalone.c
elho
$ ./zig c++ -x c++ standalone.c -o standalone && ./standalone
elho
$ .zig/x-lang/bin/zig cc -lc++ -x c++ standalone.c -o standalone  && ./standalone 
elho

Fixes #10915

@sagehane
Copy link
Contributor

Does this PR work with build.zig too?

Something like:

    const exe = b.addExecutable("whatever", null);
    exe.addCSourceFiles(&.{
        "file1.c",
        "file2.c",
    }, &.{
        "-x c++",
    });

I know a bizarre project that only compiles when compiling Lua with a C++ compiler, and this would greatly help.

@N00byEdge
Copy link
Contributor

N00byEdge commented Nov 14, 2022

wouldn't it be "-x", "c++"? For zig build it doesn't matter anyways, you can add cpp files aready afaik. @sagehane

@sagehane
Copy link
Contributor

sagehane commented Nov 15, 2022

With the status-quo, my code above would be treated as C files because of their file extensions.
The issue isn't that I need C++ support; it's that I need to treat .c files as C++.

And the stdlib currently only has addCSourceFile to handle both C and C++, so there's no way to work around it afaik.

Edit: That said, the name of addCSouceFile is quite misleading tbh. But I doubt the solution is to make addCppSourceFile and a new function for every type of language Clang supports.

@motiejus
Copy link
Contributor Author

cc @bfredl this is one of the prerequisites to compile neovim with zig cc.

@motiejus
Copy link
Contributor Author

Does this PR work with build.zig too?

Something like:

    const exe = b.addExecutable("whatever", null);
    exe.addCSourceFiles(&.{
        "file1.c",
        "file2.c",
    }, &.{
        "-x c++",
    });

I know a bizarre project that only compiles when compiling Lua with a C++ compiler, and this would greatly help.

Not in this PR. That could be added, but I am not sure if the approach will be accepted in the first place. So I will not extend the scope yet.

@motiejus
Copy link
Contributor Author

@sandhose's comment from #10915 (comment):

Can you try with #13544? I just added it to understand assembler-with-cpp.

@motiejus I built your PR and tried ; it fails with the following error:

$ /tmp/zig/build/stage3/bin/zig cc -target aarch64-linux-gnu -g -O0 -ffunction-sections -fdata-sections -fPIC -gdwarf-4 -fno-omit-frame
-pointer -Wall -Wextra -xassembler-with-cpp -DCFG_TARGET_OS_linux -DCFG_TARGET_ARCH_aarch64 -DCFG_TARGET_ENV_gnu -o /tmp/aarch_aapcs64.o -c src/arch/aar
ch_aapcs64.s
error: language 'assembly' is unsupported in this context

To reproduce:

git clone https://github.com/rust-lang/stacker
cd stacker/psm
zig cc -target aarch64-linux-gnu -g -O0 -ffunction-sections -fdata-sections -fPIC -gdwarf-4 -fno-omit-frame-pointer -Wall -Wextra -xassembler-with-cpp -DCFG_TARGET_OS_linux -DCFG_TARGET_ARCH_aarch64 -DCFG_TARGET_ENV_gnu -o /tmp/aarch_aapcs64.o -c src/arch/aarch_aapcs64.s

Thanks for providing the reproducible case. I've updated the PR and it no longer throws this error. Please test the current version.

@morenol
Copy link

morenol commented Dec 20, 2022

@sandhose's comment from #10915 (comment):

Can you try with #13544? I just added it to understand assembler-with-cpp.

@motiejus I built your PR and tried ; it fails with the following error:

$ /tmp/zig/build/stage3/bin/zig cc -target aarch64-linux-gnu -g -O0 -ffunction-sections -fdata-sections -fPIC -gdwarf-4 -fno-omit-frame
-pointer -Wall -Wextra -xassembler-with-cpp -DCFG_TARGET_OS_linux -DCFG_TARGET_ARCH_aarch64 -DCFG_TARGET_ENV_gnu -o /tmp/aarch_aapcs64.o -c src/arch/aar
ch_aapcs64.s
error: language 'assembly' is unsupported in this context

To reproduce:

git clone https://github.com/rust-lang/stacker
cd stacker/psm
zig cc -target aarch64-linux-gnu -g -O0 -ffunction-sections -fdata-sections -fPIC -gdwarf-4 -fno-omit-frame-pointer -Wall -Wextra -xassembler-with-cpp -DCFG_TARGET_OS_linux -DCFG_TARGET_ARCH_aarch64 -DCFG_TARGET_ENV_gnu -o /tmp/aarch_aapcs64.o -c src/arch/aarch_aapcs64.s

Thanks for providing the reproducible case. I've updated the PR and it no longer throws this error. Please test the current version.

I build from this branch (macos) and it worked for me. Thanks! Looking forward to having a zig release with this

@kassane
Copy link
Contributor

kassane commented Jan 11, 2023

Is it intended to add support to -x c++-module too?

@motiejus
Copy link
Contributor Author

Is it intended to add support to -x c++-module too?

Once this is merged, you are welcome to do so, should be relatively easy if you have a use case.

From my experience with working on this, it's difficult to add an extension without good understanding and example on how it will be used.

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.

Excellent work. I see only one more thing to be done before merging, which is to add the new field ext to the cache hash in the addNonIncrementalStuffToCacheManifest (search for extra_flags).

This commit adds support for "-x language" for a couple of hand-picked
supported languages. There is no reason the list of supported languages
to not grow (e.g. add "c-header"), but I'd like to keep it small at the
start.

Alternative 1
-------------

I first tried to add a new type "Language", and then add that to the
`CSourceFile`. But oh boy what a change it turns out to be. So I am
keeping myself tied to FileExt and see what you folks think.

Alternative 2
-------------

I tried adding `Language: ?[]const u8` to `CSourceFile`. However, the
language/ext, whatever we want to call it, still needs to be interpreted
in the main loop: one kind of handling for source files, other kind of
handling for everything else.

Test case
---------

*standalone.c*

    #include <iostream>

    int main() {
        std::cout << "elho\n";
    }

Compile and run:

    $ ./zig run -x c++ -lc++ standalone.c
    elho
    $ ./zig c++ -x c++ standalone.c -o standalone && ./standalone
    elho

Fixes ziglang#10915
@andrewrk andrewrk enabled auto-merge (rebase) January 13, 2023 21:52
@andrewrk andrewrk merged commit 6b3f59c into ziglang:master Jan 14, 2023
@motiejus motiejus deleted the x-lang-1 branch January 14, 2023 08:06
@motiejus
Copy link
Contributor Author

motiejus commented Apr 20, 2023

whoops, meant to comment in #14462.

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.

Zig c++ does not respect -x flag
6 participants