dcleblanc / SafeInt

SafeInt is a class library for C++ that manages integer overflows.
MIT License
216 stars 37 forks source link

Instrument return values with [[nodiscard]] when compiler supports it? #33

Closed bgianfo closed 2 years ago

bgianfo commented 3 years ago

It would be nice if the functions which return values like SafeAdd(..) were marked as [[nodiscard]] to catch bugs where the caller doesn't properly observe the return value of the operation.

You can feature detect [[nodiscard]] support by using the __has_cpp_attribute(nodiscard) macro.

dcleblanc commented 3 years ago

Sure, that’s a good suggestion, I’ll look into adding that to the next version.

From: Brian Gianforcaro notifications@github.com Sent: Monday, February 1, 2021 3:16 PM To: dcleblanc/SafeInt SafeInt@noreply.github.com Cc: Subscribed subscribed@noreply.github.com Subject: [dcleblanc/SafeInt] Instrument return values with [[nodiscard]] when compiler supports it? (#33)

It would be nice if the functions which return values like SafeAdd(..) were marked as [[nodiscard]]https://en.cppreference.com/w/cpp/language/attributes/nodiscard to catch bugs where the caller doesn't properly observe the return value on the operation.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/dcleblanc/SafeInt/issues/33, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AHX6YL53KQ2OONKNVSLAPIDS44Y3ZANCNFSM4W5UDPVA.

dcleblanc commented 2 years ago

I know this has been a while, but it is finally being worked on in a branch.

dcleblanc commented 2 years ago

This got merged in today, and currently I'm only supporting [[nodiscard]], not the older declspec or attribute (depending on compiler). Sorting out which of those support what is a bit of a pain. I did find how to detect if MSVC has the older annotation, and could support that if needed. Please let me know if what I have is sufficient, or if you want down level compiler support.

bgianfo commented 2 years ago

What you have looks great, we don't need down level support.

Thanks!