beanz (Chris Bieneman)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 20 2014, 4:35 PM (152 w, 2 d)

Recent Activity

Tue, Jul 18

beanz accepted D35219: Generate a separate compile_commands.json DB for external projects.

LGTM!

Tue, Jul 18, 2:00 PM
beanz added inline comments to D33048: [CMake] runtimes test targets need to depend on LLVM tools.
Tue, Jul 18, 1:46 PM
beanz added inline comments to D33048: [CMake] runtimes test targets need to depend on LLVM tools.
Tue, Jul 18, 1:44 PM
beanz added a comment to D35077: [RFC] Build LLVM-C.dll on MSVC that exports only the C API.

I don't think this should be added as a new tool. We support generating LLVM-C.dylib on Darwin via the tools/llvm-shlib tool. It would be better if this were added there instead of completely separate.

Tue, Jul 18, 1:41 PM
beanz accepted D34375: [libunwind][CMake] Set library dir to be LLVM's intermediate output dir.

LGTM!

Tue, Jul 18, 1:38 PM
beanz added a comment to D35346: [CMake] Enable buildings builtins for Darwin as part of runtimes.

So... Looking at the CMake in compiler-rt, COMPILER_RT_DEFAULT_TARGET_ONLY is only interpreted in a NOT APPLE block, so this would really not behave the way you intend. I think it is better to not support this on Darwin for the time being and provide a hard error rather than behaving incorrectly.

Tue, Jul 18, 1:35 PM
beanz added a comment to D35346: [CMake] Enable buildings builtins for Darwin as part of runtimes.

I'm suspicious that this would work as intended. What builtin libraries get built if your triple is Darwin?

Tue, Jul 18, 1:31 PM
beanz accepted D35343: [CMake] Set toolchain tools in cross-target runtimes build.

LGTM

Tue, Jul 18, 1:29 PM

Mon, Jul 10

beanz accepted D33760: [libunwind][CMake] Add install path variable to allow overriding the destination.

LGTM!

Mon, Jul 10, 9:22 AM
beanz accepted D33762: [libcxx][CMake] Add install path variable to allow overriding the destination.

This LGTM.

Mon, Jul 10, 9:22 AM
beanz accepted D33761: [libcxxabi][CMake] Add install path variable to allow overriding the destination.

LGTM

Mon, Jul 10, 9:20 AM
beanz accepted D33048: [CMake] runtimes test targets need to depend on LLVM tools.

LGTM.

Mon, Jul 10, 9:19 AM
beanz accepted D35080: [Docs] Updating CMake docs to include LLVM_REVERSE_ITERATION.

LGTM!

Mon, Jul 10, 8:33 AM
beanz accepted D35024: Remove obsolete "unset" in CMake configuration.

LGTM! Thanks for the cleanup!

Mon, Jul 10, 8:16 AM
beanz accepted D35023: Removes obsolete documentation section.

LGTM! Thanks for updating the docs. I am really happy to be able to purge that broken behavior from my mind!

Mon, Jul 10, 8:14 AM
beanz accepted D35017: [CMake] Use tools template for clangd and modularize.

LGTM!

Mon, Jul 10, 8:10 AM

Sun, Jul 9

beanz accepted D34520: Minor correctness improvements for LLVMTestingSupport.

LGTM!

Sun, Jul 9, 6:13 AM
beanz added a comment to D27701: [lit] Fix discovery test on Windows.

I think it is better to XFAIL Windows and have test coverage on non-windows systems than to have the tests not running anywhere.

Sun, Jul 9, 6:13 AM

Fri, Jul 7

beanz added inline comments to D33760: [libunwind][CMake] Add install path variable to allow overriding the destination.
Fri, Jul 7, 10:10 AM

Jun 21 2017

beanz accepted D33707: TableGen.cmake: Use DEPFILE for Ninja Generator with CMake>=3.7..

This looks great! Thanks for doing this.

Jun 21 2017, 3:00 PM

Jun 19 2017

beanz accepted D32816: [CMake] Support multi-target runtimes build.

CMake stuff all looks good to me!

Jun 19 2017, 11:48 AM
beanz added a comment to D34322: [lldb] Correctly escape newlines and backslashes in the JSON serializer.

I get your perspective, but holding up this relatively small patch that fixes a bug in existing code on an architectural disagreement seems like excessive bike shedding. If we assume that JSON is required for the use case would you have Kuba write a full JSON parser in LLVM to satisfy your distaste over the fact that we have multiple JSON parsers already? That seems like an unreasonable request just to fix a few small bugs in existing code.

Jun 19 2017, 9:39 AM · Restricted Project
beanz added a comment to D34322: [lldb] Correctly escape newlines and backslashes in the JSON serializer.

My suggestion would be to just use the YAML writer for now and leave a comment to make it clear that this is really JSON only.

