[Lldb-commits] [PATCH] D58838: Remove tautological #ifdefs

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rL355457: Remove tautological #ifdefs (NFC) (authored by adrian, committed by ). Herald added a project: LLVM. Herald added

[Lldb-commits] [PATCH] D58838: Remove tautological #ifdefs

davide accepted this revision. davide added a comment. LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58838/new/ https://reviews.llvm.org/D58838 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D58838: Remove tautological #ifdefs

aprantl added a comment. Ping. As far as I can tell, now this patch should be entirely NFC. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58838/new/ https://reviews.llvm.org/D58838 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D58838: Remove tautological #ifdefs

aprantl updated this revision to Diff 189003. aprantl added a comment. Removed DIEStack from this patch it distracts. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58838/new/ https://reviews.llvm.org/D58838 Files: lldb/source/Target/ExecutionContext.cpp Index:

[Lldb-commits] [PATCH] D58838: Remove tautological #ifdefs

jingham added a comment. In D58838#1415646 , @aprantl wrote: > In D58838#1415637 , @zturner wrote: > > > Yea it would be nice if we could remove all of the `LLDB_CONFIGURATION_xxx` > > macros and just use either

[Lldb-commits] [PATCH] D58838: Remove tautological #ifdefs

aprantl added a comment. Also I should note that we can only use LLVM defines that are provided by the Xcode project until we reach the point where Apple builds LLDB with CMake and the CMake-generated Xcode file is good enough to replace the hand-written one. This is a transitional

[Lldb-commits] [PATCH] D58838: Remove tautological #ifdefs

aprantl added a comment. In D58838#1415637 , @zturner wrote: > Yea it would be nice if we could remove all of the `LLDB_CONFIGURATION_xxx` > macros and just use either the LLVM ones or standard ones such as NDEBUG I generally agree with this, but we

[Lldb-commits] [PATCH] D58838: Remove tautological #ifdefs

zturner added a comment. Yea it would be nice if we could remove all of the `LLDB_CONFIGURATION_xxx` macros and just use either the LLVM ones or standard ones such as NDEBUG CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58838/new/ https://reviews.llvm.org/D58838

[Lldb-commits] [PATCH] D58838: Remove tautological #ifdefs

aprantl added a comment. This doesn't apply to DieStack since it isn't an assert, but DieStack is dead code anyway... CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58838/new/ https://reviews.llvm.org/D58838 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D58838: Remove tautological #ifdefs

aprantl added a comment. But I'm sure that `LLDB_CONFIGURATION_BUILD_AND_INTEGRATION` also defines `NDEBUG` so the assert should be a noop, right? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58838/new/ https://reviews.llvm.org/D58838

[Lldb-commits] [PATCH] D58838: Remove tautological #ifdefs

clayborg added a comment. I think we mostly wanted to catch these in the test suite. Probably best to convert to lldbassert CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58838/new/ https://reviews.llvm.org/D58838 ___ lldb-commits mailing

[Lldb-commits] [PATCH] D58838: Remove tautological #ifdefs

clayborg added a comment. So it affects Apple directly! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58838/new/ https://reviews.llvm.org/D58838 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D58838: Remove tautological #ifdefs

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. LLDB_CONFIGURATION_BUILD_AND_INTEGRATION doesn't have them. So you can change these to #if !defined(LLDB_CONFIGURATION_BUILD_AND_INTEGRATION) CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D58838: Remove tautological #ifdefs

aprantl created this revision. aprantl added a reviewer: clayborg. These conditions appear to always be true. @clayborg do you remember the reason why these exist? https://reviews.llvm.org/D58838 Files: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp