Skip to content

Build fails against LLVM 3.7.0 #28830

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
wthrowe opened this issue Oct 3, 2015 · 5 comments
Closed

Build fails against LLVM 3.7.0 #28830

wthrowe opened this issue Oct 3, 2015 · 5 comments
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.

Comments

@wthrowe
Copy link
Contributor

wthrowe commented Oct 3, 2015

Broken by 27dd6dd in #28500.

/home/wthrowe/computing/rust/src/rustllvm/RustWrapper.cpp: In function ‘LLVMOpaqueValue* LLVMRustBuildLandingPad(LLVMBuilderRef, LLVMTypeRef, LLVMValueRef, unsigned int, const char*, LLVMValueRef)’:
/home/wthrowe/computing/rust/src/rustllvm/RustWrapper.cpp:954:69: error: invalid conversion from ‘LLVMValueRef {aka LLVMOpaqueValue*}’ to ‘unsigned int’ [-fpermissive]
     return LLVMBuildLandingPad(Builder, Ty, PersFn, NumClauses, Name);
                                                                     ^
/home/wthrowe/computing/rust/src/rustllvm/RustWrapper.cpp:954:69: error: invalid conversion from ‘unsigned int’ to ‘const char*’ [-fpermissive]
/home/wthrowe/computing/rust/src/rustllvm/RustWrapper.cpp:954:69: error: too many arguments to function ‘LLVMOpaqueValue* LLVMBuildLandingPad(LLVMBuilderRef, LLVMTypeRef, unsigned int, const char*)’
In file included from /usr/include/llvm/IR/Value.h:17:0,
                 from /usr/include/llvm/IR/User.h:24,
                 from /usr/include/llvm/IR/Instruction.h:22,
                 from /usr/include/llvm/IR/BasicBlock.h:19,
                 from /usr/include/llvm/IR/IRBuilder.h:21,
                 from /home/wthrowe/computing/rust/src/rustllvm/rustllvm.h:11,
                 from /home/wthrowe/computing/rust/src/rustllvm/RustWrapper.cpp:11:
/usr/include/llvm-c/Core.h:2677:14: note: declared here
 LLVMValueRef LLVMBuildLandingPad(LLVMBuilderRef B, LLVMTypeRef Ty,
              ^
@dotdash
Copy link
Contributor

dotdash commented Oct 4, 2015

The C API for LLVM was accidentally changed in 3.7.0, this is/will be fixed in 3.7.1 and we already have that fix in our LLVM fork.

@alexcrichton do we want to add a check for 3.7.0 or just close this and pretend 3.7.0 never happened?

@wthrowe
Copy link
Contributor Author

wthrowe commented Oct 4, 2015

Feel free to close this if you decide you don't care. I already have to apply a patch to build against my system LLVM, so it's not a big deal to me to make that patch slightly larger until 3.7.1 is released.

@alexcrichton
Copy link
Member

Hm, so now I think I'm a little confused! This was indeed breakage that snuck into LLVM at some point which then snuck back out around the time of the release. I made this change in the source because our Travis builder is now installing LLVM from their apt repos which have this change. Now those repos also report that the LLVM version is 3.7.0, so I thought that this actually made its way into the 3.7.0 release and I forgot to include it.

If they update their repos every night, though, and don't bump the version just yet, that may also explain it! I'm all for supporting as many versions of LLVM as possible, but I'm not sure how to detect at compile-time the "true 3.7.0 release" vs "what's in LLVM's apt repos" now...

@dotdash
Copy link
Contributor

dotdash commented Oct 4, 2015

@alexcrichton according to http://llvm.org/apt/ the packages are indeed built from the latest commit on the respective branches (see Technical aspects down at the bottom).

So I see three options:

  1. Don't support the true 3.7.0 and move on.
  2. Wait for the packages to get their patch level bumped and then add support for true 3.7.0 and accept build failures with the intermediate versions.
  3. Add a test to do a compile check to see which version we have.

3 seems like overkill. And 2 is probably not really worth it, because I expect distros not to pick up 3.7.0 for their stable releases because of this API breakage. And users should really use 3.7.1 instead once that comes out.

So I'd say let's close this for now, and maybe do 2 later, if any distro does indeed package 3.7.0 in a stable release.

@steveklabnik steveklabnik added A-build A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. labels Oct 4, 2015
@alexcrichton
Copy link
Member

Aha, that does indeed explain everything, thanks @dotdash! I agree that going with option (2) as well, and if distros haven't picked up LLVM 3.7.0 yet it may not be so bad. Closing for now and we can reevaluate in the future if 3.7.0 vs 3.7.1 comes up!

wizeman referenced this issue in NixOS/nixpkgs Oct 18, 2015
The build was changed to compile with the bundled LLVM because compiling
against the system's LLVM now fails with compilation errors.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.
Projects
None yet
Development

No branches or pull requests

4 participants