[PATCH] D50144: Add Windows support for the GNUstep Objective-C ABI V2.

smeenai added inline comments. Comment at: lib/AST/MicrosoftMangle.cpp:1886 case BuiltinType::ObjCId: -mangleArtificalTagType(TTK_Struct, "objc_object"); +mangleArtificalTagType(TTK_Struct, ".objc_object"); break; theraven wrote: > compnerd wrote:

[PATCH] D50144: Add Windows support for the GNUstep Objective-C ABI V2.

smeenai added a subscriber: rnk. smeenai added a comment. Sorry for the belated response; I was on vacation. I'm confused by the funclet-based unwinding support. Do you intend GNUStep to be used with windows-msvc triples? If so, how is the runtime throwing exceptions? We took the approach of ha

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

smeenai added a comment. In https://reviews.llvm.org/D42933#1091234, @aaron.ballman wrote: > In https://reviews.llvm.org/D42933#1090268, @jfb wrote: > > > I was just looking at this, and I think @arphaman's patch is pretty much > > the right approach (with 2 suggested fixes below). > > > > I don

[PATCH] D46593: Allow copy elision in path concatenation

smeenai added a comment. Generating the patch from libc++ is fine (and this patch looks like it has sane paths). https://reviews.llvm.org/D46593 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/

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

smeenai added a subscriber: rsmith. smeenai added a comment. In https://reviews.llvm.org/D42933#1091943, @jyknight wrote: > In https://reviews.llvm.org/D42933#1091817, @jfb wrote: > > > In https://reviews.llvm.org/D42933#1091809, @jyknight wrote: > > > > > I also think that special casing these t

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

smeenai added a comment. In https://reviews.llvm.org/D42933#1092048, @rjmccall wrote: > I agree that the format-specifier checker is not intended to be a portability > checker. > > Any attempt to check portability problems has to account for two things: > > - Not all code is intended to be porta

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

smeenai added a comment. Yeah, I think we all agree now that a portability warning isn't really tractable. Note that even for the warnings that motivated this diff, they should have only fired if `size_t` and NSInteger had separate types, so it wasn't a portability warning in that sense to begi

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

smeenai added a comment. Emailed cfe-dev: http://lists.llvm.org/pipermail/cfe-dev/2018-May/057934.html Repository: rC Clang https://reviews.llvm.org/D42933 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mail

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

smeenai created this revision. smeenai added reviewers: DHowett-MSFT, compnerd, majnemer, rjmccall, rnk. We're implementing funclet-compatible code generation for Obj-C exceptions when using the MSVC ABI. The idea is that the Obj-C runtime will wrap Obj-C exceptions inside C++ exceptions, which al

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

smeenai updated this revision to Diff 148133. smeenai added a comment. Colocate CHECK lines with declarations Repository: rC Clang https://reviews.llvm.org/D47233 Files: lib/CodeGen/CGCXXABI.h lib/CodeGen/CGObjCMac.cpp lib/CodeGen/MicrosoftCXXABI.cpp test/CodeGenObjC/dllstorage.m t

[PATCH] D38522: [libc++] Support Microsoft ABI without vcruntime headers

smeenai created this revision. Herald added a subscriber: mgorny. The vcruntime headers are hairy and clash with both libc++ headers themselves and other libraries. libc++ normally deals with the clashes by deferring to the vcruntime headers and silencing its own definitions, but for clients which

[PATCH] D38522: [libc++] Support Microsoft ABI without vcruntime headers

smeenai updated this revision to Diff 117613. smeenai added a comment. Remove SAL annotations as well; they're unneeded and add a header dependency https://reviews.llvm.org/D38522 Files: CMakeLists.txt docs/UsingLibcxx.rst include/__config_site.in include/exception include/new src/n

[PATCH] D38522: [libc++] Support Microsoft ABI without vcruntime headers

smeenai added a comment. In https://reviews.llvm.org/D38522#887768, @compnerd wrote: > Why do we expect the tests to fail without vcruntime headers? msvcrt's implementations of the various new/delete operators call `__scrt_throw_std_bad_alloc` and `__scrt_throw_std_bad_array_new_length`, which