Jun 19 2017, 9:25 AM · Restricted Project
beanz added inline comments to D32835: [compiler-rt] [cmake] Support generic installation.
Jun 19 2017, 9:14 AM
beanz added a comment to D32816: [CMake] Support multi-target runtimes build.

One question inline below.

Jun 19 2017, 9:06 AM

Jun 16 2017

beanz accepted D34282: Call cmake_minimum_required at the top of CMakeLists.txt.

LGTM

Jun 16 2017, 1:23 PM
beanz accepted D33662: [CMake] Introduce LLVM_TARGET_TRIPLE_ENV as an option to override LLVM_DEFAULT_TARGET_TRIPLE at runtime..

LGTM

Jun 16 2017, 1:19 PM

Jun 12 2017

beanz added inline comments to D32816: [CMake] Support multi-target runtimes build.
Jun 12 2017, 4:43 PM

May 31 2017

beanz added a comment to D22499: [llvm-config] Report --bindir based on LLVM_TOOLS_INSTALL_DIR.

That looks like that should work to me. Seems good. @tstellar, does this resolve your issue?

May 31 2017, 3:45 PM
beanz accepted D32595: CMakeLists: Don't set LLVM_MAIN_SRC_DIR when building stand-alone clang.

Ah. I see what you're saying. I was thinking of the mismatch in a different way, but your solution makes sense. LGTM.

May 31 2017, 3:25 PM
beanz accepted D32668: CMake: Split static library exports into their own export file.

Ah. Makes sense. Can we maybe change the name from "StaticExports" to "ComponentExports"? As @chapuni pointed out this really applies to the component libraries, which can be static or shared based on configuration (although I keep hoping nobody would ever use BUILD_SHARED_LIBS in a distribution). Otherwise, I think this patch is good.

May 31 2017, 3:20 PM
beanz added a comment to D33444: Fix an issue of creating symlinks when llvm is embedded.

Unfortunately that doesn't really explain the need for these patches. From searching GitHub it doesn't seem like your project is overriding CMAKE_CFG_INTDIR. What is your CMake command line?

May 31 2017, 1:52 PM
beanz added a comment to D22499: [llvm-config] Report --bindir based on LLVM_TOOLS_INSTALL_DIR.

Ah! I see now. Probably the only thing that needs to be changed is handling LLVM_TOOLS_INSTALL_DIR being absolute. Otherwise the patch seems good to me.

May 31 2017, 1:47 PM
beanz added a comment to D33444: Fix an issue of creating symlinks when llvm is embedded.

How are you configuring? Unless I'm misunderstanding something when CMAKE_CONFIGURATION_TYPES is set CMAKE_CFG_INTDIR will never be ".".

May 31 2017, 11:44 AM
beanz added a comment to D22499: [llvm-config] Report --bindir based on LLVM_TOOLS_INSTALL_DIR.

I'm confused. In what situation is llvm-config installed into a directory that isn't the LLVM_TOOLS_INSTALL_DIR? It seems to me that if llvm-config is installed in LLVM_TOOLS_INSTALL_DIR this should work.

May 31 2017, 11:39 AM
beanz added inline comments to D32835: [compiler-rt] [cmake] Support generic installation.
May 31 2017, 11:31 AM
beanz accepted D32817: [CMake] Build runtimes for Fuchsia targets.

This all looks great. I'm really excited to have such a complete toolchain build example in-tree. Thank you!

May 31 2017, 11:24 AM
beanz accepted D31687: CMake: Fix sphinx build with standalone clang.

LGTM.

May 31 2017, 11:23 AM
beanz added a reviewer for D32734: [CMake][runtimes] Set default directory for runtime libraries: phosek.

Sorry for the delay, I've been out of town. I want to loop @phosek into this. I don't think this patch is correct. When I build on Darwin the sanitizer libraries are put in the build directory under lib/clang/$version/darwin/lib (which is correct), and the other runtimes are built under /lib/. This all seems right to me, so I don't understand the problem you're experiencing.

May 31 2017, 11:22 AM
beanz added a comment to D32668: CMake: Split static library exports into their own export file.

I want to echo @chapuni's question. What is the use case where a distribution would install the CMake modules and not install the LLVM component libraries? It doesn't seem to me that there is any reason to install the CMake modules to support find_package without also installing the static archives.

May 31 2017, 11:19 AM
beanz accepted D32710: [CMake][runtimes] Add install target for runtimes builtins.

LGTM!

May 31 2017, 11:15 AM
beanz added a comment to D32595: CMakeLists: Don't set LLVM_MAIN_SRC_DIR when building stand-alone clang.

Is this really something we should be supporting? Building and testing clang with potentially out-of-sync lit or gtest seems undesirable to me.

May 31 2017, 11:15 AM
beanz accepted D32604: CMakeLists: Deprecate using llvm-config to find llvm install path.

