Conversation
5da43fb to
60ecbf3
Compare
|
Very exciting! |
60ecbf3 to
48f618f
Compare
|
For maintaining Aro, I think we should pick one of these options:
Up to you, @Vexu. What do you prefer? |
|
I was planning on going with option 3. |
|
Sounds great 👍 |
|
So, what's the transition plan? For now, enable it only when |
|
That's what it does currently: // Make a decision on whether to use Clang or Aro for translate-c and compiling C files.
const c_frontend = blk: {
if (!build_options.have_llvm) break :blk .aro;
if (options.c_frontend) |explicit| break :blk explicit;
break :blk .clang;
}; |
1ef554f to
1d4e9bc
Compare
|
Not sure what the CI failure is caused by. I can't reproduce it locally with |
|
I believe the CI failure is because zig2 doesn't have any of the package management code included in it, including path-based dependencies ( Lines 4798 to 4803 in 3031819 |
|
Yes I'm sorry, after I mentioned #14603 above, I realized it wasn't actually going to work exactly that way because we want to avoid all that machinery while bootstrapping. I think it will be fine just adding logic to build.zig instead for integration. I think you had it that way in the first place anyway. |
andrewrk
left a comment
There was a problem hiding this comment.
Everything looks great and ready to merge, except I have one request-
| \\ -fclang Force using Clang as the C/C++ compilation backend | ||
| \\ -fno-clang Prevent using Clang as the C/C++ compilation backend |
There was a problem hiding this comment.
I think using CFrontend internally is a good idea, however, as for the CLI, I don't think zig ever plans to have more options for C frontends. The plan is:
- (now) support clang and aro
- (future) support aro only
I think -fclang and -fno-clang is nice since it is very similar to the llvm and lld flags above, and all 3 sets of flags will disappear together one day.
Do you have a strong preference for -c-frontend? I would prefer keeping the CLI as -fclang and -fno-clang.
There was a problem hiding this comment.
No preference, -fclang/-fno-clang works for me.
There was a problem hiding this comment.
Nice! In that case, feel free to proceed with the merge after making that change 👍
This starts work on #16268. None of it is tested currently so I'll need to update the test harness before this is contributor friendly.