[PATCH] D38522: [libc++] Support Microsoft ABI without vcruntime headers

smeenai added inline comments. Comment at: src/support/runtime/exception_msvc.ipp:31 +extern "C" { +typedef void(__CRTDECL* terminate_handler)(); +_ACRTIMP terminate_handler __cdecl set_terminate( smeenai wrote: > compnerd wrote: > > Really? clang-format removes

[PATCH] D36713: [libc++] Add a persistent way to disable availability

smeenai abandoned this revision. smeenai added a comment. I ended up handling this differently internally (via a custom site config). If someone else ends up needing the same functionality, they can revive it. https://reviews.llvm.org/D36713 ___ cf

[PATCH] D36719: [libc++] Add site config option for ABI macros

smeenai updated this revision to Diff 117757. smeenai added a comment. Address comments https://reviews.llvm.org/D36719 Files: CMakeLists.txt docs/BuildingLibcxx.rst include/__config_site.in utils/libcxx/test/config.py Index: utils/libcxx/test/config.py ===

[PATCH] D36719: [libc++] Add site config option for ABI macros

smeenai updated this revision to Diff 117758. smeenai added a comment. Fix RST https://reviews.llvm.org/D36719 Files: CMakeLists.txt docs/BuildingLibcxx.rst include/__config_site.in utils/libcxx/test/config.py Index: utils/libcxx/test/config.py

[PATCH] D36719: [libc++] Add site config option for ABI macros

This revision was automatically updated to reflect the committed changes. Closed by commit rL314946: [libc++] Add site config option for ABI macros (authored by smeenai). Changed prior to commit: https://reviews.llvm.org/D36719?vs=117758&id=117759#toc Repository: rL LLVM https://reviews.llv

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

smeenai added subscribers: zturner, rjmccall. smeenai added a comment. @rjmccall, this adds a libc++ build dependency on LLVM's cmake modules. There were some issues when @zturner had added a similar dependency for his work on lit. To confirm, Apple still cares about the use case of building lib

[PATCH] D38599: Remove warnings for dynamic_cast fallback.

smeenai added a comment. Does dlopen cause issues even with `RTLD_GLOBAL`? Repository: rL LLVM https://reviews.llvm.org/D38599 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38522: [libc++] Support Microsoft ABI without vcruntime headers

smeenai accepted this revision. smeenai added a comment. This revision is now accepted and ready to land. I spoke to @EricWF on IRC last week and got his approval to commit this without review if no one had gotten to it in a couple of days, so I'm going ahead with that. https://reviews.llvm.or

[PATCH] D38522: [libc++] Support Microsoft ABI without vcruntime headers

This revision was automatically updated to reflect the committed changes. Closed by commit rL315234: [libc++] Support Microsoft ABI without vcruntime headers (authored by smeenai). Changed prior to commit: https://reviews.llvm.org/D38522?vs=117613&id=118251#toc Repository: rL LLVM https://r

[PATCH] D37182: [libcxx] Special visibility macros for the experimental library

smeenai added a comment. Sorry, this has been on my queue for a long time, but I haven't gotten the chance to get to it yet. I'll try to take a look (at this and your other patches) in the next few days. https://reviews.llvm.org/D37182 ___ cfe-com

[PATCH] D39149: [libc++] Prevent tautological comparisons

smeenai created this revision. Tip-of-tree clang produces `-Wtautological-constant-compare` for tautological constant comparisons, and these pieces of code will trigger that diagnostic when `int` and `long` have the same size (for example, when compiling for a 32-bit target, or for Windows 64-bit)

[PATCH] D38704: [libunwind] Abstract rwlocks into a class, provide a SRW lock implementation for windows

smeenai added a comment. BTW LLVM requires a minimum of Windows 7 according to https://releases.llvm.org/3.8.0/docs/ReleaseNotes.html, so you can rely on SRW locks always being present. If LLVM still has a fallback path for when SRW locks are absent, that could be cleaned up. https://reviews.

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

smeenai added subscribers: compnerd, smeenai. smeenai added a comment. This seems ... suboptimal. It breaks existing code in the sense that their code was already broken, and the compiler is just diagnosing it correctly now. Note that Apple themselves recommend casting and using a proper printf

[PATCH] D43033: [WinEH] Put funclet bundles on inline asm calls

smeenai added a comment. LGTM, but I'd like @majnemer to take a look, since I'm not very familiar with this code. https://reviews.llvm.org/D43033 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo

[PATCH] D43110: [Sema] Don't mark plain MS enums as fixed

smeenai added a comment. We've seen lots of spurious warnings from this. Thanks for the fix! https://reviews.llvm.org/D43110 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43184: [WIP] [ItaniunCXXABI] Add an option for loading the type info vtable with dllimport

smeenai added a comment. FYI, binutils auto-import actually creates a fake IAT entry rather than using a dynamic initializer. I think it's actually a pretty cute trick. http://blog.omega-prime.co.uk/2011/07/04/everything-you-never-wanted-to-know-about-dlls/#how-auto-import-works has details. Th

[PATCH] D43586: CodeGen: handle blocks correctly when inalloca'ed

smeenai added a comment. Patch is missing context. Comment at: lib/CodeGen/CGDecl.cpp:1851 // The only implicit argument a block has is its literal. // We assume this is always passed directly. if (BlockInfo) { This comment needs to be updated.

[PATCH] D43700: Emit proper CodeView even when not using the cl driver.

smeenai added a comment. We were affected by this too. Thanks for fixing! (We're actually using CodeView + DWARF, since we have a bunch of tooling built around DWARF. We use the gcc-style driver for legacy reasons.) https://reviews.llvm.org/D43700 ___

[PATCH] D43700: Emit proper CodeView even when not using the cl driver.

smeenai added a comment. The `-g` meaning `-gcodeview` for MSVC environments change should be a separate diff though, right? https://reviews.llvm.org/D43700 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailm

[PATCH] D33082: Fix Libc++ build with MinGW64

smeenai accepted this revision. smeenai added a comment. LGTM. Thanks for all the revisions! https://reviews.llvm.org/D33082 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

smeenai abandoned this revision. smeenai added a comment. Will just drop the .exe entirely, since that works https://reviews.llvm.org/D32109 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c

[PATCH] D33923: [Driver] Don't force .exe suffix for lld

smeenai created this revision. When cross-compiling to Windows using lld, we want the driver to invoke it as lld-link rather than lld-link.exe. On Windows, the LLVM fs functions take care of adding the .exe suffix where necessary, so we can just drop the addition in the toolchain entirely. https

[PATCH] D33923: [Driver] Don't force .exe suffix for lld

This revision was automatically updated to reflect the committed changes. Closed by commit rL304761: [Driver] Don't force .exe suffix for lld (authored by smeenai). Changed prior to commit: https://reviews.llvm.org/D33923?vs=101495&id=101502#toc Repository: rL LLVM https://reviews.llvm.org/

[PATCH] D34588: Check for _MSC_VER before define _LIBCPP_MSVCRT

smeenai added reviewers: compnerd, majnemer, rnk. smeenai added subscribers: majnemer, compnerd. smeenai added a comment. This looks sensible to me. I don't know if there are any scenarios in which you'd be using the Microsoft CRT without having `_MSC_VER` defined, however. Added some people who

[PATCH] D41368: [libc++] Ignore bogus tautologic comparison warnings

smeenai created this revision. smeenai added reviewers: EricWF, mclow.lists. Herald added a subscriber: cfe-commits. clang 6 has a new diagnostic -Wtautological-constant-compare, which fires for the code in question when int and long have the same size (for example, when compiling for a 32-bit tar

[PATCH] D41368: [libc++] Ignore bogus tautologic comparison warnings

smeenai updated this revision to Diff 127416. smeenai added a comment. Remove stray comment Repository: rCXX libc++ https://reviews.llvm.org/D41368 Files: include/istream src/string.cpp Index: src/string.cpp === --- src/st

[PATCH] D39149: [libc++] Prevent tautological comparisons

smeenai added a comment. I put up https://reviews.llvm.org/D41368 to just suppress the warnings instead. https://reviews.llvm.org/D39149 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commi

[PATCH] D41368: [libc++] Ignore bogus tautologic comparison warnings

smeenai added a comment. @mclow.lists are you okay with this approach? I'm also fine using a cast to silence the warning, as @zturner suggested, but we should suppress the warning in some way, otherwise libc++ 6 is gonna have compile warnings with clang 6 out of the box, which isn't great. A t

[PATCH] D41368: [libc++] Ignore bogus tautologic comparison warnings

smeenai added a comment. Also FWIW I agree with you that the warning is completely bogus here, but it looks like the fix to that warning isn't gonna make it into clang 6, at least, so we have to adjust accordingly. Repository: rCXX libc++ https://reviews.llvm.org/D41368 _

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

smeenai added a comment. I posted to cfe-dev: http://lists.llvm.org/pipermail/cfe-dev/2017-December/056450.html Repository: rL LLVM https://reviews.llvm.org/D39462 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi

[PATCH] D41660: [cmake] Add new linux toolchain file

smeenai added a comment. Why is this a cache file rather than a toolchain file (but passing itself as a toolchain file to CMake under some circumstances?) Aren't toolchain files traditionally used for cross-compilation? Comment at: cmake/caches/linux-toolchain.cmake:20 +#

[PATCH] D41660: [cmake] Add new linux toolchain file

smeenai added a comment. In https://reviews.llvm.org/D41660#965686, @hintonda wrote: > In https://reviews.llvm.org/D41660#965656, @smeenai wrote: > > > > > > Cache files are preferred since they are only loaded once Isn't that precisely the behavior needed for cross-compilation though? You want

[PATCH] D41512: [Sema] -Wtautological-constant-compare is too good. Cripple it.

smeenai added inline comments. Comment at: include/clang/Basic/DiagnosticGroups.td:440 def TautologicalUnsignedEnumZeroCompare : DiagGroup<"tautological-unsigned-enum-zero-compare">; +def TautologicalRangeCompare : DiagGroup<"tautological-constant-range-compare">; def Tautolo

[PATCH] D41764: [libcxx] [cmake] Add a config option LIBCXX_HAS_WIN32_THREADS for enforcing win32 threads

smeenai added a comment. I think `LIBCXX_HAS_WIN32_THREAD_API` would be more consistent with the existing configuration define names? Comment at: CMakeLists.txt:277 endif() + if(LIBCXX_HAS_WIN32_THREADS) +message(FATAL_ERROR "LIBCXX_HAS_WIN32_THREADS can only be set to

[PATCH] D41764: [libcxx] [cmake] Add a config option LIBCXX_HAS_WIN32_THREADS for enforcing win32 threads

smeenai accepted this revision. smeenai added a comment. This revision is now accepted and ready to land. Can you add documentation for `_LIBCPP_HAS_THREAD_API_WIN32` to `docs/DesignDocs/ThreadingSupportAPI.rst`? It should have been documented before, but this seems like a good opportunity to co

[PATCH] D41368: [libc++] Ignore bogus tautologic comparison warnings

smeenai abandoned this revision. smeenai added a comment. The warning was removed from `-Wall`. Repository: rCXX libc++ https://reviews.llvm.org/D41368 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/

[PATCH] D39562: [CodeGen][ObjC] Fix an assertion failure caused by copy elision

smeenai added a comment. Any updates here? We were hitting a similar error internally, which this patch fixed. https://reviews.llvm.org/D39562 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cf

[PATCH] D43842: CodeGenObjCXX: handle inalloca appropriately for msgSend variant

smeenai added a comment. In https://reviews.llvm.org/D43842#1022498, @rjmccall wrote: > Ugh, I hate `inalloca` *so much*. > > It's still an indirect return, right? It's just that the return-slot pointer > has to get stored to the `inalloca` allocation like any other argument? Correct. Repos

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

smeenai created this revision. smeenai added reviewers: rnk, rsmith. EmitInlinedInheritingCXXConstructorCall may result in a CallBaseDtor cleanup being pushed. That cleanup would then be popped when the CGF's CurCodeDecl no longer points to the method which triggered the cleanup, leading to a fail

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

smeenai added a comment. Argh, this fixes my test case, but causes failures in some other ones. Back to the drawing board, I guess. Repository: rC Clang https://reviews.llvm.org/D44619 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http

[PATCH] D44640: [CodeGen] Add funclet token to ARC marker

smeenai created this revision. smeenai added reviewers: ahatanak, compnerd, majnemer, rjmccall, rnk. The inline assembly generated for the ARC autorelease elision marker must have a funclet token if it's emitted inside a funclet, otherwise the inline assembly (and all subsequent code in the funcle

[PATCH] D44640: [CodeGen] Add funclet token to ARC marker

This revision was automatically updated to reflect the committed changes. Closed by commit rC327892: [CodeGen] Add funclet token to ARC marker (authored by smeenai, committed by ). Changed prior to commit: https://reviews.llvm.org/D44640?vs=138973&id=138983#toc Repository: rL LLVM https://r

[PATCH] D44640: [CodeGen] Add funclet token to ARC marker

This revision was automatically updated to reflect the committed changes. Closed by commit rL327892: [CodeGen] Add funclet token to ARC marker (authored by smeenai, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D44640?vs=138973&id=138

[PATCH] D44646: Sema: in msvc compatibility mode, don't allow forceinline on variadics

smeenai added a reviewer: compnerd. smeenai added a comment. @DHowett-MSFT the reviewers look fine to me. Reid is the code owner for clang's MSVC compat support. David doesn't work on this stuff directly anymore, I think, but he's still pretty active in code reviews for it. I'm adding Saleem, w

[PATCH] D44671: [libcxx] Enable static libcxxabi linking on Darwin

smeenai added a comment. I would imagine there was a reason this configuration was unsupported on macOS – perhaps something to do with libc++/libc++abi being the default system libraries on that platform? Comment at: libcxx/CMakeLists.txt:330 # Check that this option is no

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

smeenai added a comment. In https://reviews.llvm.org/D47233#1115630, @DHowett-MSFT wrote: > This largely matches what we've done in the WinObjC clang patchset here > ,

[PATCH] D47564: [Parse] Use CapturedStmt for @finally on MSVC

smeenai created this revision. smeenai added reviewers: rjmccall, rnk, rsmith. Herald added a subscriber: cfe-commits. The body of a @finally needs to be executed on both exceptional and non-exceptional paths. On landingpad platforms, this is straightforward: the @finally body is emitted as a norm

[PATCH] D47564: [Parse] Use CapturedStmt for @finally on MSVC

smeenai updated this revision to Diff 149220. smeenai added a comment. Proper diff Repository: rC Clang https://reviews.llvm.org/D47564 Files: include/clang/AST/Stmt.h include/clang/Basic/CapturedStmt.h include/clang/Sema/ScopeInfo.h lib/Parse/ParseObjc.cpp test/SemaObjC/finally-ms

[PATCH] D47564: [Parse] Use CapturedStmt for @finally on MSVC

smeenai updated this revision to Diff 149351. smeenai edited the summary of this revision. smeenai added a comment. @rnk comment Repository: rC Clang https://reviews.llvm.org/D47564 Files: include/clang/AST/Stmt.h include/clang/Basic/CapturedStmt.h include/clang/Sema/ScopeInfo.h lib/

[PATCH] D47564: [Parse] Use CapturedStmt for @finally on MSVC

smeenai added a comment. In https://reviews.llvm.org/D47564#1118381, @rjmccall wrote: > That's an interesting idea. I don't see any particular reason not to do it > this way if you're willing to accept that it's never going to support the > full control-flow possibilities of @finally. You wil

[PATCH] D47669: [cmake] Support LLD for CLANG_ORDER_FILE

smeenai created this revision. smeenai added reviewers: beanz, compnerd, phosek. Herald added a subscriber: mgorny. smeenai added a subscriber: alexshap. LLD also supports order files using the `--symbol-ordering-file` option. As the name would suggest, the order file format is slightly different

[PATCH] D47669: [cmake] Support LLD for CLANG_ORDER_FILE

This revision was automatically updated to reflect the committed changes. Closed by commit rL333810: [cmake] Support LLD for CLANG_ORDER_FILE (authored by smeenai, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D47669 Files: cfe/trunk/t

[PATCH] D47564: [Parse] Use CapturedStmt for @finally on MSVC

smeenai added a comment. In https://reviews.llvm.org/D47564#1118424, @rjmccall wrote: > In https://reviews.llvm.org/D47564#1118423, @smeenai wrote: > > > In https://reviews.llvm.org/D47564#1118381, @rjmccall wrote: > > > > > That's an interesting idea. I don't see any particular reason not to do

[PATCH] D47564: [Parse] Use CapturedStmt for @finally on MSVC

smeenai added a comment. In https://reviews.llvm.org/D47564#1119925, @rjmccall wrote: > In https://reviews.llvm.org/D47564#1119874, @smeenai wrote: > > > In https://reviews.llvm.org/D47564#1118424, @rjmccall wrote: > > > > > No, it was just a general question. Have you gotten this to a point >

[PATCH] D47850: [Driver] Stop passing -fseh-exceptions for x86_64-windows-msvc

smeenai created this revision. smeenai added reviewers: compnerd, mstorsjo, rnk. Herald added subscribers: JDevlieghere, aprantl. -fseh-exceptions is only meaningful for MinGW targets, and that driver already has logic to pass either -fdwarf-exceptiosn or -fseh-exceptions as appropriate. -fseh-exc

[PATCH] D47850: [Driver] Stop passing -fseh-exceptions for x86_64-windows-msvc

This revision was automatically updated to reflect the committed changes. Closed by commit rL334145: [Driver] Stop passing -fseh-exceptions for x86_64-windows-msvc (authored by smeenai, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D47850

[PATCH] D47853: [Frontend] Disallow non-MSVC exception models for windows-msvc targets

smeenai created this revision. smeenai added reviewers: compnerd, mstorsjo, rnk. The windows-msvc target is used for MSVC ABI compatibility, including the exceptions model. It doesn't make sense to pair a windows-msvc target with a non-MSVC exception model. This would previously cause an assertion

[PATCH] D47862: [CodeGen] Always use MSVC personality for windows-msvc targets

smeenai created this revision. smeenai added reviewers: compnerd, DHowett-MSFT, rnk. The windows-msvc target is meant to be ABI compatible with MSVC, including the exception handling. Ensure that a windows-msvc triple always equates to the MSVC personality being used. This mostly affects the GNUS

[PATCH] D47853: [Frontend] Disallow non-MSVC exception models for windows-msvc targets

smeenai marked 2 inline comments as done. smeenai added inline comments. Comment at: test/CodeGen/personality.c:10 -// RUN: %clang_cc1 -triple x86_64-unknown-windows-msvc -D __SEH_EXCEPTIONS__ -fms-extensions -fexceptions -fblocks -fseh-exceptions -S -emit-llvm %s -o - | FileCh

[PATCH] D47853: [Frontend] Disallow non-MSVC exception models for windows-msvc targets

smeenai updated this revision to Diff 150394. smeenai marked 2 inline comments as done. smeenai added a comment. Add back missing MinGW coverage Repository: rC Clang https://reviews.llvm.org/D47853 Files: include/clang/Basic/DiagnosticFrontendKinds.td lib/Frontend/CompilerInvocation.cpp

[PATCH] D47564: [Parse] Use CapturedStmt for @finally on MSVC

This revision was automatically updated to reflect the committed changes. Closed by commit rL334224: [Parse] Use CapturedStmt for @finally on MSVC (authored by smeenai, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D47564 Files: cfe/tr

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

smeenai updated this revision to Diff 150399. smeenai added a comment. Address objc_exception attribute case Repository: rC Clang https://reviews.llvm.org/D47233 Files: lib/CodeGen/CGCXXABI.h lib/CodeGen/CGObjCMac.cpp lib/CodeGen/MicrosoftCXXABI.cpp test/CodeGenObjC/dllstorage.m te

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

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

[PATCH] D47912: [CMake] Consider LLVM_APPEND_VC_REV when generating SVNVersion.inc

smeenai added a comment. I believe LLVM_APPEND_VC_REV controls whether the revision is appended to LLVM version string (e.g. the output of `llvm-config --version`), whereas the SVNRevision.inc file (which is what's causing the relink after amending) is used for e.g. the `clang --version` output

[PATCH] D47853: [Frontend] Disallow non-MSVC exception models for windows-msvc targets

This revision was automatically updated to reflect the committed changes. Closed by commit rL334243: [Frontend] Disallow non-MSVC exception models for windows-msvc targets (authored by smeenai, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.or

[PATCH] D47862: [CodeGen] Always use MSVC personality for windows-msvc targets

smeenai added inline comments. Comment at: lib/CodeGen/CGException.cpp:134-135 const llvm::Triple &T = Target.getTriple(); + if (T.isWindowsMSVCEnvironment()) +return EHPersonality::MSVC_CxxFrameHandler3; + rnk wrote: > I guess previously we carefully arr

[PATCH] D47862: [CodeGen] Always use MSVC personality for windows-msvc targets

This revision was automatically updated to reflect the committed changes. Closed by commit rC334253: [CodeGen] Always use MSVC personality for windows-msvc targets (authored by smeenai, committed by ). Changed prior to commit: https://reviews.llvm.org/D47862?vs=150243&id=150437#toc Repository:

[PATCH] D47988: [CodeGen] Emit MSVC funclet IR for Obj-C exceptions

smeenai created this revision. smeenai added reviewers: DHowett-MSFT, compnerd, majnemer, rjmccall, rnk. We're implementing funclet-compatible code generation for Obj-C exceptions when using the MSVC ABI. The idea is that the Obj-C runtime will wrap Obj-C exceptions inside C++ exceptions, which al

[PATCH] D47988: [CodeGen] Emit MSVC funclet IR for Obj-C exceptions

smeenai added a comment. @rnk remember how I was asking you about inlining into catchpads on IRC a few days back? It was in relation to this change. If I have a file `finally1.m`: void f(void); void g() { @try { f(); } @finally { f(); } } and compile it with: c

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

smeenai added a comment. Wrapping an Objective-C exception inside a C++ exception means dynamically constructing a C++ exception object and traversing the class hierarchy of the thrown Obj-C object to populate the catchable types array of the C++ exception. Microsoft's C++ runtime will perform

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

smeenai added a comment. In https://reviews.llvm.org/D47233#1129810, @DHowett-MSFT wrote: > In https://reviews.llvm.org/D47233#1129207, @rjmccall wrote: > > > In https://reviews.llvm.org/D47233#1129110, @smeenai wrote: > > > > > WinObjC does this wrapping, for example. > > > > > > I see. The ide

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

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

[PATCH] D47988: [CodeGen] Emit MSVC funclet IR for Obj-C exceptions

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

[PATCH] D48356: [libcxx] [CMake] Convert paths to the right form in standalone builds on Windows

smeenai accepted this revision. smeenai added a comment. This revision is now accepted and ready to land. Looks good with the quotes added. Comment at: cmake/Modules/HandleCompilerRT.cmake:17 string(STRIP "${LIBRARY_FILE}" LIBRARY_FILE) + file(TO_CMAKE_PATH ${LIBRARY_FILE}

[PATCH] D48355: [libcxxabi] [CMake] Convert paths to the right form in standalone builds on Windows

smeenai accepted this revision. smeenai added a comment. This revision is now accepted and ready to land. LGTM with the same comments as https://reviews.llvm.org/D48356 (add quotes, and is the translation for LLVM_BINARY_DIR necessary?) Repository: rCXXA libc++abi https://reviews.llvm.org/D4

[PATCH] D48353: [libunwind] [CMake] Convert paths to the right form in standalone builds on Windows

smeenai accepted this revision. smeenai added a comment. This revision is now accepted and ready to land. LGTM with the same comments as https://reviews.llvm.org/D48356 (add quotes, and is the translation for LLVM_BINARY_DIR necessary?) Repository: rUNW libunwind https://reviews.llvm.org/D48

[PATCH] D48356: [libcxx] [CMake] Convert paths to the right form in standalone builds on Windows

smeenai added inline comments. Comment at: cmake/Modules/HandleOutOfTreeLLVM.cmake:52 else() - set(LLVM_CMAKE_PATH - "${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm") + file(TO_CMAKE_PATH ${LLVM_BINARY_DIR} LLVM_BINARY_DIR_CMAKE_STYLE) + set(L

[PATCH] D47988: [CodeGen] Emit MSVC funclet IR for Obj-C exceptions

smeenai added inline comments. Comment at: lib/CodeGen/CGCXXABI.h:248 +llvm_unreachable("Only needed for the Microsoft ABI"); + } rjmccall wrote: > Should you just generalize the existing method to only take a VarDecl* so it > can be used for either kind

[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] D52674: [AST] Add Obj-C discriminator to MS ABI RTTI

smeenai added a comment. @rjmccall I prototyped the ForRTTI parameter approach in https://reviews.llvm.org/D53546. It could definitely be cleaned up a bit, but it demonstrates the problems I saw with the added parameter. Namely, `mangleType(QualType, SourceRange, QualifierMangleMode)` has a bun

[PATCH] D52674: [AST] Add Obj-C discriminator to MS ABI RTTI

smeenai added a comment. Ping @rjmccall. Let me know if the approach in https://reviews.llvm.org/D53546 is what you'd been envisioning, or if I'm just doing something completely brain-dead :) Repository: rC Clang https://reviews.llvm.org/D52674 ___

[PATCH] D56816: [ObjC] Follow-up r350768 and allow the use of unavailable methods that are declared in a parent class from within the @implementation context

smeenai added a comment. My understanding is that rL349841 accidentally started producing some spurious warnings/errors, rL350768 fixed some instances, and this change fixes more instances. Given that the first two changes

[PATCH] D56652: [CMake][Fuchsia] Synchronize first and second stage builds

smeenai added inline comments. Comment at: cfe/trunk/cmake/caches/Fuchsia-stage2.cmake:29 -set(LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "") set(CMAKE_BUILD_TYPE RelWithDebInfo CACHE STRING "") Out of curiosity, how come you decided to disable assertions? https://

[PATCH] D57330: Adjust documentation for git migration.

smeenai accepted this revision. smeenai added a comment. This revision is now accepted and ready to land. The amount of duplicated and out-of-date documentation makes me sad, but this is an awesome effort to clean that up. Comment at: lldb/packages/Python/lldbsuite/test/funct

[PATCH] D57330: Adjust documentation for git migration.

smeenai added a comment. In D57330#1375913 , @mehdi_amini wrote: > In D57330#1375096 , @labath wrote: > > > This is not an full out-of-source build, since the build folder is still a > > subfolder of the repo root

[PATCH] D57330: Adjust documentation for git migration.

smeenai added a comment. In D57330#1376069 , @mehdi_amini wrote: > > You can avoid the git status pollution by adding the build directory to > > .git/info/exclude. > > Good to know! Should we include this in the doc? I can put that change up fore review

[PATCH] D52674: [AST] Add Obj-C discriminator to MS ABI RTTI

smeenai added a comment. In https://reviews.llvm.org/D52674#1281747, @rjmccall wrote: > In https://reviews.llvm.org/D52674#1271814, @smeenai wrote: > > > @rjmccall I prototyped the ForRTTI parameter approach in > > https://reviews.llvm.org/D53546. It could definitely be cleaned up a bit, > > bu

  1   2   3   4   5   6   7   8   >