Skip to content
This repository has been archived by the owner on Mar 26, 2020. It is now read-only.

Silence some optional Xcode 9 warnings #348

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

steipete
Copy link
Contributor

This fixes an Analyzer warning that shows up in Xcode 9:
https://github.com/llvm-mirror/clang/blob/master/test/Analysis/number-object-conversion.cpp

34:20: warning: Converting a pointer value of type 'ObjcType' to a primitive boolean value; instead, either compare the pointer to nil or call -boolValue
            assert(x);

Also fixes an optional warning:

The __unused on the exception is needed when -Wunused-exception-parameter is enabled.

warning: unused exception parameter 'e' [-Wunused-exception-parameter]
        DJINNI_TRANSLATE_EXCEPTIONS()
        ^

Both are trivial changes that do not change functionality.
Thanks for making Djinni! We from PSPDFKit love it!

Copy link
Contributor

@artwyman artwyman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comment on the exception name for the proposed change. Otherwise this looks great, thanks.

@@ -30,6 +30,6 @@ namespace djinni {
::djinni::throwUnimplemented(__PRETTY_FUNCTION__, msg);

#define DJINNI_TRANSLATE_EXCEPTIONS() \
catch (const std::exception & e) { \
catch (__unused const std::exception & e) { \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be better to simply delete the name? It is truly unused, and removing the name is a more standard way to do it in C++.

@@ -29,7 +29,7 @@ struct Bool {

struct Boxed {
using ObjcType = NSNumber*;
static CppType toCpp(ObjcType x) noexcept { assert(x); return Bool::toCpp([x boolValue]); }
static CppType toCpp(ObjcType x) noexcept { assert(x != nil); return Bool::toCpp([x boolValue]); }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. Stylistically I generally do prefer to let pointers be treated as bools directly, but in this context I'm fine with changing the code to be more generally usable by people who don't like that style and turn on warnings against it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants