[PATCH] D39509: Vary Windows toolchain selection by -fused-ld

smeenai added inline comments. Comment at: lib/Driver/Driver.cpp:3884 +const auto A = Args.getLastArgValue(options::OPT_fuse_ld_EQ, "link"); +if (A.equals_lower("link") || A.equals_lower("lld")) + TC = `lld-link` is a valid invocation for

[PATCH] D39509: Vary Windows toolchain selection by -fused-ld

smeenai added a comment. Also title says `-fused-ld` instead of `-fuse-ld` https://reviews.llvm.org/D39509 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39522: [clang-rename] Use add_clang_tool

smeenai created this revision. Herald added a subscriber: mgorny. `add_clang_tool` includes a call to `add_clang_executable`, but it also sets up the install rule, and adds an `install-*` target. The latter is required for using `LLVM_DISTRIBUTION_COMPONENTS`. https://reviews.llvm.org/D39522 Fi

[PATCH] D39523: [clang-reorder-fields] Switch to add_clang_tool

smeenai created this revision. Herald added a subscriber: mgorny. `add_clang_tool` invokes `add_clang_executable` internally, but it also takes care of setting up the install rule. It also adds an `install-*` build target, which is required for `LLVM_DISTRIBUTION_COMPONENTS`. https://reviews.llv

[PATCH] D39524: [libclang] Add dummy libclang-headers target

smeenai created this revision. Herald added a subscriber: mgorny. `LLVM_DISTRIBUTION_COMPONENTS` assumes that each component has both `component` and `install-component` targets. Add a dummy no-op target for `libclang-headers` to placate this check. https://reviews.llvm.org/D39524 Files: tool

[PATCH] D39523: [clang-reorder-fields] Switch to add_clang_tool

This revision was automatically updated to reflect the committed changes. Closed by commit rL317149: [clang-reorder-fields] Switch to add_clang_tool (authored by smeenai). Repository: rL LLVM https://reviews.llvm.org/D39523 Files: clang-tools-extra/trunk/clang-reorder-fields/tool/CMakeLists

[PATCH] D39522: [clang-rename] Use add_clang_tool

This revision was automatically updated to reflect the committed changes. Closed by commit rL317150: [clang-rename] Use add_clang_tool (authored by smeenai). Repository: rL LLVM https://reviews.llvm.org/D39522 Files: cfe/trunk/tools/clang-rename/CMakeLists.txt Index: cfe/trunk/tools/clang

[PATCH] D39524: [libclang] Add dummy libclang-headers target

smeenai updated this revision to Diff 121223. smeenai added a comment. Better comment https://reviews.llvm.org/D39524 Files: tools/libclang/CMakeLists.txt Index: tools/libclang/CMakeLists.txt === --- tools/libclang/CMakeLists.t

[PATCH] D39524: [libclang] Add dummy libclang-headers target

This revision was automatically updated to reflect the committed changes. Closed by commit rL317188: [libclang] Add dummy libclang-headers target (authored by smeenai). Repository: rL LLVM https://reviews.llvm.org/D39524 Files: cfe/trunk/tools/libclang/CMakeLists.txt Index: cfe/trunk/tool

[PATCH] D39462: [Sema] Implement -Wmaybe-tautological-constant-compare for when the tautologicalness is data model dependent

smeenai added a comment. In https://reviews.llvm.org/D39462#922847, @rjmccall wrote: > In https://reviews.llvm.org/D39462#922844, @lebedev.ri wrote: > > > In https://reviews.llvm.org/D39462#922826, @rjmccall wrote: > > > > > I don't speak for the entire project, but I'm not sure I'm interested in

[PATCH] D39821: Add CLANG_DEFAULT_OBJCOPY to allow Clang to use llvm-objcopy for dwarf fission

smeenai added a comment. It wasn't automatically closed because the commit (https://reviews.llvm.org/rL317960) referenced the wrong differential in its Differential Revision field. Not sure how that happened. Repository: rL LLVM https://reviews.llvm.org/D39821 ___

[PATCH] D140925: [CMake] Use Clang to infer the target triple

smeenai added a comment. Ping @ldionne, would you be able to take a look at this soon (or are you okay waiving the libc++ blocking requirement for it)? This is really useful for Android armv7, where the triple is traditionally spelled armv7-none-linux-androideabi but normalized to arvm7-none-li

[PATCH] D144603: Add option to disable compiler launcher on external projects

smeenai added a comment. Hmm, what cache key does ccache use for compilers? Is it the `--version` output string, the path, or some combination? If the path is involved then I don't see any value in using ccache for configuring anything past stage1, since the compiler will (presumably) be instal

[PATCH] D144603: Add option to disable compiler launcher on external projects

smeenai added a comment. In D144603#4162192 , @haowei wrote: > In D144603#4162048 , @smeenai wrote: > >> Hmm, what cache key does ccache use for compilers? Is it the `--version` >> output string, the path, or som

[PATCH] D145228: [clangd] Add clangd headers to install targets

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/D145228/new/ https://reviews.llvm.org/D145228 _

[PATCH] D153175: [Frontend] Don't output skipped includes from predefines

smeenai added a comment. In D153175#4431923 , @hans wrote: >> -H is supposed to skip outputting headers from -include command line >> arguments, but -fshow-skipped-includes was outputting any skipped >> includes encountered via -include. > > I was thrown

[PATCH] D153175: [Frontend] Don't output skipped includes from predefines

This revision was automatically updated to reflect the committed changes. Closed by commit rG67a11290df64: [Frontend] Don't output skipped includes from predefines (authored by smeenai). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153175/new/ http

[PATCH] D153176: [Frontend] Remove ShowIncludesPretendHeader

This revision was automatically updated to reflect the committed changes. Closed by commit rG1df10f15807f: [Frontend] Remove ShowIncludesPretendHeader (authored by smeenai). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153176/new/ https://reviews.l

[PATCH] D147764: Fix the two gmoules-prefered-name-* tests

smeenai added reviewers: Michael137, dblaikie, aprantl. smeenai added a comment. Thanks! I'm not super familiar with this, so I'm adding the original author and reviewers to confirm. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147764/new/ https:

[PATCH] D151393: [CodeGen] Make __clang_call_terminate have an unwind table entry

smeenai created this revision. smeenai added reviewers: efriedma, jyknight, rnk, MaskRay. Herald added a subscriber: kristof.beyls. Herald added a project: All. smeenai requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This enables debuggers

[PATCH] D151393: [CodeGen] Make __clang_call_terminate have an unwind table entry

smeenai updated this revision to Diff 525429. smeenai added a comment. Call SetLLVMFunctionAttributesForDefinition instead Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151393/new/ https://reviews.llvm.org/D151393 Files: clang/lib/CodeGen/Itaniu

[PATCH] D151393: [CodeGen] Make __clang_call_terminate have an unwind table entry

This revision was automatically updated to reflect the committed changes. Closed by commit rG8f7b51e4ec09: [CodeGen] Make __clang_call_terminate have an unwind table entry (authored by smeenai). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151393/ne

[PATCH] D151393: [CodeGen] Make __clang_call_terminate have an unwind table entry

