[PATCH] D81447: [llvm] Added support for stand-alone cmake object libraries.

[Mircea Trofin via Phabricator via llvm-commits]

mtrofin marked 3 inline comments as done.
mtrofin added inline comments.


================
Comment at: llvm/cmake/modules/AddLLVM.cmake:390
+#     If STATIC or SHARED are
+#     also specified, create an OBJECT library target named obj.${name} in
+#     addition to those.
----------------
phosek wrote:
> mtrofin wrote:
> > phosek wrote:
> > > I don't understand this part of the comment, does it mean that we always want to create `OBJECT` library even either `STATIC` or `SHARED` is specified?
> > This documents the existing behavior. Today, this is what happens:
> > 
> > llvm_add_library.   |. add_library parameter
> > 
> > STATIC                    |   STATIC
> > SHARED                 |   SHARED
> > STATIC SHARED.   |  SHARED, called ${name}, and a STATIC also, called ${name}_static
> > STATIC OBJECT.    |  STATIC called ${name}, and an OBJECT called obj.${name}. The link interface defined when calling llvm_add_library becomes 
> >                                     dependencies of this OBJECT target. The OBJECT target has no link interface 
> > SHARED OBJECT  | same as above, s/STATIC/SHARED
> > OBJECT                   | same as STATIC OBJECT. 
> > 
> > On this last point, my change is technically a breaking change - but it appears nothing uses it in that particular way, i.e. OBJECT without parameters. 
> > 
> > On this, I think we should enforce that at least one of the STATIC|OBJECT|SHARED be passed - wdyt?
> > 
> > 
> SGTM, `SHARED`, `STATIC` and `OBJECT` should be three different output modes that could be combined to build multiple different outputs in a single `llvm_add_library` invocation, but at least one of those has to be specified.
Addressed: thinking more of it, to avoid breaking changes (and also avoid some asymmetry[1] in the API), I'd propose we add a separate flag - OBJECT_ONLY. 

Checking upfront that the flags make sense - i.e. OBJECT_ONLY should come alone.

WDYT?

[1] Today's API looks at BUILD_SHARED_LIBS to figure out defaults if STATIC or SHARED aren't specified. I can see why that may be a good thing. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81447/new/

https://reviews.llvm.org/D81447