- User Since
- Aug 20 2014, 4:35 PM (144 w, 1 d)
Wed, May 10
Mon, May 8
Yep, this seems straight forward.
What is the problem you're trying to solve here? This seems odd to me.
Thu, Apr 27
SWEET! this is great to see! One small comment inline, otherwise LGTM.
This looks like a nice improvement. There are some formatting inconsistencies, can you run clang-format?
Apr 21 2017
This is awesome! Thanks!
Apr 18 2017
This looks good to me.
Apr 17 2017
Can you please move this check into HandleLLVMOptions.cmake? By putting it into a module that is vended as part of LLVM's packaging then LLVM subprojects can have consistent settings when building out-of-tree.
Removing code I accidentally left in that was from debugging, and moving some duplicated code that @labath spotted out of the ifdef.
Updating patches to reflect feedback from zturner.
Apr 14 2017
Updating to use MainLoop class, and refactor MainLoop class to operate on Windows.
Apr 12 2017
Adding Greg to the reviewers list.
Adding Greg to the reviewers.
Fixing up the include guard as per feedback from zturner, and fixing up an install logic error that I spoted.
My intention in this patch is not in any way to adversely impact the Xcode project, which is the supported and documented way to build LLDB on OS X (http://lldb.llvm.org/build.html#BuildingLldbOnMacOSX).
Apr 11 2017
Updating the hard coded Config.h
Actually uploading the updates this time...
- Fixing installation to make sure CMake always installs the generated Config.h
- Removing LLDB_CONFIG_FCNTL_GETPATH_SUPPORTED becasue we really don't need it
@labath, I could adapt this into the MainLoop class, but I would actually want to change how that class hierarchy is implemented. Regardless of the event handling/polling model you use much of the code is identical between the classes. I'd rather get rid of MainLoopPosix and MainLoopBase and instead just implement a portable MainLoop class.
This looks good to me!
Apr 7 2017
Fixing a small bug parsing the command line arguments for IPv6 addresses.
Fixing variable naming conventions
Apr 6 2017
I will fix up the naming conventions. Switching back and forth between LLVM and LLDB conventions has done a number on my brain.
Adding Jim as a reviewer.
Some cleanup to the test case:
Apr 5 2017
This looks reasonable to me.
This seems like a reasonable workaround until we can figure out a better solution. Thanks Sean!
I think putting this into AddSphinxTarget.cmake would be a better way to handle it. Also I have one small inline comment.
Apr 3 2017
One minor nitpick on the test case, but otherwise this looks fine to me.
@mgorny, because of differences in linker semantics between Darwin and ELF, I can't reproduce the failure you have locally. I think that the patch below works around it in a more-portable way.
Looks reasonable to me.
LGTM, sorry for the delay!
Apr 2 2017
Apr 1 2017
Mar 30 2017
Please revert your patch. It is *not* the right solution and is masking underlying problems.
This is definitely not the right fix. Please revert.
Mar 29 2017
Looks reasonable to me. Thanks!
Mar 28 2017
Thanks for catching this. I didn't even notice that I had written such bad tests!
LLVM has a CMake module "LLVMProcessSources.cmake" which verifies that all source files are referenced in the CMakeLists file. Since it is part of the LLVM distributed modules, you can re-use it in libcxx.
The one downside to this approach is that when you use an absolute path as the DESTINATION argument to install you can't change the install root without reconfiguring. When you use relative paths you can.
Mar 27 2017
Added a note to the unit test CMake file about why the tests are where they are. Generally we isolate debugserver from the rest of LLDB, and this comment explains the breach of isolation.
@jingham I put the unit tests at the top because they depend on LLDB's Host library (at least the current tests do). I'm attempting to write tests which cover the socket communication between LLDB and debugserver by sending data between the two using each side of the API.
Fleshed out the unit test logic to be more meaningful.
Mar 24 2017
Xcode is pretty magic to me. I don't know how to do much of anything in it other than build. I think the right solution would be to take most of the source files out of the debugserver target and generate a static archive from them, then have debugserver and the debugserverTest target link the static archive.
Mar 22 2017
Mar 21 2017
One minor style comment, otherwise looks good to me.
Cool! Seems to me like this patch addresses everyones issues, and I think the patch looks good. Thanks for iterating on this!
Mar 20 2017
@inglorion I believe the reason we had to set the policy to OLD is because the auto-passed variable masks the explicitly passed one in try_compile. If that is the case, we need the same change in the Darwin code path too.
This patch looks fine to me, however I'd like to see the resolution on D31098 before landing this. If we change the CMake policy CMP0056 to NEW, I think that will automatically address this issue resulting in us not needing this patch.
@davidxl, do you recall how the failure that caused you to make the policy change exhibited itself? It seems to me like we were working around the behavior of the old policy setting by passing the linker options through manually. I wonder if the reason for the error was CMake masking those liker flags under the new policy.
This looks like a great cleanup. Thanks!
Mar 17 2017
@mgorny, we should only support this on shared libraries that we intend as vended libraries, not all libraries. For example we don't really want to support the CMake BUILD_SHARED_LIBS option's libraries as vended libraries on any platform.
Mar 14 2017
This is great, thanks!
Ugh... I really don't like how LLVM_UTILS_PROVIDED is implemented, but I guess it is fine. This patch is good.
Mar 13 2017
Forgot a semi-colon...
Updates based on feedback from Jason and Zachary.
Removing some extra changes that accidentally came along for the ride in my initial upload.
Mar 7 2017
This patch is fine, just be warned that eventually I really want to kill the projects subdirectory entirely.
Mar 6 2017
Mar 3 2017
Updating to address Paul's feedback about DWARF32/DWARF64.
Updated based on feedback from Adrian.
Mar 2 2017
Updates based on feedback from Adrian.
Mar 1 2017
- Reducing the patch to just the NFC changes
Feb 24 2017
This is for LLVM. Will land shortly.
This all looks pretty great to me.
Feb 22 2017