<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Feb 24, 2015 at 4:19 PM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">FWIW, we need to specify this stuff much better. The current situation is actively harmful.<div><br></div><div><br></div><div>Also, Filip, if you want the open source releases to be known to work with webkit, you *need* to start testing sooner. It is pure chance that this could get into any release. If it doesn't make it, and goes in the .1 release, so be it.</div><div><br></div><div><br></div><div>However, I don think it is reasoonable to fix. I think the fix needs to be somewhat different. It needs to say that this has no effect and is deprecated and will go away in the next release, and the release notes need to say that as well.</div></div></blockquote><div><br></div><div>Do we even have a "deprecate and then remove" policy for the C bindings?</div><div><br></div><div>-- Sean Silva</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Feb 24, 2015 at 4:09 PM, Sean Silva <span dir="ltr"><<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Bindings are supposed to be pretty stable too I think. I think your patch is fine for the C stuff. CC'ing llgo guys. I think the ocaml maintainer already chimed in to PR21901 so I assume he is ok with this.<div><br></div><div>-- Sean Silva</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Feb 24, 2015 at 3:46 PM, Filip Pizlo <span dir="ltr"><<a href="mailto:fpizlo@apple.com" target="_blank">fpizlo@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Proposed fix:<br>
<br>
<br><br>
<br>
OK to commit?<br>
<br>
-Filip<div><div><br>
<br>
<br>
> On Feb 24, 2015, at 3:39 PM, Filip Pizlo <<a href="mailto:fpizlo@apple.com" target="_blank">fpizlo@apple.com</a>> wrote:<br>
><br>
> This change breaks the WebKit build.<br>
><br>
> The removal of the LLVMLinkerMode type from the C API violates the principle of C API stability. The latest release of LLVM has this type, which makes it subject to the usual API stability rules.<br>
><br>
> What do you recommend we do? Maybe just bringing back the enum is sufficient.<br>
><br>
> -Filip<br>
><br>
><br>
>> On Dec 23, 2014, at 11:16 AM, Rafael Espindola <<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>> wrote:<br>
>><br>
>> Author: rafael<br>
>> Date: Tue Dec 23 13:16:45 2014<br>
>> New Revision: 224782<br>
>><br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=224782&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=224782&view=rev</a><br>
>> Log:<br>
>> Finish removing DestroySource.<br>
>><br>
>> Fixes pr21901.<br>
>><br>
>> Modified:<br>
>> llvm/trunk/bindings/go/llvm/linker.go<br>
>> llvm/trunk/bindings/ocaml/linker/linker_ocaml.c<br>
>> llvm/trunk/bindings/ocaml/linker/<a href="http://llvm_linker.ml" target="_blank">llvm_linker.ml</a><br>
>> llvm/trunk/bindings/ocaml/linker/llvm_linker.mli<br>
>> llvm/trunk/include/llvm-c/Linker.h<br>
>> llvm/trunk/lib/Linker/LinkModules.cpp<br>
>> llvm/trunk/test/Bindings/OCaml/<a href="http://linker.ml" target="_blank">linker.ml</a><br>
>><br>
>> Modified: llvm/trunk/bindings/go/llvm/linker.go<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/bindings/go/llvm/linker.go?rev=224782&r1=224781&r2=224782&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/bindings/go/llvm/linker.go?rev=224782&r1=224781&r2=224782&view=diff</a><br>
>> ==============================================================================<br>
>> --- llvm/trunk/bindings/go/llvm/linker.go (original)<br>
>> +++ llvm/trunk/bindings/go/llvm/linker.go Tue Dec 23 13:16:45 2014<br>
>> @@ -20,16 +20,9 @@ package llvm<br>
>> import "C"<br>
>> import "errors"<br>
>><br>
>> -type LinkerMode C.LLVMLinkerMode<br>
>> -<br>
>> -const (<br>
>> - LinkerDestroySource = C.LLVMLinkerDestroySource<br>
>> - LinkerPreserveSource = C.LLVMLinkerPreserveSource<br>
>> -)<br>
>> -<br>
>> -func LinkModules(Dest, Src Module, Mode LinkerMode) error {<br>
>> +func LinkModules(Dest, Src Module) error {<br>
>> var cmsg *C.char<br>
>> - failed := C.LLVMLinkModules(Dest.C, Src.C, C.LLVMLinkerMode(Mode), &cmsg)<br>
>> + failed := C.LLVMLinkModules(Dest.C, Src.C, 0, &cmsg)<br>
>> if failed != 0 {<br>
>> err := errors.New(C.GoString(cmsg))<br>
>> C.LLVMDisposeMessage(cmsg)<br>
>><br>
>> Modified: llvm/trunk/bindings/ocaml/linker/linker_ocaml.c<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/bindings/ocaml/linker/linker_ocaml.c?rev=224782&r1=224781&r2=224782&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/bindings/ocaml/linker/linker_ocaml.c?rev=224782&r1=224781&r2=224782&view=diff</a><br>
>> ==============================================================================<br>
>> --- llvm/trunk/bindings/ocaml/linker/linker_ocaml.c (original)<br>
>> +++ llvm/trunk/bindings/ocaml/linker/linker_ocaml.c Tue Dec 23 13:16:45 2014<br>
>> @@ -23,11 +23,11 @@<br>
>><br>
>> void llvm_raise(value Prototype, char *Message);<br>
>><br>
>> -/* llmodule -> llmodule -> Mode.t -> unit */<br>
>> -CAMLprim value llvm_link_modules(LLVMModuleRef Dst, LLVMModuleRef Src, value Mode) {<br>
>> +/* llmodule -> llmodule -> unit */<br>
>> +CAMLprim value llvm_link_modules(LLVMModuleRef Dst, LLVMModuleRef Src) {<br>
>> char* Message;<br>
>><br>
>> - if (LLVMLinkModules(Dst, Src, Int_val(Mode), &Message))<br>
>> + if (LLVMLinkModules(Dst, Src, 0, &Message))<br>
>> llvm_raise(*caml_named_value("Llvm_linker.Error"), Message);<br>
>><br>
>> return Val_unit;<br>
>><br>
>> Modified: llvm/trunk/bindings/ocaml/linker/<a href="http://llvm_linker.ml" target="_blank">llvm_linker.ml</a><br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/bindings/ocaml/linker/llvm_linker.ml?rev=224782&r1=224781&r2=224782&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/bindings/ocaml/linker/llvm_linker.ml?rev=224782&r1=224781&r2=224782&view=diff</a><br>
>> ==============================================================================<br>
>> --- llvm/trunk/bindings/ocaml/linker/<a href="http://llvm_linker.ml" target="_blank">llvm_linker.ml</a> (original)<br>
>> +++ llvm/trunk/bindings/ocaml/linker/<a href="http://llvm_linker.ml" target="_blank">llvm_linker.ml</a> Tue Dec 23 13:16:45 2014<br>
>> @@ -11,11 +11,5 @@ exception Error of string<br>
>><br>
>> let () = Callback.register_exception "Llvm_linker.Error" (Error "")<br>
>><br>
>> -module Mode = struct<br>
>> - type t =<br>
>> - | DestroySource<br>
>> - | PreserveSource<br>
>> -end<br>
>> -<br>
>> -external link_modules : Llvm.llmodule -> Llvm.llmodule -> Mode.t -> unit<br>
>> +external link_modules : Llvm.llmodule -> Llvm.llmodule -> unit<br>
>> = "llvm_link_modules"<br>
>><br>
>> Modified: llvm/trunk/bindings/ocaml/linker/llvm_linker.mli<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/bindings/ocaml/linker/llvm_linker.mli?rev=224782&r1=224781&r2=224782&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/bindings/ocaml/linker/llvm_linker.mli?rev=224782&r1=224781&r2=224782&view=diff</a><br>
>> ==============================================================================<br>
>> --- llvm/trunk/bindings/ocaml/linker/llvm_linker.mli (original)<br>
>> +++ llvm/trunk/bindings/ocaml/linker/llvm_linker.mli Tue Dec 23 13:16:45 2014<br>
>> @@ -14,13 +14,6 @@<br>
>><br>
>> exception Error of string<br>
>><br>
>> -(** Linking mode. *)<br>
>> -module Mode : sig<br>
>> - type t =<br>
>> - | DestroySource<br>
>> - | PreserveSource<br>
>> -end<br>
>> -<br>
>> (** [link_modules dst src mode] links [src] into [dst], raising [Error]<br>
>> if the linking fails. *)<br>
>> -val link_modules : Llvm.llmodule -> Llvm.llmodule -> Mode.t -> unit<br>
>> \ No newline at end of file<br>
>> +val link_modules : Llvm.llmodule -> Llvm.llmodule -> unit<br>
>> \ No newline at end of file<br>
>><br>
>> Modified: llvm/trunk/include/llvm-c/Linker.h<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm-c/Linker.h?rev=224782&r1=224781&r2=224782&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm-c/Linker.h?rev=224782&r1=224781&r2=224782&view=diff</a><br>
>> ==============================================================================<br>
>> --- llvm/trunk/include/llvm-c/Linker.h (original)<br>
>> +++ llvm/trunk/include/llvm-c/Linker.h Tue Dec 23 13:16:45 2014<br>
>> @@ -20,20 +20,13 @@<br>
>> extern "C" {<br>
>> #endif<br>
>><br>
>> -<br>
>> -typedef enum {<br>
>> - LLVMLinkerDestroySource = 0, /* Allow source module to be destroyed. */<br>
>> - LLVMLinkerPreserveSource = 1 /* Preserve the source module. */<br>
>> -} LLVMLinkerMode;<br>
>> -<br>
>> -<br>
>> /* Links the source module into the destination module, taking ownership<br>
>> * of the source module away from the caller. Optionally returns a<br>
>> * human-readable description of any errors that occurred in linking.<br>
>> * OutMessage must be disposed with LLVMDisposeMessage. The return value<br>
>> * is true if an error occurred, false otherwise. */<br>
>> LLVMBool LLVMLinkModules(LLVMModuleRef Dest, LLVMModuleRef Src,<br>
>> - LLVMLinkerMode Mode, char **OutMessage);<br>
>> + unsigned Unused, char **OutMessage);<br>
>><br>
>> #ifdef __cplusplus<br>
>> }<br>
>><br>
>> Modified: llvm/trunk/lib/Linker/LinkModules.cpp<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Linker/LinkModules.cpp?rev=224782&r1=224781&r2=224782&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Linker/LinkModules.cpp?rev=224782&r1=224781&r2=224782&view=diff</a><br>
>> ==============================================================================<br>
>> --- llvm/trunk/lib/Linker/LinkModules.cpp (original)<br>
>> +++ llvm/trunk/lib/Linker/LinkModules.cpp Tue Dec 23 13:16:45 2014<br>
>> @@ -1739,7 +1739,7 @@ bool Linker::LinkModules(Module *Dest, M<br>
>> //===----------------------------------------------------------------------===//<br>
>><br>
>> LLVMBool LLVMLinkModules(LLVMModuleRef Dest, LLVMModuleRef Src,<br>
>> - LLVMLinkerMode Mode, char **OutMessages) {<br>
>> + unsigned Unused, char **OutMessages) {<br>
>> Module *D = unwrap(Dest);<br>
>> std::string Message;<br>
>> raw_string_ostream Stream(Message);<br>
>><br>
>> Modified: llvm/trunk/test/Bindings/OCaml/<a href="http://linker.ml" target="_blank">linker.ml</a><br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Bindings/OCaml/linker.ml?rev=224782&r1=224781&r2=224782&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Bindings/OCaml/linker.ml?rev=224782&r1=224781&r2=224782&view=diff</a><br>
>> ==============================================================================<br>
>> --- llvm/trunk/test/Bindings/OCaml/<a href="http://linker.ml" target="_blank">linker.ml</a> (original)<br>
>> +++ llvm/trunk/test/Bindings/OCaml/<a href="http://linker.ml" target="_blank">linker.ml</a> Tue Dec 23 13:16:45 2014<br>
>> @@ -45,7 +45,7 @@ let test_linker () =<br>
>><br>
>> let m1 = make_module "one"<br>
>> and m2 = make_module "two" in<br>
>> - link_modules m1 m2 Mode.DestroySource;<br>
>> + link_modules m1 m2;<br>
>> dispose_module m1;<br>
>><br>
>> let m1 = make_module "one"<br>
>><br>
>><br>
>> _______________________________________________<br>
>> llvm-commits mailing list<br>
>> <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br>
<br>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br></div></div></blockquote></div><br></div>
<br>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div>
</div></div></blockquote></div><br></div></div>