[llvm] r224782 - Finish removing DestroySource.
Filip Pizlo
fpizlo at apple.com
Tue Feb 24 17:24:57 PST 2015
> On Feb 24, 2015, at 4:19 PM, Chandler Carruth <chandlerc at google.com> wrote:
>
> FWIW, we need to specify this stuff much better. The current situation is actively harmful.
>
>
> 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.
WebKit currently does not test against LLVM tip-of-trunk; we usually bless some revision and live on it for a while. Also, different WebKit ports have different policies. We're not in a hurry to change this approach, because we have enough other stuff going on.
LLVM should have its own process in place for ensuring that C API stability guidelines are obeyed. These guidelines, as I understand them, are that once a C API function or type makes it into a release, it must not be removed without some period of deprecation. Function names, signatures, type names, and enum members should be part of that to enable source-level compatibility. If that isn't the policy now, then that's the policy I would ask for.
>
>
> 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.
Here is an improved fix for trunk. I tend to think that reverting is safest for the branch.
-Filip
>
> On Tue, Feb 24, 2015 at 4:09 PM, Sean Silva <chisophugis at gmail.com <mailto:chisophugis at gmail.com>> wrote:
> 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.
>
> -- Sean Silva
>
> On Tue, Feb 24, 2015 at 3:46 PM, Filip Pizlo <fpizlo at apple.com <mailto:fpizlo at apple.com>> wrote:
> Proposed fix:
>
>
>
>
> OK to commit?
>
> -Filip
>
>
>
> > On Feb 24, 2015, at 3:39 PM, Filip Pizlo <fpizlo at apple.com <mailto:fpizlo at apple.com>> wrote:
> >
> > This change breaks the WebKit build.
> >
> > 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.
> >
> > What do you recommend we do? Maybe just bringing back the enum is sufficient.
> >
> > -Filip
> >
> >
> >> On Dec 23, 2014, at 11:16 AM, Rafael Espindola <rafael.espindola at gmail.com <mailto:rafael.espindola at gmail.com>> wrote:
> >>
> >> Author: rafael
> >> Date: Tue Dec 23 13:16:45 2014
> >> New Revision: 224782
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=224782&view=rev <http://llvm.org/viewvc/llvm-project?rev=224782&view=rev>
> >> Log:
> >> Finish removing DestroySource.
> >>
> >> Fixes pr21901.
> >>
> >> Modified:
> >> llvm/trunk/bindings/go/llvm/linker.go
> >> llvm/trunk/bindings/ocaml/linker/linker_ocaml.c
> >> llvm/trunk/bindings/ocaml/linker/llvm_linker.ml <http://llvm_linker.ml/>
> >> llvm/trunk/bindings/ocaml/linker/llvm_linker.mli
> >> llvm/trunk/include/llvm-c/Linker.h
> >> llvm/trunk/lib/Linker/LinkModules.cpp
> >> llvm/trunk/test/Bindings/OCaml/linker.ml <http://linker.ml/>
> >>
> >> Modified: llvm/trunk/bindings/go/llvm/linker.go
> >> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/bindings/go/llvm/linker.go?rev=224782&r1=224781&r2=224782&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/bindings/go/llvm/linker.go?rev=224782&r1=224781&r2=224782&view=diff>
> >> ==============================================================================
> >> --- llvm/trunk/bindings/go/llvm/linker.go (original)
> >> +++ llvm/trunk/bindings/go/llvm/linker.go Tue Dec 23 13:16:45 2014
> >> @@ -20,16 +20,9 @@ package llvm
> >> import "C"
> >> import "errors"
> >>
> >> -type LinkerMode C.LLVMLinkerMode
> >> -
> >> -const (
> >> - LinkerDestroySource = C.LLVMLinkerDestroySource
> >> - LinkerPreserveSource = C.LLVMLinkerPreserveSource
> >> -)
> >> -
> >> -func LinkModules(Dest, Src Module, Mode LinkerMode) error {
> >> +func LinkModules(Dest, Src Module) error {
> >> var cmsg *C.char
> >> - failed := C.LLVMLinkModules(Dest.C, Src.C, C.LLVMLinkerMode(Mode), &cmsg)
> >> + failed := C.LLVMLinkModules(Dest.C, Src.C, 0, &cmsg)
> >> if failed != 0 {
> >> err := errors.New(C.GoString(cmsg))
> >> C.LLVMDisposeMessage(cmsg)
> >>
> >> Modified: llvm/trunk/bindings/ocaml/linker/linker_ocaml.c
> >> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/bindings/ocaml/linker/linker_ocaml.c?rev=224782&r1=224781&r2=224782&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/bindings/ocaml/linker/linker_ocaml.c?rev=224782&r1=224781&r2=224782&view=diff>
> >> ==============================================================================
> >> --- llvm/trunk/bindings/ocaml/linker/linker_ocaml.c (original)
> >> +++ llvm/trunk/bindings/ocaml/linker/linker_ocaml.c Tue Dec 23 13:16:45 2014
> >> @@ -23,11 +23,11 @@
> >>
> >> void llvm_raise(value Prototype, char *Message);
> >>
> >> -/* llmodule -> llmodule -> Mode.t -> unit */
> >> -CAMLprim value llvm_link_modules(LLVMModuleRef Dst, LLVMModuleRef Src, value Mode) {
> >> +/* llmodule -> llmodule -> unit */
> >> +CAMLprim value llvm_link_modules(LLVMModuleRef Dst, LLVMModuleRef Src) {
> >> char* Message;
> >>
> >> - if (LLVMLinkModules(Dst, Src, Int_val(Mode), &Message))
> >> + if (LLVMLinkModules(Dst, Src, 0, &Message))
> >> llvm_raise(*caml_named_value("Llvm_linker.Error"), Message);
> >>
> >> return Val_unit;
> >>
> >> Modified: llvm/trunk/bindings/ocaml/linker/llvm_linker.ml <http://llvm_linker.ml/>
> >> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/bindings/ocaml/linker/llvm_linker.ml?rev=224782&r1=224781&r2=224782&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/bindings/ocaml/linker/llvm_linker.ml?rev=224782&r1=224781&r2=224782&view=diff>
> >> ==============================================================================
> >> --- llvm/trunk/bindings/ocaml/linker/llvm_linker.ml <http://llvm_linker.ml/> (original)
> >> +++ llvm/trunk/bindings/ocaml/linker/llvm_linker.ml <http://llvm_linker.ml/> Tue Dec 23 13:16:45 2014
> >> @@ -11,11 +11,5 @@ exception Error of string
> >>
> >> let () = Callback.register_exception "Llvm_linker.Error" (Error "")
> >>
> >> -module Mode = struct
> >> - type t =
> >> - | DestroySource
> >> - | PreserveSource
> >> -end
> >> -
> >> -external link_modules : Llvm.llmodule -> Llvm.llmodule -> Mode.t -> unit
> >> +external link_modules : Llvm.llmodule -> Llvm.llmodule -> unit
> >> = "llvm_link_modules"
> >>
> >> Modified: llvm/trunk/bindings/ocaml/linker/llvm_linker.mli
> >> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/bindings/ocaml/linker/llvm_linker.mli?rev=224782&r1=224781&r2=224782&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/bindings/ocaml/linker/llvm_linker.mli?rev=224782&r1=224781&r2=224782&view=diff>
> >> ==============================================================================
> >> --- llvm/trunk/bindings/ocaml/linker/llvm_linker.mli (original)
> >> +++ llvm/trunk/bindings/ocaml/linker/llvm_linker.mli Tue Dec 23 13:16:45 2014
> >> @@ -14,13 +14,6 @@
> >>
> >> exception Error of string
> >>
> >> -(** Linking mode. *)
> >> -module Mode : sig
> >> - type t =
> >> - | DestroySource
> >> - | PreserveSource
> >> -end
> >> -
> >> (** [link_modules dst src mode] links [src] into [dst], raising [Error]
> >> if the linking fails. *)
> >> -val link_modules : Llvm.llmodule -> Llvm.llmodule -> Mode.t -> unit
> >> \ No newline at end of file
> >> +val link_modules : Llvm.llmodule -> Llvm.llmodule -> unit
> >> \ No newline at end of file
> >>
> >> Modified: llvm/trunk/include/llvm-c/Linker.h
> >> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm-c/Linker.h?rev=224782&r1=224781&r2=224782&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm-c/Linker.h?rev=224782&r1=224781&r2=224782&view=diff>
> >> ==============================================================================
> >> --- llvm/trunk/include/llvm-c/Linker.h (original)
> >> +++ llvm/trunk/include/llvm-c/Linker.h Tue Dec 23 13:16:45 2014
> >> @@ -20,20 +20,13 @@
> >> extern "C" {
> >> #endif
> >>
> >> -
> >> -typedef enum {
> >> - LLVMLinkerDestroySource = 0, /* Allow source module to be destroyed. */
> >> - LLVMLinkerPreserveSource = 1 /* Preserve the source module. */
> >> -} LLVMLinkerMode;
> >> -
> >> -
> >> /* Links the source module into the destination module, taking ownership
> >> * of the source module away from the caller. Optionally returns a
> >> * human-readable description of any errors that occurred in linking.
> >> * OutMessage must be disposed with LLVMDisposeMessage. The return value
> >> * is true if an error occurred, false otherwise. */
> >> LLVMBool LLVMLinkModules(LLVMModuleRef Dest, LLVMModuleRef Src,
> >> - LLVMLinkerMode Mode, char **OutMessage);
> >> + unsigned Unused, char **OutMessage);
> >>
> >> #ifdef __cplusplus
> >> }
> >>
> >> Modified: llvm/trunk/lib/Linker/LinkModules.cpp
> >> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Linker/LinkModules.cpp?rev=224782&r1=224781&r2=224782&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Linker/LinkModules.cpp?rev=224782&r1=224781&r2=224782&view=diff>
> >> ==============================================================================
> >> --- llvm/trunk/lib/Linker/LinkModules.cpp (original)
> >> +++ llvm/trunk/lib/Linker/LinkModules.cpp Tue Dec 23 13:16:45 2014
> >> @@ -1739,7 +1739,7 @@ bool Linker::LinkModules(Module *Dest, M
> >> //===----------------------------------------------------------------------===//
> >>
> >> LLVMBool LLVMLinkModules(LLVMModuleRef Dest, LLVMModuleRef Src,
> >> - LLVMLinkerMode Mode, char **OutMessages) {
> >> + unsigned Unused, char **OutMessages) {
> >> Module *D = unwrap(Dest);
> >> std::string Message;
> >> raw_string_ostream Stream(Message);
> >>
> >> Modified: llvm/trunk/test/Bindings/OCaml/linker.ml <http://linker.ml/>
> >> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Bindings/OCaml/linker.ml?rev=224782&r1=224781&r2=224782&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Bindings/OCaml/linker.ml?rev=224782&r1=224781&r2=224782&view=diff>
> >> ==============================================================================
> >> --- llvm/trunk/test/Bindings/OCaml/linker.ml <http://linker.ml/> (original)
> >> +++ llvm/trunk/test/Bindings/OCaml/linker.ml <http://linker.ml/> Tue Dec 23 13:16:45 2014
> >> @@ -45,7 +45,7 @@ let test_linker () =
> >>
> >> let m1 = make_module "one"
> >> and m2 = make_module "two" in
> >> - link_modules m1 m2 Mode.DestroySource;
> >> + link_modules m1 m2;
> >> dispose_module m1;
> >>
> >> let m1 = make_module "one"
> >>
> >>
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits <http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits <http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits <http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits <http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150224/e43f236f/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix-link-modules-api-stability.patch
Type: application/octet-stream
Size: 2365 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150224/e43f236f/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150224/e43f236f/attachment-0001.html>
More information about the llvm-commits
mailing list