beanz (Chris Bieneman)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 20 2014, 4:35 PM (160 w, 5 d)

Recent Activity

Fri, Sep 8

beanz added a comment to D37414: [CMake] Add bootstrap option `CLANG_BOOTSTRAP_USE_LIBCXX`.

I'm not sure this change is sufficient to do what you're intending. I think you also need to pass the library path of the cxx library into the bootstrap configuration otherwise it will use the one from the sysroot.

Fri, Sep 8, 1:38 PM
beanz accepted D37450: [CMake][runtimes] Use the same configuration for non-target and "default" target.

LGTM!

Fri, Sep 8, 1:35 PM
beanz added inline comments to D37637: [CMake] Determine early on which projects are enabled.
Fri, Sep 8, 1:33 PM
beanz added inline comments to D37602: Properly hook debuginfo-tests up to lit and CMake.
Fri, Sep 8, 1:25 PM

Thu, Aug 31

beanz accepted D36598: cmake + xcode: prevent gtests from using includes from project root.

This LGTM.

Thu, Aug 31, 11:45 AM
beanz accepted D37245: [CMake][runtimes] Use target specific name for all runtimes targets.

As a note: at some point in the future I'd like us to get rid of the practice of using ":" to create lists of lists, instead using list of list names, which is a bit cleaner.

Thu, Aug 31, 11:44 AM

Tue, Aug 29

beanz accepted D35648: [CMake] Add `project` in `runtime/CMakeList.txt`.

LGTM.

Tue, Aug 29, 3:14 PM
beanz added inline comments to D35648: [CMake] Add `project` in `runtime/CMakeList.txt`.
Tue, Aug 29, 3:13 PM

Mon, Aug 28

beanz added a comment to D37027: Fix cmake check for futimens when deploying to earlier macOS releases..

LGTM.

Mon, Aug 28, 3:09 PM
beanz accepted D37027: Fix cmake check for futimens when deploying to earlier macOS releases..
Mon, Aug 28, 3:09 PM
beanz added inline comments to D35648: [CMake] Add `project` in `runtime/CMakeList.txt`.
Mon, Aug 28, 9:45 AM
beanz accepted D35133: [cmake] Use new GetSVN.cmake SOURCE_DIRS parameter.

Since the dependent patch landed, this LGTM too.

Mon, Aug 28, 9:41 AM

Tue, Aug 22

beanz added inline comments to D35648: [CMake] Add `project` in `runtime/CMakeList.txt`.
Tue, Aug 22, 3:54 PM

Aug 10 2017

beanz accepted D36541: [CMake] Include LLVMFuzzer in Fuchsia toolchain.

LGTM!

Aug 10 2017, 5:14 PM
beanz accepted D36540: [CMake] Add install target for LLVMFuzzer.

LGTM!

Aug 10 2017, 5:13 PM
beanz accepted D36349: [CMake] Build sanitized C++ runtimes for Fuchsia.

This also looks good to me.

Aug 10 2017, 5:05 PM
beanz accepted D36348: [CMake][runtimes] Support for building target variants.

This looks pretty cool to me.

Aug 10 2017, 5:03 PM

Aug 9 2017

beanz accepted D35077: [RFC] Build LLVM-C.dll on MSVC that exports only the C API.

LGTM.

Aug 9 2017, 10:26 AM

Aug 4 2017

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

@mgorny I want to apologize here but I need to ask you not to commit this until @chapuni and I can come to an agreement between this solution and D36211. He ping'd me on IRC yesterday after I left for the day, I'll try and get in touch with him today to see if we can agree on the path forward.

Aug 4 2017, 9:56 AM · Restricted Project

Aug 3 2017

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

@chapuni, I disagree. LLDBExpression doesn't actually use RuntimeDyld directly, rather ExecutionEngine's headers do. I don't want downstream users of ExecutionEngine to know about the things it neccesitates pulling in.

Aug 3 2017, 2:05 PM · Restricted Project
beanz accepted D36211: [cmake] Expose the dependencies of ExecutionEngine as PUBLIC.
Aug 3 2017, 1:56 PM
beanz added a comment to D36211: [cmake] Expose the dependencies of ExecutionEngine as PUBLIC.

@chapuni, the reason that I wanted to have this done with PUBLIC interface specifications is because lldbExpression does't actually use anything from the RuntimeDYLD library. The problem is LLVMExecutionEngine's headers are not clean, and they reference RuntimeDYLD directly and result in needing to bring it in.

Aug 3 2017, 1:56 PM
beanz accepted D30155: [clang-tools-extra] [test] Fix clang library dir in LD_LIBRARY_PATH For stand-alone build.

Looks reasonable to me.

Aug 3 2017, 11:17 AM · Restricted Project
beanz accepted D35311: lldb-server tests: Add support for testing debugserver.

This all looks fine to me. The one thing I'd like you to consider is that someday (when lldb-server is supported on Darwin) we will want to run these tests against both debugserver and lldb-server. I'm not asking you to make those changes to this patch, but if there are things you would change about how you're doing things to make that easier to support I'd ask that you consider that.

Aug 3 2017, 10:57 AM

Jul 26 2017

beanz accepted D35924: [CMake] Disable -Werror for CMake checks.

LGTM!

Jul 26 2017, 4:49 PM

Jul 18 2017

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

LGTM!

Jul 18 2017, 2:00 PM
beanz added inline comments to D33048: [CMake] runtimes test targets need to depend on LLVM tools.
Jul 18 2017, 1:46 PM
beanz added inline comments to D33048: [CMake] runtimes test targets need to depend on LLVM tools.
Jul 18 2017, 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.

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

LGTM!

Jul 18 2017, 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.

Jul 18 2017, 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?

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

LGTM

Jul 18 2017, 1:29 PM

Jul 10 2017

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

LGTM!

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

This LGTM.

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

LGTM

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

LGTM.

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

LGTM!

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

LGTM! Thanks for the cleanup!

Jul 10 2017, 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!

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

LGTM!

Jul 10 2017, 8:10 AM

Jul 9 2017

beanz accepted D34520: Minor correctness improvements for LLVMTestingSupport.

LGTM!

Jul 9 2017, 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.

Jul 9 2017, 6:13 AM

Jul 7 2017

beanz added inline comments to D33760: [libunwind][CMake] Add install path variable to allow overriding the destination.
Jul 7 2017, 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