[libcamera-devel] [PATCH v1.1 1/5] meson: Switch to C++17
Niklas Söderlund
niklas.soderlund at ragnatech.se
Mon Aug 24 23:41:26 CEST 2020
Hi Laurent,
Thanks for your work.
On 2020-08-24 20:57:24 +0300, Laurent Pinchart wrote:
> Hi Kieran,
>
> On Mon, Aug 24, 2020 at 10:52:53AM +0100, Kieran Bingham wrote:
> > On 22/08/2020 21:04, Laurent Pinchart wrote:
> > > Due to popular request, move from C++14 to C++17. This will allow
> > > dropping some custom constructs (such as a custom utils::clamp),
> > > benefiting from new language features, and dropping gcc 5 and 6 from the
> > > compilation tests.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > ---
> > > Changes since v1:
> > >
> > > - Update documentation
> > > - Print an error message when detecting a too old compiler
> >
> > Ok, some useful additions... reading. ..
> >
> > > ---
> > > Documentation/coding-style.rst | 5 ++---
> > > meson.build | 29 ++++++++++++++++++-----------
> > > 2 files changed, 20 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/Documentation/coding-style.rst b/Documentation/coding-style.rst
> > > index 7acba37b8de8..7c56a1b70014 100644
> > > --- a/Documentation/coding-style.rst
> > > +++ b/Documentation/coding-style.rst
> > > @@ -88,13 +88,12 @@ headers, and with double quotes for other libcamera headers.
> > > C++ Specific Rules
> > > ------------------
> > >
> > > -The code shall be implemented in C++14, with the following caveats:
> > > +The code shall be implemented in C++17, with the following caveats:
> > >
> > > * Type inference (auto and decltype) shall be used with caution, to avoid
> > > drifting towards an untyped language.
> > > * The explicit, override and final specifiers are to be used where applicable.
> > > -* General-purpose smart pointers (std::unique_ptr) deprecate std::auto_ptr.
> > > - Smart pointers, as well as shared pointers and weak pointers, shall not be
> > > +* Smart pointers, as well as shared pointers and weak pointers, shall not be
> > > overused.
> > > * Classes are encouraged to define move constructors and assignment operators
> > > where applicable, and generally make use of the features offered by rvalue
> > > diff --git a/meson.build b/meson.build
> > > index ec54e68f3635..cf2636d97100 100644
> > > --- a/meson.build
> > > +++ b/meson.build
> > > @@ -6,7 +6,7 @@ project('libcamera', 'c', 'cpp',
> > > default_options : [
> > > 'werror=true',
> > > 'warning_level=2',
> > > - 'cpp_std=c++14',
> > > + 'cpp_std=c++17',
> > > ],
> > > license : 'LGPL 2.1+')
> > >
> > > @@ -62,6 +62,23 @@ if cc.get_id() == 'clang'
> > > endif
> > > endif
> > >
> > > +if cc.get_id() == 'gcc'
> > > + if cc.version().version_compare('<7')
> > > + error('gcc version is too old, libcamera requires 7.0 or newer')
> > > + endif
> >
> > Clang v5 or greater is required for C++17. Do we need to catch that too?
> > or is that considered old enough that we're not going to hit it.
>
> The lowest clang version installed on my system is 6.0, so I haven't
> considered this. It's a good point, I'll add a check.
With this check added,
Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
>
> > I think it looks like Clang 5 was released around September 2017, while
> > GCC 7.1 was released earlier in May 2017.
> >
> > That said, wouldn't the compiler warn us quite verbosely if it didn't
> > support C++17 ?
> >
> > I.e. .. do /we/ really need to do this check here in meson?
>
> Here's how the compiler warns me on gcc 5:
>
> [4/281] Compiling C++ object 'src/libcamera/4ab8042@@camera at sha/event_notifier.cpp.o'
> FAILED: src/libcamera/4ab8042@@camera at sha/event_notifier.cpp.o
> g++-5.4.0 -Isrc/libcamera/4ab8042@@camera at sha -Isrc/libcamera -I../../src/libcamera -Iinclude -I../../include -I../../include/android/hardware/libhardware/include -I../../include/android/metadata -I../../include/android/system/core/include -Iinclude/libcamera -fdiagnostics-color=always -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wnon-virtual-dtor -Wextra -Werror -std=c++17 -g -include config.h -fPIC -pthread -MD -MQ 'src/libcamera/4ab8042@@camera at sha/event_notifier.cpp.o' -MF 'src/libcamera/4ab8042@@camera at sha/event_notifier.cpp.o.d' -o 'src/libcamera/4ab8042@@camera at sha/event_notifier.cpp.o' -c ../../src/libcamera/event_notifier.cpp
> In file included from ../../include/libcamera/object.h:14:0,
> from ../../include/libcamera/event_notifier.h:10,
> from ../../src/libcamera/event_notifier.cpp:8:
> ../../include/libcamera/bound_method.h:217:64: error: ‘maybe_unused’ attribute directive ignored [-Werror=attributes]
> R activate(Args... args, [[maybe_unused]] bool deleteMethod = false) override
> ^
> In file included from ../../include/libcamera/event_notifier.h:11:0,
> from ../../src/libcamera/event_notifier.cpp:8:
> ../../include/libcamera/signal.h: In member function ‘void libcamera::Signal<Args>::disconnect()’:
> ../../include/libcamera/signal.h:73:66: error: ‘maybe_unused’ attribute directive ignored [-Werror=attributes]
> SignalBase::disconnect([]([[maybe_unused]] SlotList::iterator &iter) {
> ^
> ../../include/libcamera/signal.h: In instantiation of ‘libcamera::Signal<Args>::disconnect()::<lambda(std::__cxx11::list<libcamera::BoundMethodBase*>::iterator&)> [with Args = {libcamera::EventNotifier*}; std::__cxx11::list<libcamera::BoundMethodBase*>::iterator = std::_List_iterator<libcamera::BoundMethodBase*>]’:
> ../../include/libcamera/signal.h:73:27: required from ‘struct libcamera::Signal<Args>::disconnect() [with Args = {libcamera::EventNotifier*}]::<lambda(std::__cxx11::list<libcamera::BoundMethodBase*>::iterator&)>’
> ../../include/libcamera/signal.h:73:25: required from ‘void libcamera::Signal<Args>::disconnect() [with Args = {libcamera::EventNotifier*}]’
> ../../include/libcamera/signal.h:44:13: required from ‘libcamera::Signal<Args>::~Signal() [with Args = {libcamera::EventNotifier*}]’
> ../../src/libcamera/event_notifier.cpp:67:56: required from here
> ../../include/libcamera/signal.h:73:66: error: unused parameter ‘iter’ [-Werror=unused-parameter]
> cc1plus: all warnings being treated as errors
>
> I'd rather have
>
> ../../meson.build:66:8: ERROR: Problem encountered: gcc version is too old, libcamera requires 7.0 or newer
>
> :-)
>
> > Anyway,
> >
> > Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> >
> > Still stands.
> >
> > > +
> > > + # gcc 7.1 introduced processor-specific ABI breakages related to parameter
> > > + # passing on ARM platforms. This generates a large number of messages
> > > + # during compilation with gcc >=7.1 until gcc 9. Silence them.
> > > + if (host_machine.cpu_family() == 'arm' and
> > > + cc.version().version_compare('>=7.1') and
> > > + cc.version().version_compare('<9'))
> > > + cpp_arguments += [
> > > + '-Wno-psabi',
> > > + ]
> > > + endif
> > > +endif
> > > +
> > > # We use C99 designated initializers for arrays as C++ has no equivalent
> > > # feature. Both gcc and clang support this extension, but recent
> > > # versions of clang generate a warning that needs to be disabled.
> > > @@ -71,16 +88,6 @@ if cc.has_argument('-Wno-c99-designator')
> > > ]
> > > endif
> > >
> > > -# gcc 7.1 introduced processor-specific ABI breakages related to parameter
> > > -# passing on ARM platforms. This generates a large number of messages during
> > > -# compilation with gcc >=7.1 until gcc 9. Silence them.
> > > -if (host_machine.cpu_family() == 'arm' and cc.get_id() == 'gcc' and
> > > - cc.version().version_compare('>=7.1') and cc.version().version_compare('<9'))
> > > - cpp_arguments += [
> > > - '-Wno-psabi',
> > > - ]
> > > -endif
> > > -
> > > c_arguments += common_arguments
> > > cpp_arguments += common_arguments
> > >
>
> --
> Regards,
>
> Laurent Pinchart
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list