[llvm] r224782 - Finish removing DestroySource.

Hans Wennborg hans at chromium.org
Tue Feb 24 16:37:01 PST 2015


I was literally about to walk over to my desk to tag 3.6-final when I
saw this :-/

It seems that the removal of this functionality actually happened in
http://llvm.org/viewvc/llvm-project?view=revision&revision=220741, and
the commit message suggests that it was untested and broken.

If someone was relying on PreserveSource to work via the C API before,
this is a behaviour change, which is unfortunate. On the other hand,
if it was broken before and no one was using it, maybe it's not the
end of the world.

Maintaining the enum, as in Filip's patch, seems good to me. It would
also need a comment that this doesn't do anything, and a ReleaseNotes
update. I think that's the best we can do for 3.6 at this stage. I'd
be willing to take such a patch into the release without spinning a
new RC.

Thanks,
Hans


On Tue, Feb 24, 2015 at 4:04 PM, Sean Silva <chisophugis at gmail.com> wrote:
> Yeah, we should not be breaking the API.
>
> Hans, we should definitely include the resolution to this in the branch.
>
> -- Sean Silva
>
> On Tue, Feb 24, 2015 at 3:39 PM, Filip Pizlo <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> 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
>> > 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
>> >    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
>> >
>> > 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
>> >
>> > ==============================================================================
>> > --- 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
>> >
>> > ==============================================================================
>> > --- 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
>> > URL:
>> > 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 (original)
>> > +++ llvm/trunk/bindings/ocaml/linker/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
>> >
>> > ==============================================================================
>> > --- 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
>> >
>> > ==============================================================================
>> > --- 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
>> >
>> > ==============================================================================
>> > --- 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
>> > URL:
>> > 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 (original)
>> > +++ llvm/trunk/test/Bindings/OCaml/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
>> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>



More information about the llvm-commits mailing list