[llvm-commits] Patch for llvm-c to expose targetmachine & allow emit obj/asm files

[Duncan Sands]

Hi Carlo,

> --- include/llvm-c/TargetMachine.h	(revision 0)
> +++ include/llvm-c/TargetMachine.h	(working copy)
...

> +|* This header declares the C interface to libLLVMBitReader.a, which          *|
> +|* implements input of the LLVM bitcode format.                               *|

^ This description is not correct.

...
> +/*===-- Target Machine -------------------------------------------------------===*/
> +/** Creates a new llvm::TargetMachine. See llvm::Target::createTargetMachine */
> +LLVMTargetMachineRef LLVMCreateTargetMachine(LLVMTargetRef T, char *Triple, char *CPU, char *Features, LLVMCodeGenOptLevel Level, LLVMRelocMode Reloc, LLVMCodeModel CodeModel);

This and a bunch of other lines are too long: maximum line length is 80
characters.

...
> +/** Returns the triple used creating this target machine. See llvm::TargetMachine::getTriple. The result needs to be Disposed with LLVMDisposeMessage. */

No need for a capital D on Disposed.

...
> +/** Emits an asm or object file for the given module to the filename. This wraps several c++ only classes (among them a file stream). Returns any error in ErrorMessage */

Does ErrorMessage need to be disposed of with LLVMDisposeMessage?

> --- lib/Target/TargetMachineC.cpp	(revision 0)
> +++ lib/Target/TargetMachineC.cpp	(working copy)
...
> +LLVMTargetMachineRef LLVMCreateTargetMachine(LLVMTargetRef T, char* Triple, char* CPU, char* Features, LLVMCodeGenOptLevel Level, LLVMRelocMode Reloc, LLVMCodeModel CodeModel) {

^ More lines that are too long.  Also, there seems to be trailing whitespace
here and there.

> +  Reloc::Model RM = static_cast<Reloc::Model>(Reloc);
> +  CodeModel::Model CM = static_cast<CodeModel::Model>(CodeModel);
> +  CodeGenOpt::Level OL = static_cast<CodeGenOpt::Level>(Level);

^ You shouldn't be using static_cast on these enums: if the LLVM enum is
reordered or changed one day this needs to still work without reordering
the llvm-c enum.  You need to use switches:
   switch (Reloc) {
     case CPP_XYZ:
       val = C_XYZ;
       break;
     ...
   }
etc.

> +LLVMBool LLVMTargetMachineEmitToFile(LLVMTargetMachineRef T, LLVMModuleRef M, char* Filename, LLVMCodeGenFileType codegen, char** ErrorMessage) {
> +// Inspired by the D binding

If you are grateful to D then mention it in the commit message not here.

Ciao, Duncan.