This is an archive of the discontinued LLVM Phabricator instance.

Replace __ANDROID_NDK__ with simply ANDROID
ClosedPublic

Authored by labath on Dec 1 2016, 9:59 AM.

Details

Summary

This replaces all the uses of the ANDROID_NDK define with ANDROID. This is a
preparatory step to remove our custom android toolchain file and rely on the
standard android NDK one instead. Our toolchain file defined both ANDROID and
ANDROID_NDK, while the NDK one just defines ANDROID. So, make things
consistent and just use ANDROID everywhere.

I haven't yet removed the cmake variable with the same name, as we will need to
do something completely different there -- NDK toolchain defines
CMAKE_SYSTEM_NAME to Android, while our current one pretends it's linux.

Diff Detail

Repository
rL LLVM

Event Timeline

labath updated this revision to Diff 79936.Dec 1 2016, 9:59 AM
labath retitled this revision from to Replace __ANDROID_NDK__ with simply ANDROID.
labath updated this object.
labath added reviewers: tberghammer, zturner.
labath added a subscriber: lldb-commits.
danalbert added inline comments.Dec 1 2016, 10:08 AM
cmake/platforms/Android.cmake
36 ↗(On Diff #79936)

You're removing it a line before just to add it back?

__ANDROID__ is probably the one you want. That one is defined by Clang (and GCC if that matters) for any Android target, so no need to worry about dealing with it in cmake (or any other build system).

This isn't a concern right now, but if LLDB ever needed to become part of the Android platform build, ANDROID doesn't actually mean ANDROID; it gets set for host modules too :(

labath planned changes to this revision.Dec 1 2016, 10:17 AM

Thanks for the suggestion. I'll try using __ANDROID__ instead.

cmake/platforms/Android.cmake
36 ↗(On Diff #79936)

I have no idea who wrote this file, but a bit messy, and that's why I am trying to get rid of it. :)

I didn't know about the __ANDROID__ thing. I'll try using that instead (and yes, I'd still like this to compile with gcc -- that's what we are using currently, although we may switch once this is sorted out).

tberghammer added inline comments.Dec 2 2016, 2:43 AM
include/lldb/Core/RegularExpression.h
34 ↗(On Diff #79936)

In most case you use "#ifdef". It would be nice to uniformalize

source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp
31 ↗(On Diff #79936)

In most case you use "#ifdef". It would be nice to uniformalize

source/Plugins/Process/Utility/RegisterContextMacOSXFrameBackchain.cpp
123 ↗(On Diff #79936)

(unrelated): What does an ANDROID specific define do in a MacOSX specific file?

labath updated this revision to Diff 80043.Dec 2 2016, 2:58 AM

Update to use ANDROID

labath added a comment.Dec 2 2016, 3:03 AM

I've tried to unify this slightly, but sometimes further unification would make things inconsistent with surrounding preprocessor directives.

I was suprised by the presence of some of these ifdefs as well -- some of them seem completely unnecessary. I think that at some point I'll have to make another pass at this and clean them up.

labath updated this revision to Diff 80046.Dec 2 2016, 3:16 AM

Remove ifdef in driver.cpp -- it is completely unnecessary

labath updated this revision to Diff 80047.Dec 2 2016, 3:24 AM

A bit more cleanup.

This revision was automatically updated to reflect the committed changes.