LGTM!

May 31 2017, 11:05 AM
beanz added inline comments to D32816: [CMake] Support multi-target runtimes build.
May 31 2017, 10:44 AM
beanz added inline comments to D32930: New framework for lldb client-server communication tests..
May 31 2017, 10:22 AM

May 30 2017

beanz accepted D32930: New framework for lldb client-server communication tests..

One small comment below. In general I agree with the thoughts here, and I think that this is a huge step forward for testing the debug server components.

May 30 2017, 8:54 AM

May 10 2017

beanz created D33048: [CMake] runtimes test targets need to depend on LLVM tools.
May 10 2017, 9:09 AM

May 8 2017

beanz accepted D32815: [clang-tidy][CMake] Make clang-tidy usable as distribution component.

Yep, this seems straight forward.

May 8 2017, 7:51 PM · Restricted Project
beanz added a comment to D32734: [CMake][runtimes] Set default directory for runtime libraries.

What is the problem you're trying to solve here? This seems odd to me.

May 8 2017, 1:24 PM
beanz added inline comments to D25886: [Test Suite] Properly respect --framework option.
May 8 2017, 11:39 AM

Apr 27 2017

beanz accepted D32603: Build the Apple-style stage2 with modules and full debug info.

SWEET! this is great to see! One small comment inline, otherwise LGTM.

Apr 27 2017, 11:19 AM
beanz accepted D32600: Resurrect pselect MainLoop implementation.

This looks like a nice improvement. There are some formatting inconsistencies, can you run clang-format?

Apr 27 2017, 11:02 AM

Apr 21 2017

beanz accepted D32357: Add more arguments to SocketAddress::GetAddressInfo.

This is awesome! Thanks!

Apr 21 2017, 2:06 PM

Apr 18 2017

beanz accepted D32125: [LLVM][MIPS] Fix different definition of off_t in LLDB and LLVM.

This looks good to me.

Apr 18 2017, 1:25 PM

Apr 17 2017

beanz added a comment to D32125: [LLVM][MIPS] Fix different definition of off_t in LLDB and LLVM.

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.

Apr 17 2017, 10:54 AM
beanz updated the diff for D31823: Update LLDB Host to support IPv6 over TCP.

Removing code I accidentally left in that was from debugging, and moving some duplicated code that @labath spotted out of the ifdef.

Apr 17 2017, 10:39 AM
beanz added inline comments to D32079: Ask Python where platform specific lib dir is..
Apr 17 2017, 10:31 AM
beanz updated the diff for D31823: Update LLDB Host to support IPv6 over TCP.

Updating patches to reflect feedback from zturner.

Apr 17 2017, 10:16 AM

Apr 14 2017

beanz updated the diff for D31823: Update LLDB Host to support IPv6 over TCP.

Updating to use MainLoop class, and refactor MainLoop class to operate on Windows.

Apr 14 2017, 3:39 PM

Apr 12 2017

beanz added a reviewer for D31823: Update LLDB Host to support IPv6 over TCP: clayborg.

Adding Greg to the reviewers list.

Apr 12 2017, 2:15 PM
beanz added a reviewer for D31824: Update DebugServer to support IPv6 over TCP: clayborg.

Adding Greg to the reviewers.

Apr 12 2017, 2:14 PM
beanz added inline comments to D31822: [NFC] Adding a new wrapper for getaddrinfo.
Apr 12 2017, 2:07 PM
beanz accepted D31985: Support: Add a VCSRevision.h header file..

LGTM.

Apr 12 2017, 1:43 PM
beanz accepted D31942: [CMake][runtimes] Use -nodefaultlibs for the runtimes build.

LGTM!

Apr 12 2017, 1:26 PM
beanz updated the diff for D31969: [CMake] Support generating Config.h.

Fixing up the include guard as per feedback from zturner, and fixing up an install logic error that I spoted.

Apr 12 2017, 1:24 PM
beanz added a comment to D31969: [CMake] Support generating Config.h.

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 12 2017, 1:20 PM

Apr 11 2017

beanz updated the diff for D31969: [CMake] Support generating Config.h.

Updating the hard coded Config.h

Apr 11 2017, 6:47 PM
beanz updated the diff for D31969: [CMake] Support generating Config.h.

Actually uploading the updates this time...

Apr 11 2017, 6:36 PM
beanz updated the diff for D31969: [CMake] Support generating Config.h.
  • 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
Apr 11 2017, 6:35 PM
beanz created D31969: [CMake] Support generating Config.h.
Apr 11 2017, 6:19 PM
beanz added a comment to D31823: Update LLDB Host to support IPv6 over TCP.

@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.

Apr 11 2017, 4:09 PM
beanz accepted D31858: Reland "[CMake][libunwind] Use -nodefaultlibs for CMake checks".

This looks good to me!

Apr 11 2017, 7:38 AM

Apr 7 2017

beanz updated the diff for D31824: Update DebugServer to support IPv6 over TCP.

Fixing a small bug parsing the command line arguments for IPv6 addresses.

Apr 7 2017, 12:23 PM
beanz created D31824: Update DebugServer to support IPv6 over TCP.
Apr 7 2017, 11:58 AM
beanz created D31823: Update LLDB Host to support IPv6 over TCP.
Apr 7 2017, 11:56 AM
beanz created D31822: [NFC] Adding a new wrapper for getaddrinfo.
Apr 7 2017, 11:55 AM
beanz added inline comments to D31696: Automatically add include-what-you-use for when building in tree.
Apr 7 2017, 11:54 AM
beanz accepted D31773: CMake: Move sphinx detection into AddSphinxTarget.cmake.

LGTM!

Apr 7 2017, 11:48 AM
beanz updated the diff for D31357: Support Unit Testing debugserver.

Fixing variable naming conventions

Apr 7 2017, 8:52 AM

Apr 6 2017

beanz added a comment to D31357: Support Unit Testing debugserver.

I will fix up the naming conventions. Switching back and forth between LLVM and LLDB conventions has done a number on my brain.

Apr 6 2017, 10:41 AM
beanz added a reviewer for D31357: Support Unit Testing debugserver: jingham.

Adding Jim as a reviewer.

Apr 6 2017, 8:24 AM
beanz updated the diff for D31357: Support Unit Testing debugserver.

Some cleanup to the test case:

Apr 6 2017, 8:22 AM

Apr 5 2017

beanz accepted D31639: [CMake][libcxxabi] Use -nodefaultlibs for CMake checks .

This looks reasonable to me.

Apr 5 2017, 2:01 PM
beanz accepted D31718: Disable the darwin_log tests as a category while we clean them up.

This seems like a reasonable workaround until we can figure out a better solution. Thanks Sean!

Apr 5 2017, 1:38 PM · Restricted Project
beanz added a comment to D31687: CMake: Fix sphinx build with standalone clang.

I think putting this into AddSphinxTarget.cmake would be a better way to handle it. Also I have one small inline comment.

Apr 5 2017, 1:13 PM

Apr 3 2017

beanz accepted D31447: [Driver] Add option to print the resource directory.

One minor nitpick on the test case, but otherwise this looks fine to me.

Apr 3 2017, 3:31 PM
beanz updated subscribers of D31367: Expression: add missing linkage to RuntimeDyld component.

@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.

Apr 3 2017, 3:27 PM · Restricted Project
beanz accepted D31617: [CMake][libcxx] Use builtins rather than gcc_s when compiler-rt is requested.

Looks reasonable to me.

Apr 3 2017, 3:11 PM
beanz accepted D30508: Align all scalar numbers to LLVM_YAML_IS_FLOW_SEQUENCE_VECTOR.

LGTM, sorry for the delay!

Apr 3 2017, 11:06 AM

Apr 2 2017

jdoerfert awarded D31570: Revert "Remove autoconf support" a Party Time token.
Apr 2 2017, 3:11 AM

Apr 1 2017

beanz created D31570: Revert "Remove autoconf support".
Apr 1 2017, 8:32 AM

Mar 30 2017

beanz added a comment to D31367: Expression: add missing linkage to RuntimeDyld component.

Please revert your patch. It is *not* the right solution and is masking underlying problems.

Mar 30 2017, 11:00 AM · Restricted Project
beanz added a comment to D31367: Expression: add missing linkage to RuntimeDyld component.

This is definitely not the right fix. Please revert.

Mar 30 2017, 10:06 AM · Restricted Project

Mar 29 2017

beanz accepted D30906: Revert r297545 - Revert r297516 - Respect CMAKE_INSTALL_MANDIR for sphinx generated manpages.

Looks reasonable to me. Thanks!

Mar 29 2017, 3:11 PM

Mar 28 2017

beanz accepted D31436: [yaml2obj] Enable and fix tests.

Thanks for catching this. I didn't even notice that I had written such bad tests!

Mar 28 2017, 3:18 PM
beanz added a comment to D31363: [libc++] Remove cmake glob for source files.

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.

Mar 28 2017, 11:06 AM
beanz added a comment to D30906: Revert r297545 - Revert r297516 - Respect CMAKE_INSTALL_MANDIR for sphinx generated manpages.

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 28 2017, 10:57 AM

Mar 27 2017

beanz updated the diff for D31357: Support Unit Testing debugserver.

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.

Mar 27 2017, 3:55 PM
beanz added a comment to D31357: Support Unit Testing debugserver.

@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.

Mar 27 2017, 12:34 PM
beanz updated the diff for D31357: Support Unit Testing debugserver.

Fleshed out the unit test logic to be more meaningful.

Mar 27 2017, 12:17 PM