Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix warning (-Wexpansion-to-defined) in Xcode 9.3 #765

Merged
merged 2 commits into from Apr 2, 2018

Conversation

anderscarling
Copy link
Contributor

@anderscarling anderscarling commented Feb 1, 2018

Xcode 9.3 seems to run with -Wexpansion-to-defined by default, which creates a warning in both the Mixpanel target and (due to this happening in headers) in my app target.

I believe this should fix is, but probably if someone more experience in macro expansion trickery could weight in.

Fix is adapted from https://stackoverflow.com/a/42074134

@wileykestner
Copy link

@anderscarling Have you received any feedback about this PR? I'm encountering the same issue - have you been using this patch successfully?

@anderscarling
Copy link
Contributor Author

@wileykestner I've heard nothing - and I've not really used the patch in production builds or anything :/

@wileykestner
Copy link

@anderscarling Thanks for the update - I think we may give it a try in our staging environment.

@anderscarling
Copy link
Contributor Author

Hope it works for you! It feels like simple enough a change not to mess anything up - but as I've no real knowledge in the intricate behaviors of C macros I don't want to make any promises. :)

@kdawgwilk
Copy link

I was just about to open a PR with a fix for this. I think it can be simplified down to this:

#if defined(MIXPANEL_TVOS) || defined(MIXPANEL_WATCHOS) || defined(MIXPANEL_MACOS)
    #define MIXPANEL_NO_REACHABILITY_SUPPORT 1
    #define MIXPANEL_NO_AUTOMATIC_EVENTS_SUPPORT 1
    #define MIXPANEL_NO_NOTIFICATION_AB_TEST_SUPPORT 1
    #define MIXPANEL_NO_CONNECT_INTEGRATION_SUPPORT 1
#elseif defined(MIXPANEL_WATCHOS) || defined(MIXPANEL_MACOS)
    #define MIXPANEL_NO_UIAPPLICATION_ACCESS 1
#elseif defined(MIXPANEL_WATCHOS)
    #define MIXPANEL_FLUSH_IMMEDIATELY 1
#endif

You don't need to actually define them as zero since not defining them at all has the same effect

@kdawgwilk
Copy link

And

#if defined(MIXPANEL_TVOS) || defined(MIXPANEL_WATCHOS) || defined(MIXPANEL_MACOS)
    #define MIXPANEL_NO_NETWORK_ACTIVITY_INDICATOR 1
#endif

@kdawgwilk
Copy link

Now that Xcode 9.3 is released it seems that this should get released soon so we can rid our projects of these warnings

@charliebartel
Copy link

What is the status on getting this released?

@delebedev
Copy link

@zihejia could you or other maintainers make a release with this change, please? Xcode 9.3 is now live

@anderscarling
Copy link
Contributor Author

I pushed new commits where i rebased on latest master, and removed the #else-cases that #defined zero values (as per suggestion from @kdawgwilk).

I didn't pull in the #elseifs from @kdawgwilk though, as I think they would actually block MIXPANEL_NO_UIAPPLICATION_ACCESS and MIXPANEL_FLUSH_IMMEDIATELY from ever being defined.

Travis CI seems to fail for unrelated reason.

@zihejia
Copy link
Contributor

zihejia commented Apr 2, 2018

@garnett yes, we will be working on this.

Copy link
Contributor

@zihejia zihejia left a comment

Choose a reason for hiding this comment

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

Thanks Anders for making the changes

@zihejia zihejia merged commit 90e58da into mixpanel:master Apr 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants