This is an archive of the discontinued LLVM Phabricator instance.

Migrate llvm::make_unique to std::make_unique
ClosedPublic

Authored by JDevlieghere on Aug 14 2019, 2:56 PM.

Details

Summary

Now that we've moved to C++14, we no longer need the llvm::make_unique implementation from STLExtras.h. This patch is a mechanical replacement of (hopefully) all the llvm::make_unique instances across the monorepo.

I've omitted context given the size of this diff.

Diff Detail

Event Timeline

JDevlieghere created this revision.Aug 14 2019, 2:56 PM

Remove whitespace related changes.

jfb added inline comments.Aug 14 2019, 3:02 PM
llvm/include/llvm/ADT/STLExtras.h
1429

This is used in NativeRegisterContextLinux_x86_64.h

JDevlieghere marked an inline comment as done.

Don't remove FreeDeleter

jfb accepted this revision.Aug 14 2019, 3:09 PM

Thanks!

jfb added a comment.Aug 14 2019, 3:11 PM

BTW, if this breaks stuff maybe it's better to do it one project at a time, and remove the helper at the very end.

In D66259#1630454, @jfb wrote:

BTW, if this breaks stuff maybe it's better to do it one project at a time, and remove the helper at the very end.

Yep, that sounds like a good approach. Removing the helper was useful for finding cases my regex missed :-)

llvm/include/llvm/ADT/STLExtras.h
1429

Good catch!

This revision was not accepted when it landed; it landed in state Needs Review.Aug 14 2019, 3:19 PM
This revision was automatically updated to reflect the committed changes.
thakis added a subscriber: thakis.Aug 15 2019, 8:41 AM

It looks like this breaks building with gcc5.1:

/b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/gcc510trusty/bin/g++  -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools/lld/ELF -I/b/s/w/ir/cache/builder/src/third_party/llvm/lld/ELF -I/b/s/w/ir/cache/builder/src/third_party/llvm/lld/include -Itools/lld/include -I/usr/include/libxml2 -Iinclude -I/b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include -DLLVM_FORCE_HEAD_REVISION -fvisibility-inlines-hidden -Werror=date-time -std=c++14 -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wno-maybe-uninitialized -Wdelete-non-virtual-dtor -Wno-comment -fdiagnostics-color -ffunction-sections -fdata-sections -O3    -UNDEBUG  -fno-exceptions -fno-unwind-tables -fno-asynchronous-unwind-tables -fno-rtti -MD -MT tools/lld/ELF/CMakeFiles/lldELF.dir/LinkerScript.cpp.o -MF tools/lld/ELF/CMakeFiles/lldELF.dir/LinkerScript.cpp.o.d -o tools/lld/ELF/CMakeFiles/lldELF.dir/LinkerScript.cpp.o -c /b/s/w/ir/cache/builder/src/third_party/llvm/lld/ELF/LinkerScript.cpp
In file included from /b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/gcc510trusty/include/c++/5.1.0/bits/stl_algobase.h:71:0,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/gcc510trusty/include/c++/5.1.0/memory:62,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/ADT/Optional.h:22,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/ADT/STLExtras.h:19,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/ADT/StringRef.h:12,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/ADT/StringMap.h:16,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/Support/Host.h:16,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/ADT/Hashing.h:48,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm/lld/include/lld/Common/LLVM.h:19,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm/lld/include/lld/Common/ErrorHandler.h:71,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm/lld/ELF/Config.h:12,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm/lld/ELF/LinkerScript.h:12,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm/lld/ELF/LinkerScript.cpp:13:
/b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/gcc510trusty/include/c++/5.1.0/bits/predefined_ops.h: In instantiation of ‘constexpr bool __gnu_cxx::__ops::_Iter_comp_iter<_Compare>::operator()(_Iterator1, _Iterator2) [with _Iterator1 = lld::elf::InputSectionBase*; _Iterator2 = lld::elf::InputSectionBase*; _Compare = std::function<bool(lld::elf::InputSectionBase*, lld::elf::InputSectionBase*)>]’:
/b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/gcc510trusty/include/c++/5.1.0/functional:1982:6:   required by substitution of ‘template<class _Res, class ... _ArgTypes> template<class _Functor> using _Invoke = decltype (std::__callable_functor(declval<_Functor&>())((declval<_ArgTypes>)()...)) [with _Functor = __gnu_cxx::__ops::_Iter_comp_iter<std::function<bool(lld::elf::InputSectionBase*, lld::elf::InputSectionBase*)> >; _Res = bool; _ArgTypes = {lld::elf::InputSectionBase*, lld::elf::InputSectionBase*}]’
/b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/gcc510trusty/include/c++/5.1.0/functional:1992:56:   required by substitution of ‘template<class _Res, class ... _ArgTypes> template<class _Functor> using _Callable = std::__and_<std::function<_Res(_ArgTypes ...)>::_NotSelf<_Functor>, std::__check_func_return_type<std::function<_Res(_ArgTypes ...)>::_Invoke<_Functor>, _Res> > [with _Functor = __gnu_cxx::__ops::_Iter_comp_iter<std::function<bool(lld::elf::InputSectionBase*, lld::elf::InputSectionBase*)> >; _Res = bool; _ArgTypes = {lld::elf::InputSectionBase*, lld::elf::InputSectionBase*}]’
/b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/gcc510trusty/include/c++/5.1.0/functional:2057:9:   required by substitution of ‘template<class _Functor, class> std::function<_Res(_ArgTypes ...)>::function(_Functor) [with _Functor = __gnu_cxx::__ops::_Iter_comp_iter<std::function<bool(lld::elf::InputSectionBase*, lld::elf::InputSectionBase*)> >; <template-parameter-1-2> = <missing>]’
/b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/gcc510trusty/include/c++/5.1.0/bits/predefined_ops.h:130:46:   required from ‘constexpr __gnu_cxx::__ops::_Iter_comp_iter<_Compare> __gnu_cxx::__ops::__iter_comp_iter(_Compare) [with _Compare = std::function<bool(lld::elf::InputSectionBase*, lld::elf::InputSectionBase*)>]’
/b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/gcc510trusty/include/c++/5.1.0/bits/stl_algo.h:4933:43:   required from ‘void std::stable_sort(_RAIter, _RAIter, _Compare) [with _RAIter = lld::elf::InputSection**; _Compare = std::function<bool(lld::elf::InputSectionBase*, lld::elf::InputSectionBase*)>]’
/b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/ADT/STLExtras.h:1322:19:   required from ‘void llvm::stable_sort(R&&, Compare) [with R = llvm::MutableArrayRef<lld::elf::InputSection*>&; Compare = std::function<bool(lld::elf::InputSectionBase*, lld::elf::InputSectionBase*)>]’
/b/s/w/ir/cache/builder/src/third_party/llvm/lld/ELF/LinkerScript.cpp:347:44:   required from here
/b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/gcc510trusty/include/c++/5.1.0/bits/predefined_ops.h:123:46: error: no match for call to ‘(std::function<bool(lld::elf::InputSectionBase*, lld::elf::InputSectionBase*)>) (lld::elf::InputSectionBase&, lld::elf::InputSectionBase&)’
         { return bool(_M_comp(*__it1, *__it2)); }
                                              ^
In file included from /b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/gcc510trusty/include/c++/5.1.0/memory:79:0,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/ADT/Optional.h:22,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/ADT/STLExtras.h:19,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/ADT/StringRef.h:12,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/ADT/StringMap.h:16,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/Support/Host.h:16,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/ADT/Hashing.h:48,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm/lld/include/lld/Common/LLVM.h:19,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm/lld/include/lld/Common/ErrorHandler.h:71,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm/lld/ELF/Config.h:12,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm/lld/ELF/LinkerScript.h:12,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm/lld/ELF/LinkerScript.cpp:13:
/b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/gcc510trusty/include/c++/5.1.0/functional:2266:5: note: candidate: _Res std::function<_Res(_ArgTypes ...)>::operator()(_ArgTypes ...) const [with _Res = bool; _ArgTypes = {lld::elf::InputSectionBase*, lld::elf::InputSectionBase*}]
     function<_Res(_ArgTypes...)>::
     ^
/b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/gcc510trusty/include/c++/5.1.0/functional:2266:5: note:   no known conversion for argument 1 from ‘lld::elf::InputSectionBase’ to ‘lld::elf::InputSectionBase*’

Sorry, wrong patch, looks like this is actually due to D66195

It looks like this breaks building with gcc5.1:

Why are you using 5.1? That's dumb. There are tons of bugs in 5.1 that are fixed in later 5.x releases, especially if you're using C++14.

That's a really bad choice of minimum version to insist on supporting.

Source: me, because I broke lots of stuff in 5.1 and then fixed it.

jfb added a comment.Aug 15 2019, 10:18 AM

It looks like this breaks building with gcc5.1:

Why are you using 5.1? That's dumb. There are tons of bugs in 5.1 that are fixed in later 5.x releases, especially if you're using C++14.

That's a really bad choice of minimum version to insist on supporting.

Source: me, because I broke lots of stuff in 5.1 and then fixed it.

I'll bring this up on the list and suggest we move to 5.5 as you suggested privately.

FWIW I think this particular bug was https://gcc.gnu.org/PR65942

jfb added a comment.Aug 15 2019, 1:15 PM

FWIW I think this particular bug was https://gcc.gnu.org/PR65942

Indeed, thanks for the reference!

asbirlea removed a subscriber: asbirlea.Aug 15 2019, 1:26 PM