Page MenuHomePhabricator

theraven (David Chisnall)
User

Projects

User does not belong to any projects.

User Details

User Since
Nov 19 2012, 2:56 AM (316 w, 6 d)

Recent Activity

Thu, Dec 13

theraven added a comment to D55640: [clang-tidy] Implement a check for large Objective-C type encodings ūüĒć.

I wonder if we want to have an option to elide ObjC type info for all non-POD C++ types. Nothing that you do with the type encoding is likely to be correct (for example, you can see the pointer field in a std::shared_ptr, but you can't see that changes to it need to update reference counts) so it probably does more harm than good.

Thu, Dec 13, 1:04 AM · Restricted Project
theraven added a comment to D55229: [COFF] Statically link certain runtime library functions.

If we're going to change the default, please at least add a flag to allow callers to opt into dlimport. We can then make that dependent on -static-objc or similar.

Thu, Dec 13, 1:01 AM

Tue, Dec 11

theraven added a comment to D55544: Warning: objc-encodings-larger-than=.

It would probably be a good idea to have a similar check on properties, as property encoding strings contain the type encoding (plus extra stuff).

Tue, Dec 11, 6:02 AM · Restricted Project
theraven added a comment to rL348726: Add OpenBSD support to OpenMP.

I wonder if it would be cleaner to define a KMP_OS_POSIX and a KMP_OS_BSD and avoid a load of 'if Linux, FreeBSD, NetBSD, DragonFlyBSD, HURD' checks. This would make it easier for the next person to support a UNIX-like or BSD-derived OS.

Tue, Dec 11, 12:58 AM

Tue, Nov 20

theraven added a comment to rL346732: CMake: Deprecate using llvm-config to detect llvm installation.

It's not clear from this whether there's a plan to remove llvm-config, or just to remove llvm-config as a way for clang to detect llvm. I would be strongly opposed to the former. Having tried to maintain out-of-tree code that builds with multiple versions of LLVM, it is more or less possible with llvm-config and totally impossible with the LLVM CMake files. Unless dropping llvm-config is accompanied by a strong commitment to long-term API stability for the LLVM CMake infrastructure, this would have a significant negative impact on downstream consumers of LLVM (and, even then, extracting the CMake information when your downstream build system is not CMake is painful).

Tue, Nov 20, 2:17 AM

Nov 15 2018

theraven added inline comments to D54539: [CodeGen] Replace '@' characters in block descriptors' symbol names with '\1' on ELF targets..
Nov 15 2018, 1:05 AM · Restricted Project

Oct 23 2018

theraven accepted D53547: NFC: Remove the ObjC1/ObjC2 distinction from clang (and related projects).

Looks good for me, and removing all of the code describing Objective-C 4 as ObjC1 makes me happy.

Oct 23 2018, 11:14 AM

Oct 12 2018

theraven added inline comments to D53162: [DataLayout] Add bit width of pointers to global values.
Oct 12 2018, 4:58 AM
theraven added inline comments to D53162: [DataLayout] Add bit width of pointers to global values.
Oct 12 2018, 3:23 AM

Sep 28 2018

theraven added a comment to D52581: [AST] Revert mangling changes from r339428.

The simplest option is something like P8109, where we add a .objc discriminator when mangling the RTTI itself. It would require the GNUStep runtime for Windows to be altered accordingly (e.g. https://github.com/gnustep/libobjc2/blob/master/eh_win32_msvc.cc#L85 would have to use ".objc.PAU" instead of ".PAU.objc_cls_"); would that be acceptable, @theraven and @DHowett-MSFT?

Sep 28 2018, 1:45 AM

Sep 27 2018

theraven added a comment to D47233: [CodeGen] Emit MSVC RTTI for Obj-C EH types.

We ran into some critical issues with this approach on x64. The pointers in the exception record are supposed to be image-relative instead of absolute, so I tried to make them absolute to libobjc2's load address, but I never quite solved it.

A slightly better-documented and cleaner version of the code you linked is here.

Sep 27 2018, 1:00 AM
theraven added a comment to D52581: [AST] Revert mangling changes from r339428.

I would have done the same for the GNUstep RTTI here, except I don't actually
see the code for that anywhere, and no tests seem to break either, so I
believe it's not upstreamed yet.

Sep 27 2018, 12:59 AM

Sep 23 2018

theraven added a comment to D52344: [Clang][CodeGen][ObjC]: Fix non-bridged CoreFoundation builds on ELF targets that use `-fconstant-cfstrings`..

Would you be okay with me renaming cfstring to .cfstring for ELF so it's in line with ELF section naming conventions (I'm not entirely sure if that could cause issues with ObjC stuff)? And yes I think it's best to avoid that code-path altogether if it turns out to be a constant as that's likely from the declaration of the type, I'll write a FileCheck test in a moment and check that I can apply the same logic to ELF aside from the DLL stuff as CoreFoundation needs to export the symbol somehow while preserving the implicit extern attribute for every other TU that comes from the builtin that CFSTR expands to. Is what I'm suggesting making sense? If not, let me know, I may be misunderstanding what's happening here.

Following the ELF conventions seems like the right thing to do, assuming it doesn't cause compatibility problems. David Chisnall might know best here.

Sep 23 2018, 2:39 AM

Sep 4 2018

theraven added a comment to D50783: [CodeGen] Merge identical block descriptor global variables.

The GNUstep documentation I found replaces '@' with '\1'. Would that fix the problem?

https://github.com/gnustep/libobjc2/blob/master/ABIDoc/abi.tex

Sep 4 2018, 6:03 AM
theraven committed rL341355: Add release notes for the new GNUstep Objective-C ABI..
Add release notes for the new GNUstep Objective-C ABI.
Sep 4 2018, 3:41 AM
theraven committed rL341354: Disable the GNUstep v2 ABI on Windows..
Disable the GNUstep v2 ABI on Windows.
Sep 4 2018, 3:41 AM
theraven committed rL341352: Revert "Disable the GNUstep v2 ABI on Windows.".
Revert "Disable the GNUstep v2 ABI on Windows."
Sep 4 2018, 3:10 AM
theraven committed rC341352: Revert "Disable the GNUstep v2 ABI on Windows.".
Revert "Disable the GNUstep v2 ABI on Windows."
Sep 4 2018, 3:10 AM
theraven committed rC341350: Disable the GNUstep v2 ABI on Windows..
Disable the GNUstep v2 ABI on Windows.
Sep 4 2018, 2:27 AM
theraven committed rL341350: Disable the GNUstep v2 ABI on Windows..
Disable the GNUstep v2 ABI on Windows.
Sep 4 2018, 2:27 AM

Aug 31 2018

theraven added inline comments to D50783: [CodeGen] Merge identical block descriptor global variables.
Aug 31 2018, 3:41 AM
theraven added a comment to D50783: [CodeGen] Merge identical block descriptor global variables.

This revision broke blocks on all ELF targets. The block descriptors' symbol names can now include the @ character, which is reserved on ELF platforms as a separator between symbol name and symbol version. As a result, nothing containing a block that has an Objective-C object argument will link. Please add mangling similar to that in the GNUstep runtime to avoid this.

Aug 31 2018, 3:40 AM

Aug 28 2018

theraven committed rL340807: Fix in getAllocationDataForFunction.
Fix in getAllocationDataForFunction
Aug 28 2018, 2:00 AM
theraven closed D50959: Fix in getAllocationDataForFunction.
Aug 28 2018, 2:00 AM

Aug 23 2018

theraven added a comment to D50144: Add Windows support for the GNUstep Objective-C ABI V2..

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 having the Obj-C runtime wrap Obj-C exceptions in C++ exceptions and then have vcruntime handle the rest (see https://reviews.llvm.org/D47233 and https://reviews.llvm.org/D47988), but I don't see any of the associated RTTI emission changes in this patch.

Aug 23 2018, 3:56 AM

Aug 21 2018

theraven accepted D50959: Fix in getAllocationDataForFunction.

Looks good to me. I will commit it unless anyone objects.

Aug 21 2018, 6:23 AM

Aug 16 2018

theraven added inline comments to D50144: Add Windows support for the GNUstep Objective-C ABI V2..
Aug 16 2018, 1:26 AM

Aug 15 2018

theraven added inline comments to D50144: Add Windows support for the GNUstep Objective-C ABI V2..
Aug 15 2018, 1:46 AM

Aug 14 2018

theraven committed rL339668: [gnu-objc] Make selector order deterministic..
[gnu-objc] Make selector order deterministic.
Aug 14 2018, 3:06 AM
theraven committed rC339668: [gnu-objc] Make selector order deterministic..
[gnu-objc] Make selector order deterministic.
Aug 14 2018, 3:06 AM
theraven closed D50559: [gnu-objc] Make selector order deterministic..
Aug 14 2018, 3:06 AM
theraven committed rL339667: Add a stub mangling for ObjC selectors in the Microsoft ABI..
Add a stub mangling for ObjC selectors in the Microsoft ABI.
Aug 14 2018, 3:05 AM
theraven committed rC339667: Add a stub mangling for ObjC selectors in the Microsoft ABI..
Add a stub mangling for ObjC selectors in the Microsoft ABI.
Aug 14 2018, 3:05 AM

Aug 13 2018

theraven updated the diff for D50559: [gnu-objc] Make selector order deterministic..
  • Add a test case.
Aug 13 2018, 2:17 AM

Aug 11 2018

theraven added inline comments to D50559: [gnu-objc] Make selector order deterministic..
Aug 11 2018, 4:15 AM

Aug 10 2018

theraven created D50559: [gnu-objc] Make selector order deterministic..
Aug 10 2018, 6:40 AM
theraven committed rC339429: Fix a deprecated warning in the last commit..
Fix a deprecated warning in the last commit.
Aug 10 2018, 5:54 AM
theraven committed rL339429: Fix a deprecated warning in the last commit..
Fix a deprecated warning in the last commit.
Aug 10 2018, 5:54 AM
theraven committed rL339428: Add Windows support for the GNUstep Objective-C ABI V2..
Add Windows support for the GNUstep Objective-C ABI V2.
Aug 10 2018, 5:54 AM
theraven committed rC339428: Add Windows support for the GNUstep Objective-C ABI V2..
Add Windows support for the GNUstep Objective-C ABI V2.
Aug 10 2018, 5:53 AM
theraven closed D50144: Add Windows support for the GNUstep Objective-C ABI V2..
Aug 10 2018, 5:53 AM
theraven updated the diff for D50144: Add Windows support for the GNUstep Objective-C ABI V2..

Squashed into a single commit.

Aug 10 2018, 5:12 AM

Aug 9 2018

theraven updated the diff for D50144: Add Windows support for the GNUstep Objective-C ABI V2..
  • Address Dustin's review comments.
  • Fix an issue in protocol generation.
  • Fix failing test.
  • Address rjmcall's review comments.
  • Add some missing comments and factor out SEH check.
Aug 9 2018, 3:42 AM
theraven added a comment to D50144: Add Windows support for the GNUstep Objective-C ABI V2..

I believe that this is now ready to land.

Aug 9 2018, 3:42 AM
theraven updated the diff for D50144: Add Windows support for the GNUstep Objective-C ABI V2..
  • Address rjmcall's review comments.
  • Revert blocks part of the patch to put in a separate review.
Aug 9 2018, 1:06 AM
theraven committed rC339317: Correctly initialise global blocks on Windows..
Correctly initialise global blocks on Windows.
Aug 9 2018, 1:03 AM
theraven committed rL339317: Correctly initialise global blocks on Windows..
Correctly initialise global blocks on Windows.
Aug 9 2018, 1:03 AM
theraven closed D50436: Correctly initialise global blocks on Windows..
Aug 9 2018, 1:03 AM

Aug 8 2018

theraven accepted D50448: [CGObjCGNU] Rename GetSelector helper method to fix -Woverloaded-virtual warning (PR38210).

Looks good to me. This method should probably take a StringRef rather than a const std::string&, but I can make that change separately.

Aug 8 2018, 8:39 AM
theraven added inline comments to D50144: Add Windows support for the GNUstep Objective-C ABI V2..
Aug 8 2018, 4:20 AM
theraven created D50436: Correctly initialise global blocks on Windows..
Aug 8 2018, 4:19 AM
theraven updated the diff for D50144: Add Windows support for the GNUstep Objective-C ABI V2..
  • Revert blocks part of the patch to put in a separate review.
Aug 8 2018, 1:54 AM
theraven updated the diff for D50144: Add Windows support for the GNUstep Objective-C ABI V2..
  • Address Dustin's review comments.
  • Fix an issue in protocol generation.
  • Fix failing test.
  • [gnu-objc] Make selector order deterministic.
  • Address rjmcall's review comments.
Aug 8 2018, 1:42 AM

Aug 7 2018

theraven committed rC339128: [objc-gnustep] Don't emit .guess ivar offset vars..
[objc-gnustep] Don't emit .guess ivar offset vars.
Aug 7 2018, 5:03 AM
theraven committed rL339128: [objc-gnustep] Don't emit .guess ivar offset vars..
[objc-gnustep] Don't emit .guess ivar offset vars.
Aug 7 2018, 5:03 AM
theraven added a comment to D50144: Add Windows support for the GNUstep Objective-C ABI V2..

I'd like to commit this, unless @rjmccall has any objections.

Aug 7 2018, 1:04 AM

Aug 6 2018

theraven updated the diff for D50144: Add Windows support for the GNUstep Objective-C ABI V2..
  • Fix failing test.
Aug 6 2018, 8:41 AM
theraven updated the diff for D50144: Add Windows support for the GNUstep Objective-C ABI V2..
  • Fix an issue in protocol generation.
Aug 6 2018, 6:38 AM
theraven added inline comments to D50144: Add Windows support for the GNUstep Objective-C ABI V2..
Aug 6 2018, 5:18 AM

Aug 3 2018

theraven updated the diff for D50144: Add Windows support for the GNUstep Objective-C ABI V2..
  • Address Dustin's review comments.
Aug 3 2018, 6:38 AM

Aug 1 2018

theraven created D50144: Add Windows support for the GNUstep Objective-C ABI V2..
Aug 1 2018, 9:03 AM

Jul 10 2018

theraven added a comment to D10480: Implement shared_mutex re: N4508.

I missed this when it went in and coming across the code now I'm quite surprised that it did. Why is shared_mutex not implemented as a wrapper around rwlocks (pthreads and Windows both provide this abstraction)? The current implementation looks a lot less efficient than the system version on any operating system that I'm familiar with and has the added effect that native_handle isn't possible to implement, meaning that these can't easily interoperate with C APIs either.

Jul 10 2018, 3:45 AM

May 26 2018

theraven added inline comments to D46052: GNUstep Objective-C ABI version 2.
May 26 2018, 4:25 AM
theraven added inline comments to D46052: GNUstep Objective-C ABI version 2.
May 26 2018, 4:19 AM

May 22 2018

theraven committed rL332966: [objc-gnustep2] Use unsigned char to avoid potential UB in isalnum..
[objc-gnustep2] Use unsigned char to avoid potential UB in isalnum.
May 22 2018, 3:17 AM
theraven committed rL332963: Revert "Revert r332955 "GNUstep Objective-C ABI version 2"".
Revert "Revert r332955 "GNUstep Objective-C ABI version 2""
May 22 2018, 3:17 AM
theraven committed rL332965: [objc-gnu] Fix test..
[objc-gnu] Fix test.
May 22 2018, 3:17 AM
theraven committed rL332964: [objc-gnustep2] Use isalnum instead of a less efficient and nonportable….
[objc-gnustep2] Use isalnum instead of a less efficient and nonportable…
May 22 2018, 3:17 AM
theraven committed rC332965: [objc-gnu] Fix test..
[objc-gnu] Fix test.
May 22 2018, 3:17 AM
theraven committed rC332966: [objc-gnustep2] Use unsigned char to avoid potential UB in isalnum..
[objc-gnustep2] Use unsigned char to avoid potential UB in isalnum.
May 22 2018, 3:17 AM
theraven committed rC332964: [objc-gnustep2] Use isalnum instead of a less efficient and nonportable….
[objc-gnustep2] Use isalnum instead of a less efficient and nonportable…
May 22 2018, 3:17 AM
theraven committed rC332963: Revert "Revert r332955 "GNUstep Objective-C ABI version 2"".
Revert "Revert r332955 "GNUstep Objective-C ABI version 2""
May 22 2018, 3:17 AM
theraven added inline comments to rC332950: GNUstep Objective-C ABI version 2.
May 22 2018, 1:50 AM
theraven added inline comments to rC332950: GNUstep Objective-C ABI version 2.
May 22 2018, 1:38 AM
theraven added inline comments to D46052: GNUstep Objective-C ABI version 2.
May 22 2018, 1:31 AM
theraven added inline comments to rC332950: GNUstep Objective-C ABI version 2.
May 22 2018, 12:27 AM
theraven committed rL332955: Add cctype include..
Add cctype include.
May 22 2018, 12:26 AM
theraven committed rC332955: Add cctype include..
Add cctype include.
May 22 2018, 12:26 AM

May 21 2018

theraven committed rL332950: GNUstep Objective-C ABI version 2.
GNUstep Objective-C ABI version 2
May 21 2018, 11:13 PM
theraven committed rC332950: GNUstep Objective-C ABI version 2.
GNUstep Objective-C ABI version 2
May 21 2018, 11:13 PM
theraven closed D46052: GNUstep Objective-C ABI version 2.
May 21 2018, 11:13 PM

May 20 2018

theraven updated the diff for D46052: GNUstep Objective-C ABI version 2.

Address review comments, including adding a size field for ivars.

May 20 2018, 11:02 AM
theraven added inline comments to D46052: GNUstep Objective-C ABI version 2.
May 20 2018, 11:01 AM

May 1 2018

theraven added inline comments to D46052: GNUstep Objective-C ABI version 2.
May 1 2018, 1:57 AM

Apr 28 2018

theraven added inline comments to D46052: GNUstep Objective-C ABI version 2.
Apr 28 2018, 4:41 AM
theraven updated the diff for D46052: GNUstep Objective-C ABI version 2.
Apr 28 2018, 4:41 AM

Apr 26 2018

theraven updated the diff for D46052: GNUstep Objective-C ABI version 2.

Fix the symbol names that I thought I'd done previously.

Apr 26 2018, 4:20 AM
theraven updated the diff for D46052: GNUstep Objective-C ABI version 2.
  • Fixed non-use of isOptional (in all three places where it should have been used)
  • Reject -fobjc-runtime=gnustep-2.x in the driver for non-ELF targets
  • Fix a bug where the wrong variable was assigned in protocol generation, which triggered an assertion failure in one of the runtime's tests.
  • Rename protocol reference variables to prevent accidental conflicts with protocol references (this was already done for other indirection variables, but I'd missed protocols)
Apr 26 2018, 1:27 AM
theraven added a comment to D46052: GNUstep Objective-C ABI version 2.

Are you asking for a code review or a design review of the ABI?

Apr 26 2018, 1:17 AM

Apr 25 2018

theraven created D46052: GNUstep Objective-C ABI version 2.
Apr 25 2018, 5:17 AM

Apr 11 2018

theraven committed rL329882: ObjCGNU: Fix empty v3 protocols being emitted two fields short.
ObjCGNU: Fix empty v3 protocols being emitted two fields short
Apr 11 2018, 11:51 PM
theraven committed rC329882: ObjCGNU: Fix empty v3 protocols being emitted two fields short.
ObjCGNU: Fix empty v3 protocols being emitted two fields short
Apr 11 2018, 11:51 PM
theraven closed D45305: ObjCGNU: Fix empty v3 protocols being emitted two fields short.
Apr 11 2018, 11:51 PM · Restricted Project

Apr 8 2018

theraven added a comment to D45305: ObjCGNU: Fix empty v3 protocols being emitted two fields short.

I think that we emit empty method lists so that the GCC runtime doesn't choke on them, though there's no reason why we couldn't with the GNUstep ABI. It would therefore be nice if the test failed if we did change to emitting null so that we could update the test at that point and check that it's actually doing the right thing.

Apr 8 2018, 9:57 AM · Restricted Project

Apr 7 2018

theraven added a comment to D45305: ObjCGNU: Fix empty v3 protocols being emitted two fields short.

Isn't it better to test for the correct structure existing in the IR?

Apr 7 2018, 10:45 AM · Restricted Project

Apr 5 2018

theraven accepted D45305: ObjCGNU: Fix empty v3 protocols being emitted two fields short.
Apr 5 2018, 1:51 AM · Restricted Project
theraven added a comment to D45305: ObjCGNU: Fix empty v3 protocols being emitted two fields short.

Looks correct to me. Protocols are only referenced by pointer, so it doesn't matter for old versions of the runtime if these fields are present.

Apr 5 2018, 1:45 AM · Restricted Project

Mar 17 2018

theraven added a comment to D44580: Sema: allow comparison between blocks & block-compatible objc types.

We seem to be missing tests for the assignment part of this patch.

Mar 17 2018, 1:58 AM · Restricted Project

Feb 8 2018

theraven accepted D37052: Add default address space for functions to the data layout (1/3).

Looks fine to me.

Feb 8 2018, 3:21 AM

Feb 7 2018

theraven added inline comments to D42123: Derive GEP index type from Data Layout.
Feb 7 2018, 2:54 AM