[PATCH] D64582: cmake: Fix install of libclang_shared.so when LLVM_INSTALL_TOOLCHAIN_ONLY=ON

smeenai accepted this revision. smeenai added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64582/new/ https://reviews.llvm.org/D64582 _

[PATCH] D64580: cmake: Add INSTALL_WITH_TOOLCHAIN option to add_*_library macros

smeenai added a comment. Nice. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64580/new/ https://reviews.llvm.org/D64580 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D64579: [clang-shlib] Fix clang-shlib for PRIVATE dependencies

This revision was automatically updated to reflect the committed changes. Closed by commit rL365825: [clang-shlib] Fix clang-shlib for PRIVATE dependencies (authored by smeenai, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https

[PATCH] D64579: [clang-shlib] Fix clang-shlib for PRIVATE dependencies

smeenai added a comment. In D64579#1581557 , @beanz wrote: > Yea, this makes sense even if it is a bit sad. We do use the > `--whole-archive` approach for `libLLVM`, and it works. I was hoping to avoid > it here in part to free up the ability to link `li

[PATCH] D64089: [Driver] Introduce -stdlib++-isystem

smeenai added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64089/new/ https://reviews.llvm.org/D64089 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D61479: Finish "Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO"

smeenai added a comment. In D61479#1586987 , @sberg wrote: > eh, the summary here doesn't get updated from the actual git commit message :( > > thanks for the reviews! You can use `arc diff --verbatim` to update the summary based on your local commit me

[PATCH] D64089: [Driver] Introduce -stdlib++-isystem

smeenai marked an inline comment as done. smeenai added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1307 + HasStdlibxxIsystem ? TC.AddClangCXXStdlibIsystemArgs(Args, CmdArgs) + : TC.AddClangCXXStdlibIncludeArgs(Args, Cmd

[PATCH] D64089: [Driver] Introduce -stdlib++-isystem

smeenai added a reviewer: rnk. smeenai added a subscriber: rnk. smeenai added a comment. Adding @rnk as someone familiar with the driver/frontend :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64089/new/ https://reviews.llvm.org/D64089 ___

[PATCH] D64089: [Driver] Introduce -stdlib++-isystem

This revision was automatically updated to reflect the committed changes. Closed by commit rL367982: [Driver] Introduce -stdlib++-isystem (authored by smeenai, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.or

[PATCH] D65838: [Driver] Use enumeration for quoting mode. NFC

smeenai created this revision. smeenai added reviewers: compnerd, phosek, rnk. Herald added a project: clang. Herald added a subscriber: cfe-commits. Boolean parameters are generally hard to understand, especially when we don't consistently have a comment for them. Change to an enumeration. While

[PATCH] D65839: [Driver] Add verbatim dry run option

smeenai created this revision. smeenai added reviewers: compnerd, phosek, rnk. Herald added a project: clang. Herald added a subscriber: cfe-commits. smeenai added a parent revision: D65838: [Driver] Use enumeration for quoting mode. NFC. When writing driver tests, it's useful to have a way to ou

[PATCH] D64538: [Driver] Don't escape backslashes on Windows

smeenai abandoned this revision. smeenai added a comment. Abandoning in favor of D65839 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64538/new/ https://reviews.llvm.org/D64538 __

[PATCH] D65839: [Driver] Add verbatim dry run option

smeenai added a comment. I'm not tied to the name `-###-verbatim` and am open to suggestions if anyone can think of something better. My troll suggestion was `-`, but @compnerd didn't like that for some reason... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://r

[PATCH] D65839: [Driver] Add verbatim dry run option

smeenai updated this revision to Diff 213773. smeenai edited the summary of this revision. smeenai added a comment. Output to stdout Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65839/new/ https://reviews.llvm.org/D65839 Files: clang/include/cl

[PATCH] D65841: [Driver] Switch -stdlib++-isystem test to -###-verbatim

smeenai created this revision. smeenai added reviewers: compnerd, phosek, rnk. Herald added a project: clang. Herald added a subscriber: cfe-commits. This allows the test to pass on Windows and avoid backslash quoting issues. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D65841

[PATCH] D64089: [Driver] Introduce -stdlib++-isystem

smeenai created this revision. smeenai added reviewers: compnerd, phosek, rsmith, zer0. Herald added a subscriber: srhines. Herald added a project: clang. There are times when we wish to explicitly control the C++ standard library search paths used by the driver. For example, when we're building a

[PATCH] D64089: [Driver] Introduce -stdlib++-isystem

smeenai added a comment. In D64089#1567028 , @danalbert wrote: > > For example, when we're building against the Android NDK, we might want to > > use the NDK's C++ headers (which have a custom inline namespace) even if we > > have C++ headers installed n

[PATCH] D64383: build: use multiple `install` rather than building up a list

smeenai accepted this revision. smeenai added a comment. Nice! Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64383/new/ https://reviews.llvm.org/D64383 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https

[PATCH] D64383: build: use multiple `install` rather than building up a list

smeenai added inline comments. Comment at: src/CMakeLists.txt:440 + endif() + if (LIBCXX_INSTALL_STATIC_LIBRARY) +install(TARGETS cxx_static Super nit: you don't have a newline here but do have a newline before the LIBCXX_INSTALL_EXPERIMENTAL_LIBRARY condi

[PATCH] D64089: [Driver] Introduce -stdlib++-isystem

smeenai added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64089/new/ https://reviews.llvm.org/D64089 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D64089: [Driver] Introduce -stdlib++-isystem

smeenai added a comment. In D64089#1576902 , @ldionne wrote: > Background question: I'm familiar with the behavior on Darwin, where we > prefer C++ headers in the toolchain (alongside the driver), however do other > platforms behave the same? I thought t

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

smeenai added subscribers: beanz, smeenai. smeenai added a comment. In D58418#1577349 , @jkorous wrote: > Thanks for the revert. > > There's actually one more problem - seems like ninja doesn't like the > generated build.ninja file on Linux. > > ninja:

[PATCH] D64538: [Driver] Don't escape backslashes on Windows

smeenai created this revision. smeenai added reviewers: compnerd, hans, mstorsjo, rnk, thakis, zturner. Herald added a project: clang. Herald added a subscriber: cfe-commits. Escaping backslashes results in unnatural looking output, and the actual escapes are mostly unnecessary. We were also not c

[PATCH] D64538: [Driver] Don't escape backslashes on Windows

smeenai added a comment. Hmm, this only fixes `-###`. Looking into `-v` now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64538/new/ https://reviews.llvm.org/D64538 ___ cfe-commits mailing list cfe-co

[PATCH] D64538: [Driver] Don't escape backslashes on Windows

smeenai added a comment. In D64538#1579510 , @smeenai wrote: > Hmm, this only fixes `-###`. Looking into `-v` now. Ignore this – I was testing with a version without my change :D Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:

[PATCH] D64538: [Driver] Don't escape backslashes on Windows

smeenai added a subscriber: phosek. smeenai added a comment. CC @phosek, since I believe you had issues writing tests referencing the installation directory in the past. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64538/new/ https://reviews.llvm

[PATCH] D64538: [Driver] Don't escape backslashes on Windows

smeenai added a comment. In D64538#1579561 , @rnk wrote: > An extremely convenient feature of the current escaping pattern is that it > magically works for both the cmd shell and various bash implementations. You > can simply copy paste the commands and

[PATCH] D64538: [Driver] Don't escape backslashes on Windows

smeenai added a comment. In D64538#1579610 , @rnk wrote: > In D64538#1579573 , @smeenai wrote: > > > In D64538#1579561 , @rnk wrote: > > > > > An extremely convenient feature

[PATCH] D61974: [ObjC] Fix encoding of ObjC pointer types that are pointers to typedefs

smeenai added a comment. In D61974#1508097 , @ahatanak wrote: > Delete the entire code path that tries to work around a bug in gcc. Should something be added to the release notes for this? Repository: rC Clang CHANGES SINCE LAST ACTION https://rev

[PATCH] D55543: [CodeGen] Fix assertion on throwing object with inlined inherited constructor and non-trivial destructor.

smeenai added a comment. I'd tried this exact same patch back in https://reviews.llvm.org/D44619, but I was running into a bunch of check-clang failures with it, and I was never able to figure them out. It looks like it works now though, so I'm glad :) Richard's comment from that diff might sti

[PATCH] D44619: [CodeGen] Add cleanup scope to EmitInlinedInheritingCXXConstructorCall

smeenai abandoned this revision. smeenai added a comment. https://reviews.llvm.org/D55543 does the same thing Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D44619/new/ https://reviews.llvm.org/D44619 ___ cfe-commits m

[PATCH] D56466: [CodeGen] Clarify comment about COFF common symbol alignment

smeenai created this revision. smeenai added reviewers: erichkeane, majnemer, mstorsjo. After a discussion on the commit thread, it seems the 32 byte alignment limitation is an MSVC toolchain artifact, not an inherent COFF restriction. Clarify the comment accordingly, since saying COFF in the comm

[PATCH] D56466: [CodeGen] Clarify comment about COFF common symbol alignment

smeenai updated this revision to Diff 180771. smeenai added a comment. Refer to link.exe, and then also refer to other linkers in the second half of the comment to match Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56466/new/ https://reviews.llvm.org/D56466 Fi

[PATCH] D56466: [CodeGen] Clarify comment about COFF common symbol alignment

This revision was automatically updated to reflect the committed changes. Closed by commit rC350754: [CodeGen] Clarify comment about COFF common symbol alignment (authored by smeenai, committed by ). Changed prior to commit: https://reviews.llvm.org/D56466?vs=180771&id=180897#toc Repository:

[PATCH] D45529: [CMake] Set the default ABI version for Fuchsia in CMake as well

smeenai added a comment. This LGTM given that Eric accepted your previous change. I'm happy to accept it in a day or two if he hasn't gotten the chance to take a look by then. Comment at: libcxx/CMakeLists.txt:101 +if (FUCHSIA) + set(ABI_VERSION_DEFAULT 2) +else() ---

[PATCH] D45529: [CMake] Set the default ABI version for Fuchsia in CMake as well

smeenai accepted this revision. smeenai added a comment. This revision is now accepted and ready to land. Eric accepted on IRC. Repository: rCXX libc++ https://reviews.llvm.org/D45529 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http:/

[PATCH] D45779: [ARM] Remove redundant #if in test

smeenai accepted this revision. smeenai added a comment. This revision is now accepted and ready to land. LGTM; it's an obvious NFC patch. Repository: rC Clang https://reviews.llvm.org/D45779 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D45829: Remove impossible _MSC_VER check

smeenai accepted this revision. smeenai added a comment. This revision is now accepted and ready to land. LGTM. I'd tried to clean all stale `_MSC_VER` conditionals in the past, but I must have missed some. https://reviews.llvm.org/D45829 ___ cfe-c

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

smeenai added a comment. In https://reviews.llvm.org/D45639#1078494, @thakis wrote: > > because the headers that are part of Clang toolchain are incompatible with > > the system library > > Do you have details on this? This isn't supposed to be the case as far as I > know. We link chrome/mac ag

[PATCH] D46287: [Driver] Don't add -dwarf-column-info when using -gcodeview on non-msvc targets

smeenai accepted this revision. smeenai added a comment. This revision is now accepted and ready to land. Makes sense to me. You may wanna wait for @zturner, but I can't really imagine any benefit to restricting this to MSVC. Repository: rC Clang https://reviews.llvm.org/D46287 __

[PATCH] D45779: [ARM] Remove redundant #if in test

This revision was automatically updated to reflect the committed changes. Closed by commit rC331305: [ARM] Remove redundant #if in test. NFC (authored by smeenai, committed by ). Herald added a reviewer: javed.absar. Changed prior to commit: https://reviews.llvm.org/D45779?vs=142969&id=144775#t

[PATCH] D45713: [libclang] Fix the type of 'int (Foo);'

This revision was automatically updated to reflect the committed changes. Closed by commit rL331306: [libclang] Fix the type of 'int (Foo);' (authored by smeenai, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D45713 Files: cfe/trunk/te

[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers

smeenai added a comment. Note that the alignment matters in addition to the size. The pattern I've seen internally is people using `%zd` for NSInteger and `%tu` for NSUInteger, since until clang 6 neither of those were format-checked at all. I'd be fine with adding an option to relax the printf

[PATCH] D48459: Respect CMAKE_SYSROOT and CMAKE_CROSSCOMPILING when searching for libxml2.

smeenai added reviewers: beanz, phosek. smeenai added a comment. I think it might be more appropriate to pass NO_SYSTEM_ENVIRONMENT_PATH to find_package in case CMAKE_SYSROOT or CMAKE_CROSSCOMPILING are set? That way, we won't search the host system for libxml2 in those cases, but we'll still be

[PATCH] D48459: Respect CMAKE_SYSROOT and CMAKE_CROSSCOMPILING when searching for libxml2.

smeenai added a comment. Actually, I would imagine that if you're cross-compiling or using a custom sysroot, you should probably also specify CMAKE_FIND_ROOT_PATH and set CMAKE_FIND_ROOT_PATH_MODE_PACKAGE to ONLY, to limit all these searches to the desired directories? (I'm actually playing ar

[PATCH] D48459: Respect CMAKE_SYSROOT and CMAKE_CROSSCOMPILING when searching for libxml2.

smeenai added a comment. Ah, the documentation is confusing, but CMAKE_FIND_ROOT_PATH and CMAKE_FIND_ROOT_PATH_MODE_PACKAGE only have an effect when using the config mode of find_package, whereas this invocation is using the module mode. That's a bummer, and definitely makes custom sysroots and

[PATCH] D48459: Respect CMAKE_SYSROOT and CMAKE_CROSSCOMPILING when searching for libxml2.

smeenai added a comment. Okay, upon playing with this further, the following seems to do what I want (for a custom sysroot), and seems to be a pretty common pattern as well: set(CMAKE_FIND_ROOT_PATH_MODE_INCLUDE ONLY) set(CMAKE_FIND_ROOT_PATH_MODE_LIBRARY ONLY) set(CMAKE_FIND_ROOT_PATH_MOD

[PATCH] D48626: New option -fwindows-filesystem, affecting #include paths.

smeenai added subscribers: smeenai, compnerd, zturner. smeenai added a comment. Adding some people who are interested in Windows cross-compilation. Repository: rC Clang https://reviews.llvm.org/D48626 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D48675: [libc++abi] Limit libc++ header search to specified paths

smeenai edited subscribers, added: cfe-commits; removed: llvm-commits. smeenai added a comment. Huh, this went to llvm-commits instead of cfe-commits automatically. I guess the auto mailing list subscribing doesn't work great with the monorepo? Repository: rL LLVM https://reviews.llvm.org/D4

[PATCH] D48694: [libc++abi] Limit libc++ header search to specified paths

smeenai created this revision. smeenai added reviewers: beanz, compnerd, dexonsmith, EricWF, mclow.lists. Herald added subscribers: cfe-commits, ldionne, christof, mgorny. Right now, when libc++abi is locating libc++ headers, it specifies several search locations, but it also doesn't prevent CMake

[PATCH] D48675: [libc++abi] Limit libc++ header search to specified paths

smeenai abandoned this revision. smeenai added a comment. https://reviews.llvm.org/D48694 Repository: rL LLVM https://reviews.llvm.org/D48675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/c

[PATCH] D48694: [libc++abi] Limit libc++ header search to specified paths

smeenai added a comment. This doesn't map to any command line change; it's purely a CMake thing. It just changes where libc++abi's build system looks for libc++ headers. Before this patch, it would look for a file named "vector" in all the standard system include directories (`/usr/local/includ

[PATCH] D48694: [libc++abi] Limit libc++ header search to specified paths

smeenai added a comment. In https://reviews.llvm.org/D48694#1146232, @EricWF wrote: > LGTM, but I'm a bit confused. You seem to argue that no system places C++ > headers on the default search paths, but also that it would be a problem if > such a system did. > Why wouldn't the conclusion be tr

[PATCH] D48694: [libc++abi] Limit libc++ header search to specified paths

smeenai added a comment. In https://reviews.llvm.org/D48694#1148718, @EricWF wrote: > > For whether it makes sense to search for includes in the system path, that > > boils down to @ldionne's question of whether building libc++abi against a > > system-installed libc++ is supported. I actually d

[PATCH] D48694: [libc++abi] Limit libc++ header search to specified paths

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 rL336032: [libc++abi] Limit libc++ header search to specified paths (authored by smeenai, committed by ). Herald added a sub

[PATCH] D49054: [MinGW] Treat any -lucrt* as replacing -lmsvcrt

smeenai accepted this revision. smeenai added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang https://reviews.llvm.org/D49054 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-

[PATCH] D49059: [MinGW] Skip adding default win32 api libraries if -lwindowsapp is specified

smeenai accepted this revision. smeenai added a comment. This revision is now accepted and ready to land. LGTM, particularly given r314138. There are other umbrella libraries as well, e.g. OneCore and OneCoreUAP. Do you care about those as well or just WindowsApp? Comment at:

[PATCH] D49059: [MinGW] Skip adding default win32 api libraries if -lwindowsapp is specified

smeenai added inline comments. Comment at: test/Driver/mingw-windowsapp.c:5-6 +// CHECK_DEFAULT: "-lmsvcrt" "-ladvapi32" "-lshell32" "-luser32" "-lkernel32" "-lmingw32" +// CHECK_WINDOWSAPP: "-lwindowsapp" "-lmingw32" +// CHECK_WINDOWSAPP-SAME: "-lmsvcrt" "-lmingw32" ---

[PATCH] D49059: [MinGW] Skip adding default win32 api libraries if -lwindowsapp is specified

smeenai added inline comments. Comment at: test/Driver/mingw-windowsapp.c:5-6 +// CHECK_DEFAULT: "-lmsvcrt" "-ladvapi32" "-lshell32" "-luser32" "-lkernel32" "-lmingw32" +// CHECK_WINDOWSAPP: "-lwindowsapp" "-lmingw32" +// CHECK_WINDOWSAPP-SAME: "-lmsvcrt" "-lmingw32" ---

[PATCH] D47233: [CodeGen] Emit MSVC RTTI for Obj-C EH types

smeenai added inline comments. Comment at: lib/CodeGen/CGObjCMac.cpp:7457-7460 CGObjCNonFragileABIMac::GetEHType(QualType T) { // There's a particular fixed type info for 'id'. if (T->isObjCIdType() || T->isObjCQualifiedIdType()) { +if (CGM.getTriple().isWindowsMSVCEnv

[PATCH] D49227: Override a bit fields layout from an external source

smeenai resigned from this revision. smeenai added a comment. I'm not familiar enough with this to review it. Repository: rC Clang https://reviews.llvm.org/D49227 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-b

[PATCH] D49345: [CMake] Use correct variable as header install prefix

smeenai accepted this revision. smeenai added a comment. This revision is now accepted and ready to land. Seems like an obvious enough fix, so I'm jumping in to LGTM. Repository: rCXX libc++ https://reviews.llvm.org/D49345 ___ cfe-commits mailing

[PATCH] D47233: [CodeGen] Emit MSVC RTTI for Obj-C EH types

smeenai added inline comments. Comment at: lib/CodeGen/CGObjCMac.cpp:7457-7460 CGObjCNonFragileABIMac::GetEHType(QualType T) { // There's a particular fixed type info for 'id'. if (T->isObjCIdType() || T->isObjCQualifiedIdType()) { +if (CGM.getTriple().isWindowsMSVCEnv

[PATCH] D68412: [clang] [cmake] Support LLVM_DISTRIBUTION_COMPONENTS in stand-alone build

smeenai accepted this revision. smeenai added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68412/new/ https://reviews.llvm.org/D68412 ___ cfe-commits mailing list cfe-commits

[PATCH] D69194: build: add clang-cl and clang++ symlinks in the build tree

smeenai added a comment. > Furthermore, the current build only installs the symbolic links in the > install tree, not the build tree This isn't right ... if I look at my LLVM build tree, I have clang, clang++ and clang-cl symlinks, all pointing to the actual clang-10 binary. Where are you seei

[PATCH] D80947: Add to the Coding Standard our that single-line bodies omit braces

smeenai added inline comments. Comment at: llvm/docs/CodingStandards.rst:1600 + // Omit the braces, since the body is simple and clearly associated with the if. + if (isa(D)) +handleFunctionDecl(D); I'm late to the party here (and I haven't read the entire

[PATCH] D82694: [clang-shlib] Don't link with static clang libraries

smeenai accepted this revision. smeenai added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82694/new/ https://reviews.llvm.org/D82694 _

[PATCH] D67321: Respect CLANG_LINK_CLANG_DYLIB=ON in libclang and c-index-test

smeenai added a comment. Super late to the party here, sorry. I don't understand why the type of linking needs to be changed depending on if the library is static or shared. If a static library A depends on a static library B as PRIVATE, and then executable C links to A, CMake will make sure t

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

smeenai accepted this revision. smeenai added a comment. This revision is now accepted and ready to land. Oops, missed that I'd become a blocking reviewer for this (cos of requesting changes before). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79219/new/ https://reviews.llvm.org/D792

[PATCH] D78534: [libclang] Install both libclang.a and libclang.so when LIBCLANG_BUILD_STATIC=ON

smeenai added reviewers: beanz, phosek. smeenai added subscribers: phosek, beanz. smeenai added a comment. This makes sense to me, but I'd like @beanz and/or @phosek to take a look as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78534/new/

[PATCH] D78648: [CMake] Bump CMake minimum version to 3.13.4

smeenai accepted this revision. smeenai added a comment. This revision is now accepted and ready to land. With the newer minimum CMake version, we can eliminate a bunch of the policy settings as well, but that can be a follow-up (especially given this isn't gonna ship for a few months). (Anothe

[PATCH] D78534: [libclang] Install both libclang.a and libclang.so when LIBCLANG_BUILD_STATIC=ON

smeenai added a comment. @zhuhan0, do you need someone to commit this for you? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78534/new/ https://reviews.llvm.org/D78534 ___ cfe-commits mailing list cfe-

[PATCH] D78534: [libclang] Install both libclang.a and libclang.so when LIBCLANG_BUILD_STATIC=ON

smeenai added a comment. Committed. Please forward any buildbot failure emails to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78534/new/ https://reviews.llvm.org/D78534 ___ cfe-commits mailing li

[PATCH] D78534: [libclang] Install both libclang.a and libclang.so when LIBCLANG_BUILD_STATIC=ON

This revision was automatically updated to reflect the committed changes. Closed by commit rGf8990feb125a: [libclang] Install both libclang.a and libclang.so when LIBCLANG_BUILD_STATIC=ON (authored by zhuhan0, committed by smeenai). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

[PATCH] D78762: build: use `find_package(Python3)` if available

smeenai added a comment. In D78762#2006406 , @JDevlieghere wrote: > I talked to Saleem and he cleared up some of my concerns. Given that the > community seems to have agreed to support only Python 3, this change seems a > lot more reasonable. My earlier

[PATCH] D77621: ADT: SmallVector size/capacity use word-size integers when elements are small

smeenai added inline comments. Comment at: llvm/include/llvm/ADT/SmallVector.h:52 + // The maximum size depends on size_type used. + static constexpr size_t SizeMax() { +return std::numeric_limits::max(); browneee wrote: > dexonsmith wrote: > > STL data str

[PATCH] D85362: [CMake] Print the autodetected host linker version

smeenai accepted this revision. smeenai added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85362/new/ https://reviews.llvm.org/D85362 ___ cfe-commits mailing list cfe-commits@lists.llvm.

[PATCH] D79059: Fix and re-submit D78534 - [libclang] Install both libclang.a and libclang.so when LIBCLANG_BUILD_STATIC=ON

smeenai accepted this revision. smeenai added a comment. This revision is now accepted and ready to land. Sorry for the delayed review here. I'm not the biggest fan of this cos it's encoding knowledge of `llvm_add_library`'s internal behavior (that it creates a `*_static` target if both STATIC

[PATCH] D79059: Fix and re-submit D78534 - [libclang] Install both libclang.a and libclang.so when LIBCLANG_BUILD_STATIC=ON

smeenai added a comment. Do you need me to commit this for you? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79059/new/ https://reviews.llvm.org/D79059 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D79059: Fix and re-submit D78534 - [libclang] Install both libclang.a and libclang.so when LIBCLANG_BUILD_STATIC=ON

smeenai added a comment. Committed. Please let me know of any buildbot failure emails you receive. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79059/new/ https://reviews.llvm.org/D79059 ___ cfe-commi

[PATCH] D79059: Fix and re-submit D78534 - [libclang] Install both libclang.a and libclang.so when LIBCLANG_BUILD_STATIC=ON

This revision was automatically updated to reflect the committed changes. Closed by commit rG0c9230dad169: Reland [libclang] Install both libclang.a and libclang.so when… (authored by zhuhan0, committed by smeenai). Changed prior to commit: https://reviews.llvm.org/D79059?vs=262506&id=262530#to

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

smeenai added inline comments. Comment at: lld/test/lit.site.cfg.py.in:16 config.target_triple = "@TARGET_TRIPLE@" -config.python_executable = "@Python3_EXECUTABLE@" -config.have_zlib = @HAVE_LIBZ@ +config.python_executable = "@PYTHON_EXECUTABLE@" +config.have_zlib = @LLVM_ENABL

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

smeenai requested changes to this revision. smeenai added a comment. This revision now requires changes to proceed. Requesting changes to remove from my review queue while the above comments are being addressed. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79219/new/ https://reviews.l

[PATCH] D40660: Enable auto-linking on Windows

smeenai added inline comments. Comment at: include/__config:1266 +#if defined(_LIBCPP_ABI_MICROSOFT) +# if defined(_DLL) && !defined(_LIBCPP_BUILDING_LIBRARY) +# if defined(_LIBCPP_DEBUG) This feels more like a Windows (or perhaps COFF) thing than a Microsoft A

[PATCH] D40660: Enable auto-linking on Windows

smeenai added inline comments. Comment at: include/__config:1266 +#if defined(_LIBCPP_ABI_MICROSOFT) +# if defined(_DLL) && !defined(_LIBCPP_BUILDING_LIBRARY) +# if defined(_LIBCPP_DEBUG) rnk wrote: > smeenai wrote: > > This feels more like a Windows (or perhap

[PATCH] D40660: Enable auto-linking on Windows

smeenai added a comment. I'm fine with future proofing, but not if it doesn't actually work in the present :) Comment at: include/__config:1267 +# if defined(_DLL) && !defined(_LIBCPP_BUILDING_LIBRARY) +# if defined(_LIBCPP_DEBUG) +# pragma comment(lib, "c++d.lib") -

[PATCH] D40675: [clang] Use add_llvm_install_targets

smeenai created this revision. Herald added a subscriber: mgorny. Use this function to create the install targets rather than doing so manually, which gains us the `-stripped` install targets to perform stripped installations. Repository: rC Clang https://reviews.llvm.org/D40675 Files: cma

[PATCH] D40675: [clang] Use add_llvm_install_targets

This revision was automatically updated to reflect the committed changes. Closed by commit rC319489: [clang] Use add_llvm_install_targets (authored by smeenai). Changed prior to commit: https://reviews.llvm.org/D40675?vs=125013&id=125026#toc Repository: rC Clang https://reviews.llvm.org/D40

[PATCH] D40680: [libc++] Create install-stripped targets

smeenai created this revision. Herald added a subscriber: mgorny. LLVM is gaining install-*-stripped targets to perform stripped installs, and in order for this to be useful for install-distribution, all potential distribution components should have stripped installation targets. LLVM has a functi

[PATCH] D40680: [libc++] Create install-stripped targets

smeenai updated this revision to Diff 125034. smeenai added a comment. Don't add deprecated target names, since clients should be switching over to the new ones. https://reviews.llvm.org/D40680 Files: include/CMakeLists.txt lib/CMakeLists.txt Index: lib/CMakeLists.txt ===

[PATCH] D40681: [libc++abi] Add install-cxxabi-stripped target

smeenai created this revision. Herald added a subscriber: mgorny. LLVM is gaining install-*-stripped targets to perform stripped installs, and in order for this to be useful for install-distribution, all potential distribution components should have stripped installation targets. LLVM has a functi

[PATCH] D40685: [libunwind] Switch to add_llvm_install_targets

smeenai created this revision. Herald added a subscriber: mgorny. This gains us the install-unwind-stripped target, to perform stripping during installation. https://reviews.llvm.org/D40685 Files: src/CMakeLists.txt Index: src/CMakeLists.txt =

[PATCH] D40687: [compiler-rt] Switch to add_llvm_install_targets

smeenai created this revision. Herald added subscribers: mgorny, dberris. This gains us the install-*-stripped targets, to strip binaries during installation. These targets otherwise mimic the existing install targets. https://reviews.llvm.org/D40687 Files: cmake/Modules/AddCompilerRT.cmake

[PATCH] D40685: [libunwind] Switch to add_llvm_install_targets

This revision was automatically updated to reflect the committed changes. Closed by commit rL319498: [libunwind] Switch to add_llvm_install_targets (authored by smeenai). Repository: rL LLVM https://reviews.llvm.org/D40685 Files: libunwind/trunk/src/CMakeLists.txt Index: libunwind/trunk/s

[PATCH] D40681: [libc++abi] Add install-cxxabi-stripped target

This revision was automatically updated to reflect the committed changes. Closed by commit rL319499: [libc++abi] Add install-cxxabi-stripped target (authored by smeenai). Repository: rL LLVM https://reviews.llvm.org/D40681 Files: libcxxabi/trunk/src/CMakeLists.txt Index: libcxxabi/trunk/s

[PATCH] D40660: Enable auto-linking on Windows

smeenai accepted this revision. smeenai added a comment. This revision is now accepted and ready to land. I think it would make sense for this change to also have the conditional for the static C++ library selection. Basically something like #if defined(_LIBCPP_ABI_MICROSOFT) && !defined(_LIBC

[PATCH] D40687: [compiler-rt] Switch to add_llvm_install_targets

smeenai added a comment. Ah, that's a bummer. compiler-rt's CMakeLists has this big shiny comment up top: # This build assumes that CompilerRT is checked out into the # 'projects/compiler-rt' or 'runtimes/compiler-rt' inside of an LLVM tree. # Standalone build system for CompilerRT is not

[PATCH] D40687: [compiler-rt] Add install-*-stripped targets

smeenai updated this revision to Diff 125180. smeenai retitled this revision from "[compiler-rt] Switch to add_llvm_install_targets" to "[compiler-rt] Add install-*-stripped targets". smeenai edited the summary of this revision. smeenai added a comment. Add targets manually https://reviews.llvm

[PATCH] D40740: [compiler-rt] Remove out of date comment

smeenai created this revision. Herald added subscribers: mgorny, dberris. Per beanz, building compiler-rt standalone is a pretty important use case, so the comment is very out of date. https://reviews.llvm.org/D40740 Files: CMakeLists.txt Index: CMakeLists.txt ==

[PATCH] D40687: [compiler-rt] Add install-*-stripped targets

This revision was automatically updated to reflect the committed changes. Closed by commit rL319569: [compiler-rt] Add install-*-stripped targets (authored by smeenai). Repository: rL LLVM https://reviews.llvm.org/D40687 Files: compiler-rt/trunk/cmake/Modules/AddCompilerRT.cmake compiler-

<    1   2   3   4   5   6   7   8   >