Обсуждение: LLVM breakage on seawasp
Hi, llvmjit_inline.cpp:177:55: error: ‘make_unique’ is not a member of ‘llvm’ std::unique_ptr<ImportMapTy> globalsToInline = llvm::make_unique<ImportMapTy>(); That's because they just moved to C++14 and replaced their own llvm::make_unique<> with std::make_unique<>: https://github.com/llvm-mirror/llvm/commit/114087caa6f95b526861c3af94b3093d9444c57b Perhaps we'll need some macrology to select between llvm and std versions? I am guessing we can't decree that PostgreSQL's minimum C++ level is C++14 and simply change it to std::make_unique. -- Thomas Munro https://enterprisedb.com
Hi, On August 24, 2019 1:08:11 PM PDT, Thomas Munro <thomas.munro@gmail.com> wrote: >Hi, > >llvmjit_inline.cpp:177:55: error: ‘make_unique’ is not a member of >‘llvm’ > std::unique_ptr<ImportMapTy> globalsToInline = >llvm::make_unique<ImportMapTy>(); > >That's because they just moved to C++14 and replaced their own >llvm::make_unique<> with std::make_unique<>: > >https://github.com/llvm-mirror/llvm/commit/114087caa6f95b526861c3af94b3093d9444c57b > >Perhaps we'll need some macrology to select between llvm and std >versions? I am guessing we can't decree that PostgreSQL's minimum C++ >level is C++14 and simply change it to std::make_unique. Perhaps just a #if new_enough using std::make_unique #else using llvm::mak_eunique At the start of the file, and then use it unqualified? Andres -- Sent from my Android device with K-9 Mail. Please excuse my brevity.
Andres Freund <andres@anarazel.de> writes: > On August 24, 2019 1:08:11 PM PDT, Thomas Munro <thomas.munro@gmail.com> wrote: >> That's because they just moved to C++14 and replaced their own >> llvm::make_unique<> with std::make_unique<>: >> https://github.com/llvm-mirror/llvm/commit/114087caa6f95b526861c3af94b3093d9444c57b >> Perhaps we'll need some macrology to select between llvm and std >> versions? I am guessing we can't decree that PostgreSQL's minimum C++ >> level is C++14 and simply change it to std::make_unique. So we're depending on APIs that upstream doesn't think are stable? regards, tom lane
Hi, On August 24, 2019 1:57:56 PM PDT, Tom Lane <tgl@sss.pgh.pa.us> wrote: >Andres Freund <andres@anarazel.de> writes: >> On August 24, 2019 1:08:11 PM PDT, Thomas Munro ><thomas.munro@gmail.com> wrote: >>> That's because they just moved to C++14 and replaced their own >>> llvm::make_unique<> with std::make_unique<>: >>> >https://github.com/llvm-mirror/llvm/commit/114087caa6f95b526861c3af94b3093d9444c57b >>> Perhaps we'll need some macrology to select between llvm and std >>> versions? I am guessing we can't decree that PostgreSQL's minimum >C++ >>> level is C++14 and simply change it to std::make_unique. > >So we're depending on APIs that upstream doesn't think are stable? Seawasp iirc builds against the development branch of llvm, which explains why we see failures there. Does that addresswhat you are concerned about? If not, could you expand? Andres -- Sent from my Android device with K-9 Mail. Please excuse my brevity.
On Sun, Aug 25, 2019 at 8:24 AM Andres Freund <andres@anarazel.de> wrote: > On August 24, 2019 1:08:11 PM PDT, Thomas Munro <thomas.munro@gmail.com> wrote: > >Perhaps we'll need some macrology to select between llvm and std > >versions? I am guessing we can't decree that PostgreSQL's minimum C++ > >level is C++14 and simply change it to std::make_unique. > > Perhaps just a > #if new_enough > using std::make_unique > #else > using llvm::mak_eunique > > At the start of the file, and then use it unqualified? Yeah, it's a pain though, you'd have to say: #if llvm >= 9 # if cpp >= 14 # using std::make_unique; # else # error "postgres needs at least c++ 14 to use llvm 9" # endif #else # using llvm::make_unique; #endif Maybe we should just use std::unique_ptr's constructor, ie give it new ImportMayTy() instead of using make_unique(), even though that's not cool C++ these days? -- Thomas Munro https://enterprisedb.com
Andres Freund <andres@anarazel.de> writes:
> On August 24, 2019 1:57:56 PM PDT, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> So we're depending on APIs that upstream doesn't think are stable?
> Seawasp iirc builds against the development branch of llvm, which explains why we see failures there. Does that
addresswhat you are concerned about? If not, could you expand?
I know it's the development branch. The question is whether this
breakage is something *they* ought to be fixing. If not, I'm
worried that we're too much in bed with implementation details
of LLVM that we shouldn't be depending on.
regards, tom lane
Hi, On August 24, 2019 2:37:55 PM PDT, Tom Lane <tgl@sss.pgh.pa.us> wrote: >Andres Freund <andres@anarazel.de> writes: >> On August 24, 2019 1:57:56 PM PDT, Tom Lane <tgl@sss.pgh.pa.us> >wrote: >>> So we're depending on APIs that upstream doesn't think are stable? > >> Seawasp iirc builds against the development branch of llvm, which >explains why we see failures there. Does that address what you are >concerned about? If not, could you expand? > >I know it's the development branch. The question is whether this >breakage is something *they* ought to be fixing. If not, I'm >worried that we're too much in bed with implementation details >of LLVM that we shouldn't be depending on. Don't think so - it's a C++ standard feature in the version of the standard LLVM is based on. So it's pretty reasonable forthem to drop their older backwards compatible function. Access -- Sent from my Android device with K-9 Mail. Please excuse my brevity.
Hi, On 2019-08-25 09:15:35 +1200, Thomas Munro wrote: > On Sun, Aug 25, 2019 at 8:24 AM Andres Freund <andres@anarazel.de> wrote: > > On August 24, 2019 1:08:11 PM PDT, Thomas Munro <thomas.munro@gmail.com> wrote: > > >Perhaps we'll need some macrology to select between llvm and std > > >versions? I am guessing we can't decree that PostgreSQL's minimum C++ > > >level is C++14 and simply change it to std::make_unique. > > > > Perhaps just a > > #if new_enough > > using std::make_unique > > #else > > using llvm::mak_eunique > > > > At the start of the file, and then use it unqualified? > > Yeah, it's a pain though, you'd have to say: > > #if llvm >= 9 > # if cpp >= 14 > # using std::make_unique; > # else > # error "postgres needs at least c++ 14 to use llvm 9" > # endif > #else > # using llvm::make_unique; > #endif I don't think we'd really need the inner part, because you can't use llvm 9 without a new enough compiler. There's plenty make_unique use in inline functions etc. > Maybe we should just use std::unique_ptr's constructor, ie give it new > ImportMayTy() instead of using make_unique(), even though that's not > cool C++ these days? Yea, wfm. do you want to make it so? Greetings, Andres Freund
Andres Freund <andres@anarazel.de> writes:
> On August 24, 2019 2:37:55 PM PDT, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I know it's the development branch. The question is whether this
>> breakage is something *they* ought to be fixing. If not, I'm
>> worried that we're too much in bed with implementation details
>> of LLVM that we shouldn't be depending on.
> Don't think so - it's a C++ standard feature in the version of the standard LLVM is based on. So it's pretty
reasonablefor them to drop their older backwards compatible function.
Whether it's reasonable or not doesn't really matter to my point.
We shouldn't be in the business of tracking multitudes of small
changes in LLVM, no matter whether they're individually "reasonable".
The more often this happens, the more concerned I am that we chose
the wrong semantic level to interface at.
regards, tom lane
On Sun, Aug 25, 2019 at 9:46 AM Andres Freund <andres@anarazel.de> wrote: > > Maybe we should just use std::unique_ptr's constructor, ie give it new > > ImportMayTy() instead of using make_unique(), even though that's not > > cool C++ these days? > > Yea, wfm. do you want to make it so? Done. -- Thomas Munro https://enterprisedb.com