Skip to content

main: use internal libclang to build C and assembly files #184

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

Closed
wants to merge 1 commit into from

Conversation

aykevl
Copy link
Member

@aykevl aykevl commented Feb 11, 2019

This avoids a dependency on the clang-7 tool for release builds.

Note: this may need some more testing.

@aykevl
Copy link
Member Author

aykevl commented Feb 11, 2019

With this change, the release tarball grows from 44MB to 50MB compressed, the extracted files combined (according to du) grow from 211MB to 233MB. I think avoiding the clang dependency is well worth the cost.

@aykevl
Copy link
Member Author

aykevl commented Feb 11, 2019

Also, this is a building block for supporting standard header includes in CGo preambles:

// #include <stdint.h>
import "C"

@aykevl
Copy link
Member Author

aykevl commented Feb 13, 2019

Related to the discussion we had in Slack, I wonder whether the inclusion of LLVM code is allowed? It's licensed under a BSD-like license, but not the BSD 3-clause license.

@deadprogram
Copy link
Member

Please provide link to specific code and we can try to ensure that we are including any required notices.

@aykevl
Copy link
Member Author

aykevl commented Feb 13, 2019

This is the source:
https://github.com/llvm-mirror/clang/blob/master/tools/driver/cc1as_main.cpp

I would like to refactor this upstream so we can use it more easily (without copyright issues), but that's not the case at the moment of course.

@deadprogram deadprogram changed the base branch from master to dev February 14, 2019 12:27
@deadprogram
Copy link
Member

I think we should add

// Includes part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.

to the file https://github.com/tinygo-org/tinygo/blob/dev/LICENSE

as well as updating year and the authorship of as we had previously discussed to "The TinyGo Authors".

However, perhaps all that is best done in a separate PR?

@deadprogram
Copy link
Member

Is this PR otherwise ready to be merged?

@aykevl
Copy link
Member Author

aykevl commented Feb 27, 2019

There are a few things which I'd like to fix before merging.

@deadprogram
Copy link
Member

OK sounds good. Just post to this PR when ready for review.

@aykevl
Copy link
Member Author

aykevl commented Mar 9, 2019

I found a rather inconvenient issue: libclang.so does not export clang::Driver, it only exports a C interface that does not include support for the compiler driver.
At the moment, we use the clang -cc1 interface to parse the C snippet before import "C" statements. This happens to work, because the clang -cc1 command line interface is superficially similar to the interface you would normally use to invoke a compiler. But there are many things that aren't the same, and so we need the Clang compiler driver (clang::Driver) to translate the high-level command line interface that people normally use to the low-level clang -cc1 command line interface used internally in libclang, which is undocumented and can change at any time.

So I see 3 possible ways around this:

  1. Stop linking to the system-installed clang and LLVM shared libraries. Always use static linking. This could actually work fairly well because it seems that LLVM by default ships with static libraries (.a) so we could keep using the workflow we're already using. The biggest downside is that linking will be slower, which is very inconvenient for compiler development.
  2. Keep using dynamic linking, but disable CGo support in that case. Releases will have CGo support, but TinyGo installed with go get won't support CGo.
  3. Only implement proper command line parsing when building statically and otherwise fall back to passing CFLAGS directly to libclang without driver. This will lead to many non-obvious compiler errors.

I'm slightly in favor of 2. @deadprogram what do you think?

@aykevl
Copy link
Member Author

aykevl commented Mar 9, 2019

Oh, there is a 4th option: invoke clang-7 as an external command and let it print the commands it would execute instead of executing them, there's a flag for this. I think this could work but it feels like a hack.

@deadprogram
Copy link
Member

If we go with option 2, what do we then need to do to install TinyGo with the CGo support?

@deadprogram
Copy link
Member

Pinging this PR to re-ask same question, as above.

@aykevl
Copy link
Member Author

aykevl commented Apr 3, 2019

If we go with option 2, what do we then need to do to install TinyGo with the CGo support?

Build it statically with LLVM, like is done now for release builds. Maybe we can automate this somehow, with (for example) make llvm.

@aykevl
Copy link
Member Author

aykevl commented Apr 3, 2019

Also, I want to build it statically in CircleCI to make sure CGo support gets proper test coverage. This depends on local changes I have here (almost finished!) which in turn depend on #211.

@deadprogram
Copy link
Member

Now that #211 is merged, this PR can proceed, correct?

@aykevl
Copy link
Member Author

aykevl commented Apr 4, 2019

No, there are a few more changes needed. In particular, #256.

This avoids a dependency on the clang-7 tool for release builds.
@deadprogram
Copy link
Member

Now that we have a viable way to build static executable, especially on macOS, we need this more than ever! :)

@deadprogram
Copy link
Member

Just wondering if this PR is now possible on all supported platforms?

@aykevl
Copy link
Member Author

aykevl commented Apr 25, 2019

Probably. I have started work on rebasing it, but it turned out to be a bit more difficult.

@justinclift
Copy link
Member

I found a rather inconvenient issue: libclang.so does not export clang::Driver

Any idea if that's on purpose, or if it could be fixed upstream so that it does?

@aykevl
Copy link
Member Author

aykevl commented May 1, 2019

All exported functions in libclang are related to traversing the C AST, I think that's their goal and they don't want to expose the compiler itself.

@aykevl aykevl mentioned this pull request Jun 4, 2019
@deadprogram
Copy link
Member

Just noticed the last comment. That is not entirely encouraging. Is this going to be a viable long-term approach?

@aykevl
Copy link
Member Author

aykevl commented Jun 6, 2019

I had misunderstood something. Libclang on its own is good enough: it already uses the Clang driver internally.

@aykevl
Copy link
Member Author

aykevl commented Jun 26, 2019

If accepted, this would make the PR much simpler: https://reviews.llvm.org/D63852

@deadprogram
Copy link
Member

This PR is a bit dormant. Also I see that the LLVM PR never has seemed to go anywhere.

That said, the LLVM installation is the biggest inconvenience when using TinyGo.

Perhaps once TinyGo is using LLVM 9 we can revisit this?

@aykevl
Copy link
Member Author

aykevl commented Dec 4, 2019

This PR was grossly out of date, so I replaced it with a new one: #769.

@aykevl aykevl closed this Dec 4, 2019
@deadprogram deadprogram deleted the builtin-clang branch December 30, 2019 13:35
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.

3 participants