delcypher added inline comments.
================
Comment at: cmake/modules/FindZ3.cmake:5
+PKG_CHECK_MODULES(PC_Z3 QUIET libz3)
+set(Z3_DEFINITIONS ${PC_LIBZ3_CFLAGS_OTHER})
+
----------------
ddcc wrote:
> delcypher wrote:
> > @ddcc This CMake variable is set but never used. Also based on the name it
> > suggests that they are compiler definitions rather than other compiler
> > flags. Does `_CFLAGS_OTHER` have those semantics? It's unclear from
> > https://cmake.org/cmake/help/v3.7/module/FindPkgConfig.html#command:pkg_check_modules
> > what they are supposed to be.
> >
> > To consume these flags you could add `target_compile_definitions()` and
> > `target_compile_options()` to all users of Z3. A more elegant way of doing
> > this is to create an imported library target (e.g. `z3::libz3`) and set
> > compile definitions, compile options and include directories on this
> > imported target with `INTERFACE` visibility so that these usage
> > requirements implicitly propagate to all users of `z3::libz3`.
> I'm not very familiar with CMake, so I based it off of the FindLibXml2.cmake
> from the upstream CMake repository. The `pkg-config` part isn't used
> currently, but I left it in case z3 does get a proper package.
I'm not a huge fan of leaving around dead code. I don't have any intention of
implementing pkg-config files for Z3 so I think it's unlikely that support will
ever happen. So personally I'd rather you removed the pkg-config stuff here.
https://reviews.llvm.org/D28952
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