smeenai added a comment. I searched for `__clang_call_terminate` in the Clang test directory when I was working on this diff. Most tests were testing calls to it, not definitions. There were a couple of tests checking the definition, but they seemed pretty targeted (e.g. https://github.com/llv

[PATCH] D151595: [BOLT][CMake] Redo the build and install targets

smeenai added a comment. Makes sense, though @Amir should also take a look. Comment at: bolt/CMakeLists.txt:122 + +add_custom_target(bolt DEPENDS ${BOLT_DEPENDS}) +set_target_properties(bolt PROPERTIES FOLDER "BOLT") Where is BOLT_DEPENDS being set now? The [C

[PATCH] D149504: [clang][CodeGenPGO] Don't use an invalid index when region counts disagree

smeenai accepted this revision. smeenai added a comment. This revision is now accepted and ready to land. Seems pretty reasonable to add a bounds check here. You should probably add a comment explaining the reasoning though. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

[PATCH] D148654: Modify BoundsSan to improve debuggability

smeenai added a reviewer: melver. smeenai added a comment. The patch should be uploaded with full context to make review easier. Adding another potential reviewer. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148654/new/ https://reviews.llvm.org/

[PATCH] D148654: Modify BoundsSan to improve debuggability

smeenai added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp:189 auto GetTrapBB = [&TrapBB](BuilderTy &IRB) { -if (TrapBB && SingleTrapBB) - return TrapBB; - -Function *Fn = IRB.GetInsertBlock()->getParent(); -// FIXME: Thi

[PATCH] D148654: Modify BoundsSan to improve debuggability

smeenai added a comment. You should upload this with full context and add some test cases :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148654/new/ https://reviews.llvm.org/D148654 ___ cfe-commits mai

[PATCH] D148654: Modify BoundsSan to improve debuggability

smeenai added a comment. Thinking about this a bit more, should the trap not have an associated stack trace that can be symbolicated to tell you which line of code was crashing? If the issue is that multiple traps can get folded together, the `nomerge` attribute (D78659

[PATCH] D153175: [Frontend] Don't output skipped includes from predefines

smeenai created this revision. smeenai added reviewers: hans, rnk, thakis. Herald added a project: All. smeenai requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. `-H` is supposed to skip outputting headers from `-include` command line argumen

[PATCH] D153176: [Frontend] Remove ShowIncludesPretendHeader

smeenai created this revision. smeenai added reviewers: hans, rnk, thakis. Herald added a project: All. smeenai requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. It hasn't been written to since https://reviews.llvm.org/D46652, so it was alway

[PATCH] D27153: [libc++] Make __num_get_float hidden

smeenai added a comment. Ping. https://reviews.llvm.org/D27153 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26949: [libc++abi] Clean up visibility

smeenai added a comment. Ping. https://reviews.llvm.org/D26949 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D25208: [libc++] Make _LIBCPP_TYPE_VIS export members

smeenai requested a review of this revision. smeenai added a comment. Actually, I think this can stand on its own. I'll do the extern template fixes in another diff. Also, ping :) https://reviews.llvm.org/D25208 ___ cfe-commits mailing list cfe-co

[PATCH] D27430: [libc++] Annotate template methods with visibility

smeenai requested a review of this revision. smeenai added a comment. I think this actually stands well on its own. I'll do the extern template annotations in a different diff. Also, ping :) https://reviews.llvm.org/D27430 ___ cfe-commits mailing

[PATCH] D27153: [libc++] Make __num_get_float hidden

This revision was automatically updated to reflect the committed changes. Closed by commit rL290503: [libc++] Make __num_get_float hidden (authored by smeenai). Changed prior to commit: https://reviews.llvm.org/D27153?vs=79362&id=82452#toc Repository: rL LLVM https://reviews.llvm.org/D27153

[PATCH] D26949: [libc++abi] Clean up visibility

smeenai added a comment. Ping. https://reviews.llvm.org/D26949 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28174: [libcxx] [NFC] Rename _LIBCPP_TYPE_VIS_ONLY to _LIBCPP_TEMPLATE_VIS.

smeenai added a comment. As you're aware, I have plans to change `_LIBCPP_TYPE_VIS` to expand to `visibility` instead of `type_visibility`, but I still think this rename makes sense. https://reviews.llvm.org/D28174 ___ cfe-commits mailing list cfe

[PATCH] D25208: [libc++] Make _LIBCPP_TYPE_VIS export members

smeenai added a comment. Not gonna submit this till https://reviews.llvm.org/D27430 has been submitted (will address comments on that one after holidays). https://reviews.llvm.org/D25208 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http:

[PATCH] D28220: provide Win32 native threading

smeenai added inline comments. Comment at: include/__threading_support:83 +} __libcpp_mutex_t; +#define _LIBCPP_MUTEX_INITIALIZER {{0}, SRWLOCK_INIT, 0} + Why not a tagged union? Repository: rL LLVM https://reviews.llvm.org/D28220

[PATCH] D28220: provide Win32 native threading

smeenai added inline comments. Comment at: include/__threading_support:300-305 +int __libcpp_recursive_mutex_init(__libcpp_mutex_t *__m) +{ + InitializeSRWLock(__m); + return 0; +} + majnemer wrote: > compnerd wrote: > > majnemer wrote: > > > I don't think you

[PATCH] D28223: clean up use of _WIN32

smeenai added a comment. Sweet. Comment at: include/__config:158 +#if defined(_WIN32) +# define _LIBCPP_WIN32 1 # define _LIBCPP_LITTLE_ENDIAN 1 Perhaps `_LIBCPP_WIN32API` instead, to be clear that this is specific to the usage of Win32 APIs, rather

[PATCH] D28223: clean up use of _WIN32

smeenai added a comment. In https://reviews.llvm.org/D28223#634199, @compnerd wrote: > I figured that using the explicit encoding is nicer since it means that its > more documenting. I think @kimgr's point was that if you're using an explicit encoding, it should be UTF-16 rather than UCS-2 :)

[PATCH] D28223: clean up use of _WIN32

smeenai added inline comments. Comment at: include/support/win32/support.h:113 // Returns zero if no set bit is found. -#if defined(_WIN64) +#if defined(__LP64__) if (_BitScanForward64(&where, mask)) Windows wouldn't have `__LP64__` defined (since it's LLP64

[PATCH] D27430: [libc++] Annotate template methods with visibility

smeenai added a comment. In https://reviews.llvm.org/D27430#632652, @EricWF wrote: > Please put these attributes to the first declaration instead of the > definition. Sounds good to me for `_LIBCPP_HIDDEN`, but as far as I understand, for the `inline _LIBCPP_INLINE_VISIBILITY` cases, at least

[PATCH] D27430: [libc++] Annotate template methods with visibility

smeenai added a comment. In https://reviews.llvm.org/D27430#634880, @smeenai wrote: > Sounds good to me for `_LIBCPP_HIDDEN`, but as far as I understand, for the > `inline _LIBCPP_INLINE_VISIBILITY` cases, at least the `inline` needs to be > at the definition itself, otherwise it won't have any

[PATCH] D27430: [libc++] Annotate template methods with visibility

smeenai added a comment. In https://reviews.llvm.org/D27430#634882, @smeenai wrote: > In https://reviews.llvm.org/D27430#634880, @smeenai wrote: > > > Sounds good to me for `_LIBCPP_HIDDEN`, but as far as I understand, for the > > `inline _LIBCPP_INLINE_VISIBILITY` cases, at least the `inline` n

[PATCH] D28338: thread_support: split out {,non-}recursive mutex

smeenai added a comment. Adding cfe-commits as a subscriber after diff creation won't send the email out AFAIK. You'll have to re-upload the patch. Repository: rL LLVM https://reviews.llvm.org/D28338 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D28316: [libc++] Cleanup and document <__threading_support>

smeenai added inline comments. Comment at: docs/DesignDocs/ThreadingSupportAPI.rst:17 + +The ``<__threading_support>`` header is where libc++ defines it's internal +threading interface. It contains forward declarations of the internal threading s/it's/its https

[PATCH] D28383: build: add a heuristic to determine the C++ ABI

smeenai added a comment. In https://reviews.llvm.org/D28383#637570, @majnemer wrote: > Why isn't this equivalent to `_MSC_VER` ? You can have scenarios where you're targeting the Itanium ABI but still have `_MSC_VER` defined, e.g. % clang -target i686-windows-itanium -fmsc-version=1900 -E -

[PATCH] D28220: provide Win32 native threading

smeenai added inline comments. Comment at: include/__threading_support:30 +#define WIN32_LEAN_AND_MEAN +#include +#include EricWF wrote: > > Can we do as Reid suggests and not expose users to windows.h? > > I was about to ask the same question. These includes

[PATCH] D28426: [libc++] Tolerate presence of __deallocate macro

smeenai added a comment. I'm guessing always defining `_LIBCPP_DISABLE_MACRO_CONFLICT_WARNINGS` on Windows isn't considered an acceptable workaround either? https://reviews.llvm.org/D28426 ___ cfe-commits mailing list cfe-commits@lists.llvm.org htt

[PATCH] D28220: provide Win32 native threading

smeenai added inline comments. Comment at: include/__threading_support:30 +#define WIN32_LEAN_AND_MEAN +#include +#include EricWF wrote: > EricWF wrote: > > smeenai wrote: > > > EricWF wrote: > > > > > Can we do as Reid suggests and not expose users to windows.

[PATCH] D28441: [libc++] [CMake] Link with /nodefaultlibs on Windows

smeenai added inline comments. Comment at: CMakeLists.txt:43 +if (WIN32 AND NOT MINGW) + set(LIBCXX_TARGETING_WINDOWS ON) +else() Not the biggest fan of this name, since it's not obvious why MinGW shouldn't count as targeting Windows. I thought of `LIBCXX_TARGE

[PATCH] D28442: [libc++] Implement terminate(), unexpected() and uncaught_exceptions() on Windows

smeenai accepted this revision. smeenai added a comment. This revision is now accepted and ready to land. Hooray for Microsoft for putting all these in msvcrt (their **C** runtime library) instead of msvcprt (their C++ runtime library), I guess :p Comment at: src/exception.cpp

[PATCH] D28441: [libc++] [CMake] Link with /nodefaultlibs on Windows

smeenai added inline comments. Comment at: lib/CMakeLists.txt:108-109 +if (LIBCXX_TARGETING_WINDOWS) + add_compile_flags(/Zl) + add_link_flags(/nodefaultlib) + add_library_flags(ucrt) # Universal C runtime halyavin wrote: > smeenai wrote: > > These should be g

[PATCH] D28441: [libc++] [CMake] Link with /nodefaultlibs on Windows

smeenai added a comment. One potential issue with always forcing the non-debug msvcrt is that any apps that link against libc++ would also then need to use the non-debug CRT libraries, otherwise you'd get all sorts of fun runtime errors (e.g. cross-domain frees). I think the default Visual Stud

[PATCH] D28512: [libc++] Pair _aligned_malloc with _aligned_free

smeenai created this revision. smeenai added reviewers: compnerd, EricWF, mclow.lists. smeenai added a subscriber: cfe-commits. Attempting to pair an `_aligned_malloc` with a regular free causes heap corruption. Pairing with `_aligned_free` is required instead. Makes the following libc++ tests pa

[PATCH] D28590: [Sema] Restrict explicit instantation definition dllexport

smeenai created this revision. smeenai added reviewers: hans, rnk. smeenai added subscribers: cfe-commits, steveire. In the case where the template class itself is already `dllexport`, the implicit instantiation will have already emitted all members. When we check the explicit instantiation defini

[PATCH] D28512: [libc++] Pair _aligned_malloc with _aligned_free

This revision was automatically updated to reflect the committed changes. Closed by commit rL291743: [libc++] Pair _aligned_malloc with _aligned_free (authored by smeenai). Changed prior to commit: https://reviews.llvm.org/D28512?vs=83783&id=84075#toc Repository: rL LLVM https://reviews.llv

[PATCH] D27387: [libc++] Add a key function for bad_function_call

smeenai added a subscriber: dexonsmith. smeenai added a comment. @jyknight http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20161205/178830.html has the reasoning for guarding behind an ABI macro (with @dexonsmith from Apple weighing in). https://reviews.llvm.org/D27387

[PATCH] D31363: [libc++] Remove cmake glob for source files

smeenai created this revision. Herald added a subscriber: mgorny. Globbing for source files is problematic because if a source file is added or removed, the configuration won't be run again, and so the build won't pick up on the added or removed file until the next configuration. cmake's help expl

[PATCH] D27387: [libc++] Add a key function for bad_function_call

smeenai added a comment. In https://reviews.llvm.org/D27387#711866, @EricWF wrote: > This LGTM, but I have a question: Should we add these dylib definitions right > away so that it's easier to adopt the ABI change in future? This will allow > legacy dylibs to support this ABI change in future.

[PATCH] D27387: [libc++] Add a key function for bad_function_call

This revision was automatically updated to reflect the committed changes. Closed by commit rL298937: [libc++] Add a key function for bad_function_call (authored by smeenai). Changed prior to commit: https://reviews.llvm.org/D27387?vs=90524&id=93276#toc Repository: rL LLVM https://reviews.ll

[PATCH] D31363: [libc++] Remove cmake glob for source files

smeenai planned changes to this revision. smeenai added a comment. I like the idea of verifying that all source files have been included. Will amend this accordingly. Thanks for the cmake pointer @beanz! Comment at: lib/CMakeLists.txt:304 if (LIBCXX_ENABLE_EXPERIMENTAL_LIBRAR

[PATCH] D31502: [libc++abi] Delete config.h

smeenai created this revision. It's now completely empty, so we can remove it entirely. https://reviews.llvm.org/D31502 Files: src/config.h src/cxa_default_handlers.cpp src/cxa_exception.cpp src/cxa_exception_storage.cpp src/cxa_guard.cpp src/cxa_handlers.cpp src/cxa_personality.c

[PATCH] D31502: [libc++abi] Delete config.h

smeenai added a comment. In https://reviews.llvm.org/D31502#714341, @EricWF wrote: > Thanks. I'm surprised this wasn't done earlier. I removed the `unistd.h` include this morning (r299087 ), which is what made the file empty. https://reviews.llvm.org/D31502

[PATCH] D31502: [libc++abi] Delete config.h

This revision was automatically updated to reflect the committed changes. Closed by commit rL299129: [libc++abi] Delete config.h (authored by smeenai). Changed prior to commit: https://reviews.llvm.org/D31502?vs=93496&id=93565#toc Repository: rL LLVM https://reviews.llvm.org/D31502 Files:

[PATCH] D31644: [libcxx] Fix check-cxx-abilist on OS X

smeenai accepted this revision. smeenai added a comment. This revision is now accepted and ready to land. Makes sense to me. Some questions inline, but they may not necessitate changes, so I'm accepting. Comment at: lib/CMakeLists.txt:166 + if (NOT LIBCXX_ENABLE_NEW_DELET

[PATCH] D31725: [libc++] Add _LIBCPP_DISABLE_EXTERN_TEMPLATE config option

smeenai created this revision. When the libc++ extern template macros were added, the intent was for it to be possible for consumers of the headers to disable extern templates (via `-D_LIBCPP_EXTERN_TEMPLATE(...)=`). Unfortunately, support for specifying function-like macros varies on the command

[PATCH] D31737: [libc++] Respect Windows Store app CRT restrictions

smeenai created this revision. Some CRT APIs are unavailable for Windows Store apps [1]. Detect when we're targeting the Windows Store and don't try to refer to non-existent CRT functions in that case. (This would otherwise lead to a compile error when using the libc++ headers and compiling for Wi

[PATCH] D31737: [libc++] Respect Windows Store app CRT restrictions

This revision was automatically updated to reflect the committed changes. Closed by commit rL299625: [libc++] Respect Windows Store app CRT restrictions (authored by smeenai). Changed prior to commit: https://reviews.llvm.org/D31737?vs=94308&id=94317#toc Repository: rL LLVM https://reviews.

[PATCH] D31798: [libc++] Drop support for CRTs older than VS 2015

smeenai created this revision. LLVM dropped support for Visual Studio versions older than 2015 quite some time ago, so I consider it safe to drop libc++'s support for older CRTs. The CRT in Visual Studio 2015 provides a lot of previously missing functions, so targeting it requires less special cas

[PATCH] D31798: [libc++] Drop support for CRTs older than VS 2015

This revision was automatically updated to reflect the committed changes. Closed by commit rL299743: [libc++] Drop support for CRTs older than VS 2015 (authored by smeenai). Changed prior to commit: https://reviews.llvm.org/D31798?vs=94468&id=94476#toc Repository: rL LLVM https://reviews.ll

[PATCH] D31725: [libc++] Add _LIBCPP_DISABLE_EXTERN_TEMPLATE config option

smeenai updated this revision to Diff 95189. smeenai added a comment. Add documentation https://reviews.llvm.org/D31725 Files: docs/UsingLibcxx.rst include/__config Index: include/__config === --- include/__config +++ include

[PATCH] D31725: [libc++] Add _LIBCPP_DISABLE_EXTERN_TEMPLATE config option

This revision was automatically updated to reflect the committed changes. Closed by commit rL300246: [libc++] Add _LIBCPP_DISABLE_EXTERN_TEMPLATE config option (authored by smeenai). Changed prior to commit: https://reviews.llvm.org/D31725?vs=95189&id=95190#toc Repository: rL LLVM https://r

[PATCH] D32109: [Driver] Limit .exe extension addition to Windows hosts

smeenai created this revision. The driver should invoke lld as `lld-link` rather than `lld-link.exe` when cross compiling. The alternative would be to teach lld's build system to always produce `lld-link.exe` even on non-Windows hosts, but I think it's cleaner to change the driver to respect the

[PATCH] D32109: [Driver] Limit .exe extension addition to Windows hosts

smeenai planned changes to this revision. smeenai added a comment. Forgot to fix a test https://reviews.llvm.org/D32109 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32213: [Sema] Use MSVC inner class behavior on Itanium

smeenai created this revision. Windows Itanium aims to use MSVC export and import semantics. Inner class members shouldn't be exported on a dllexport explicit instantiation definition of the outer class, and they shouldn't be imported on a dllimport explicit instantiation declaration of the outer

[PATCH] D28590: [Sema] Restrict explicit instantation definition dllexport

This revision was automatically updated to reflect the committed changes. Closed by commit rL291877: [Sema] Restrict explicit instantation definition dllexport (authored by smeenai). Changed prior to commit: https://reviews.llvm.org/D28590?vs=84062&id=84202#toc Repository: rL LLVM https://r

[PATCH] D28441: [libc++] [CMake] Link with /nodefaultlibs on Windows

smeenai added a comment. In https://reviews.llvm.org/D28441#646145, @EricWF wrote: > In https://reviews.llvm.org/D28441#639023, @smeenai wrote: > > > I think the right thing to do would be to either always compile two > > versions of libc++, one linked against the debug libraries and the other

[PATCH] D28441: [libc++] [CMake] Link with /nodefaultlibs on Windows

smeenai accepted this revision. smeenai added a comment. This revision is now accepted and ready to land. There's a bunch of follow-up work here, but I'm fine with this for now. https://reviews.llvm.org/D28441 ___ cfe-commits mailing list cfe-commit

[PATCH] D28441: [libc++] [CMake] Link with /nodefaultlibs on Windows

smeenai added inline comments. Comment at: lib/CMakeLists.txt:112 + add_library_flags(vcruntime) # C++ runtime + add_library_flags(msvcrt) # C runtime startup files + add_library_flags(iso_stdio_wide_specifiers) EricWF wrote: > smeenai wrote: > > EricWF wrote:

[PATCH] D28725: [libc++][CMake] Use debug MSVC runtimes when libc++ is built in debug mode

smeenai added a comment. Awesome. LGTM except that force-removing the `_DEBUG` define and then linking against the debug libraries might cause some wonkiness. Comment at: CMakeLists.txt:394 # non-debug DLLs remove_flags("/D_DEBUG" "/MTd" "/MDd" "/MT" "/Md" "/RTC1") ---

[PATCH] D28725: [libc++][CMake] Use debug MSVC runtimes when libc++ is built in debug mode

smeenai added inline comments. Comment at: CMakeLists.txt:394 # non-debug DLLs remove_flags("/D_DEBUG" "/MTd" "/MDd" "/MT" "/Md" "/RTC1") EricWF wrote: > smeenai wrote: > > We should be able to remove this now, right? > I would still rather strip it and re-ad

[PATCH] D28725: [libc++][CMake] Use debug MSVC runtimes when libc++ is built in debug mode

smeenai added inline comments. Comment at: CMakeLists.txt:394 # non-debug DLLs remove_flags("/D_DEBUG" "/MTd" "/MDd" "/MT" "/Md" "/RTC1") EricWF wrote: > smeenai wrote: > > EricWF wrote: > > > smeenai wrote: > > > > We should be able to remove this now, right

[PATCH] D28725: [libc++][CMake] Use debug MSVC runtimes when libc++ is built in debug mode

smeenai accepted this revision. smeenai added a comment. This revision is now accepted and ready to land. LGTM apart from minor comments. https://reviews.llvm.org/D28725 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/c

[PATCH] D28728: [libc++] Introduce _LIBCPP_EXTERN_VIS to fix __libcpp_debug_function link errors

smeenai added a comment. This will need to be exported on other platforms too when hidden visibility happens :) Is it okay for these to be exported unconditionally, or should their exporting be determined by `_LIBCPP_DEBUG`? This LGTM if it's the former. https://reviews.llvm.org/D28728 ___

[PATCH] D25208: [libc++] Make _LIBCPP_TYPE_VIS export members

smeenai added a comment. In https://reviews.llvm.org/D25208#647743, @EricWF wrote: > Actually I probably shouldn't have approved this due to > http://llvm.org/PR30642. I forgot about that when I last reviewed this. Yup. I wrote https://github.com/smeenai/bad-visibility-finder to find all prob

[PATCH] D28785: Split exception.cpp and new.cpp implementation into different files for different runtimes

smeenai added a comment. I really like the cleanup, but I'm not the biggest fan of taking more Microsoft dependencies than necessary. I don't see any way around `operator new`/`operator delete` (since replacement in COFF can only work with a static library with one function per object file, the

[PATCH] D28785: Split exception.cpp and new.cpp implementation into different files for different runtimes

smeenai added inline comments. Comment at: include/exception:85 +#if defined(_LIBCPP_ABI_MICROSOFT) +#include EricWF wrote: > smeenai wrote: > > What's the rationale for relying on Microsoft's exception implementation > > rather than libc++'s? > `vcruntime_ne

[PATCH] D28785: Split exception.cpp and new.cpp implementation into different files for different runtimes

smeenai added a comment. @EricWF and I discussed this on IRC a bunch yesterday. We were inadvertently statically linking the Microsoft C++ libraries into tests, which is why `std::set_new_handler` was working with this patch. With that incorrect link removed, we get an undefined reference to `

[PATCH] D25208: [libc++] Make _LIBCPP_TYPE_VIS export members

smeenai updated this revision to Diff 85816. smeenai edited the summary of this revision. smeenai removed a reviewer: compnerd. smeenai added a comment. Folding https://reviews.llvm.org/D27430 and addressing comments from that diff. https://reviews.llvm.org/D25208 Files: docs/DesignDocs/Visib

[PATCH] D27430: [libc++] Annotate template methods with visibility

smeenai abandoned this revision. smeenai added a comment. Folded into https://reviews.llvm.org/D25208 https://reviews.llvm.org/D27430 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29157: [libc++] Make _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS export members

smeenai created this revision. When building libc++ with hidden visibility, we want explicit template instantiations to export members. This is consistent with existing Windows behavior, and is necessary for clients to be able to link against a hidden visibility built libc++ without running into l

[PATCH] D25208: [libc++] Make _LIBCPP_TYPE_VIS export members

smeenai added a comment. Ping. https://reviews.llvm.org/D25208 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29157: [libc++] Make _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS export members

smeenai added a comment. Ping. https://reviews.llvm.org/D29157 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26949: [libc++abi] Clean up visibility

smeenai updated this revision to Diff 87362. smeenai edited the summary of this revision. smeenai added a comment. Addressing comments https://reviews.llvm.org/D26949 Files: include/__cxxabi_config.h src/abort_message.cpp src/abort_message.h src/cxa_exception.cpp src/cxa_exception.hpp

[PATCH] D26949: [libc++abi] Clean up visibility

smeenai added inline comments. Comment at: src/abort_message.cpp:25 -#pragma GCC visibility push(hidden) - EricWF wrote: > Is this really redundant? There is an `#include ` > after it. Is this not going to affect those symbols? That's a fair point. I think it'

[PATCH] D25208: [libc++] Make _LIBCPP_TYPE_VIS export members

smeenai updated this revision to Diff 87911. smeenai added a comment. Rebase and ping https://reviews.llvm.org/D25208 Files: docs/DesignDocs/VisibilityMacros.rst include/__config include/__locale include/__mutex_base include/condition_variable include/future include/mutex includ

<    1   2   3   4   5   6   7   8   >