| Message ID | 20210204143736.1762344-1-kieran.bingham@ideasonboard.com |
|---|---|
| State | Rejected |
| Headers | show |
| Series |
|
| Related | show |
Hi Kieran, Thank you for the patch. On Thu, Feb 04, 2021 at 02:37:36PM +0000, Kieran Bingham wrote: > Utilise the new compiler specific __nodiscard to maintain coding style. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > > Only applicable along with Laurent's [[nodiscard]] series. I'm open to discussing this, but as replied in another e-mail, I was envisioning the __nodiscard macro as a temporary fix until we declare the public API to require C++17. I was annoyed by the C++ [[...]] attributes initially, but after some time I got used to them and I don't mind them anymore. Using native language features is usually better from a readability point of view compared to wrapping them in custom macros, as the latter needs additional knowledge from the reader. > src/android/jpeg/exif.cpp | 2 +- > src/android/jpeg/exif.h | 3 ++- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp > index fdd46070ba7c..ed995ca13167 100644 > --- a/src/android/jpeg/exif.cpp > +++ b/src/android/jpeg/exif.cpp > @@ -515,7 +515,7 @@ std::u16string Exif::utf8ToUtf16(const std::string &str) > return ret; > } > > -[[nodiscard]] int Exif::generate() > +__nodiscard int Exif::generate() > { > if (exifData_) { > free(exifData_); > diff --git a/src/android/jpeg/exif.h b/src/android/jpeg/exif.h > index b0d614027ede..67dbd20524d8 100644 > --- a/src/android/jpeg/exif.h > +++ b/src/android/jpeg/exif.h > @@ -13,6 +13,7 @@ > > #include <libexif/exif-data.h> > > +#include <libcamera/compiler.h> > #include <libcamera/geometry.h> > #include <libcamera/span.h> > > @@ -75,7 +76,7 @@ public: > void setWhiteBalance(WhiteBalance wb); > > libcamera::Span<const uint8_t> data() const { return { exifData_, size_ }; } > - [[nodiscard]] int generate(); > + __nodiscard int generate(); > > private: > ExifEntry *createEntry(ExifIfd ifd, ExifTag tag);
Hi Laurent, On 04/02/2021 16:37, Laurent Pinchart wrote: > Hi Kieran, > > Thank you for the patch. > > On Thu, Feb 04, 2021 at 02:37:36PM +0000, Kieran Bingham wrote: >> Utilise the new compiler specific __nodiscard to maintain coding style. >> >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> >> --- >> >> Only applicable along with Laurent's [[nodiscard]] series. > > I'm open to discussing this, but as replied in another e-mail, I was > envisioning the __nodiscard macro as a temporary fix until we declare > the public API to require C++17. I was annoyed by the C++ [[...]] Yup, I find it a little 'ugly' but no matter. > attributes initially, but after some time I got used to them and I don't > mind them anymore. Using native language features is usually better from > a readability point of view compared to wrapping them in custom macros, > as the latter needs additional knowledge from the reader. Don't worry - if this is temporary, then don't worry about this patch. -- Kieran > >> src/android/jpeg/exif.cpp | 2 +- >> src/android/jpeg/exif.h | 3 ++- >> 2 files changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp >> index fdd46070ba7c..ed995ca13167 100644 >> --- a/src/android/jpeg/exif.cpp >> +++ b/src/android/jpeg/exif.cpp >> @@ -515,7 +515,7 @@ std::u16string Exif::utf8ToUtf16(const std::string &str) >> return ret; >> } >> >> -[[nodiscard]] int Exif::generate() >> +__nodiscard int Exif::generate() >> { >> if (exifData_) { >> free(exifData_); >> diff --git a/src/android/jpeg/exif.h b/src/android/jpeg/exif.h >> index b0d614027ede..67dbd20524d8 100644 >> --- a/src/android/jpeg/exif.h >> +++ b/src/android/jpeg/exif.h >> @@ -13,6 +13,7 @@ >> >> #include <libexif/exif-data.h> >> >> +#include <libcamera/compiler.h> >> #include <libcamera/geometry.h> >> #include <libcamera/span.h> >> >> @@ -75,7 +76,7 @@ public: >> void setWhiteBalance(WhiteBalance wb); >> >> libcamera::Span<const uint8_t> data() const { return { exifData_, size_ }; } >> - [[nodiscard]] int generate(); >> + __nodiscard int generate(); >> >> private: >> ExifEntry *createEntry(ExifIfd ifd, ExifTag tag); >
diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp index fdd46070ba7c..ed995ca13167 100644 --- a/src/android/jpeg/exif.cpp +++ b/src/android/jpeg/exif.cpp @@ -515,7 +515,7 @@ std::u16string Exif::utf8ToUtf16(const std::string &str) return ret; } -[[nodiscard]] int Exif::generate() +__nodiscard int Exif::generate() { if (exifData_) { free(exifData_); diff --git a/src/android/jpeg/exif.h b/src/android/jpeg/exif.h index b0d614027ede..67dbd20524d8 100644 --- a/src/android/jpeg/exif.h +++ b/src/android/jpeg/exif.h @@ -13,6 +13,7 @@ #include <libexif/exif-data.h> +#include <libcamera/compiler.h> #include <libcamera/geometry.h> #include <libcamera/span.h> @@ -75,7 +76,7 @@ public: void setWhiteBalance(WhiteBalance wb); libcamera::Span<const uint8_t> data() const { return { exifData_, size_ }; } - [[nodiscard]] int generate(); + __nodiscard int generate(); private: ExifEntry *createEntry(ExifIfd ifd, ExifTag tag);
Utilise the new compiler specific __nodiscard to maintain coding style. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> --- Only applicable along with Laurent's [[nodiscard]] series. src/android/jpeg/exif.cpp | 2 +- src/android/jpeg/exif.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)