Hi Michael,
On Sun, Apr 26, 2020 at 11:21 PM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Sat, Apr 25, 2020 at 09:41:20PM -0700, Jesse Zhang wrote:
> > I searched my inbox and the archive, strange that nobody else is seeing
> > this.
> >
> > Turns out that LLVM has recently removed "llvm/IR/CallSite.h" in
> > (unreleased) version 11 [1][2]. To fix the build I tried conditionally
> > (on LLVM_VERSION_MAJOR < 11) including CallSite.h, but that looks yuck.
> > Then I poked at llvmjit_inline.cpp a bit and found that CallSite.h
> > doesn't seem to be really necessary. PFA a patch that simply removes
> > this #include.
> >
> > In addition, I've done the due dilligence of trying to build against
> > LLVM versions 8, 9, 10.
>
> LLVM 11 has not been released yet. Do you think that this part or
> even more are subject to change before the 11 release? My take would
> be to wait more before fixing this issue and make sure that our code
> is fixed when their code is GA'd.
> --
> Michael
Are you expressing a concern against "churning" this part of the code in
reaction to upstream LLVM changes? I'd agree with you in general. But
then the question we need to ask is "will we need to revert this 3 weeks
from now if upstream reverts their changes?", or "we change X to Y now,
will we need to instead change X to Z 3 weeks later?". In that frame of
mind, the answer is simply "no" w.r.t this patch: it's removing an
#include that simply has been dead: the upstream change merely exposed
it.
OTOH, is your concern more around "how many more dead #include will LLVM
11 reveal before September?", I'm open to suggestions. I personally have
a bias to keep things working.
Cheers,
Jesse