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

theraven updated this revision to Diff 159312. theraven added a comment. - Fix failing test. Repository: rC Clang https://reviews.llvm.org/D50144 Files: include/clang/Driver/Options.td lib/AST/MicrosoftMangle.cpp lib/CodeGen/CGBlocks.cpp lib/CodeGen/CGException.cpp lib/CodeGen/CGOb

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

theraven added a comment. I'd like to commit this, unless @rjmccall has any objections. Repository: rC Clang https://reviews.llvm.org/D50144 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cf

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

theraven updated this revision to Diff 159651. theraven marked 2 inline comments as done. theraven added a comment. Herald added a subscriber: mgrang. - Address Dustin's review comments. - Fix an issue in protocol generation. - Fix failing test. - [gnu-objc] Make selector order deterministic. - Ad

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

theraven updated this revision to Diff 159653. theraven added a comment. - Revert blocks part of the patch to put in a separate review. Repository: rC Clang https://reviews.llvm.org/D50144 Files: include/clang/Driver/Options.td lib/AST/MicrosoftMangle.cpp lib/CodeGen/CGException.cpp

[PATCH] D50436: Correctly initialise global blocks on Windows.

theraven created this revision. theraven added a reviewer: rjmccall. Herald added a subscriber: cfe-commits. Windows does not allow globals to be initialised to point to globals in another DLL. Exported globals may be referenced only from code. Work around this by creating an initialiser that ru

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

theraven added inline comments. Comment at: lib/CodeGen/CGBlocks.cpp:1262 + if (IsWindows) { +auto *Init = llvm::Function::Create(llvm::FunctionType::get(CGM.VoidTy, + {}), llvm::GlobalValue::InternalLinkage, ".block_isa_init", rjmccall wrote: > the

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

theraven accepted this revision. theraven added a comment. This revision is now accepted and ready to land. Looks good to me. This method should probably take a StringRef rather than a `const std::string&`, but I can make that change separately. Repository: rC Clang https://reviews.llvm.org

[PATCH] D50436: Correctly initialise global blocks on Windows.

This revision was automatically updated to reflect the committed changes. Closed by commit rL339317: Correctly initialise global blocks on Windows. (authored by theraven, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D50436 Files: cfe/

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

theraven updated this revision to Diff 159868. theraven added a comment. - Address rjmcall's review comments. - Revert blocks part of the patch to put in a separate review. Repository: rC Clang https://reviews.llvm.org/D50144 Files: include/clang/Driver/Options.td lib/AST/MicrosoftMangle

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

theraven updated this revision to Diff 159887. theraven added a comment. - 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. Repository: rC Clang https://revie

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

theraven marked an inline comment as done. theraven added a comment. I believe that this is now ready to land. Comment at: lib/CodeGen/CGObjCGNU.cpp:915 +return name; + } /// The GCC ABI superclass message lookup function. Takes a pointer to a rjmccall

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

theraven updated this revision to Diff 160091. theraven added a comment. Squashed into a single commit. Repository: rC Clang https://reviews.llvm.org/D50144 Files: include/clang/Driver/Options.td lib/AST/MicrosoftMangle.cpp lib/CodeGen/CGException.cpp lib/CodeGen/CGObjCGNU.cpp lib/

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

This revision was automatically updated to reflect the committed changes. Closed by commit rC339428: Add Windows support for the GNUstep Objective-C ABI V2. (authored by theraven, committed by ). Changed prior to commit: https://reviews.llvm.org/D50144?vs=160091&id=160095#toc Repository: rC

[PATCH] D50559: [gnu-objc] Make selector order deterministic.

theraven created this revision. theraven added a reviewer: rjmccall. Herald added subscribers: cfe-commits, mgrang. This probably fixes PR35277, though there may be other sources of nondeterminism (this was the only case of iterating over a DenseMap). It's difficult to provide a test case for thi

[PATCH] D50559: [gnu-objc] Make selector order deterministic.

theraven added inline comments. Comment at: lib/CodeGen/CGObjCGNU.cpp:3547 + allSelectors.push_back(entry.first); +llvm::sort(allSelectors.begin(), allSelectors.end()); bmwiedemann wrote: > compilation failed here: > ../tools/clang/lib/CodeGen/CGObjCGN

[PATCH] D50559: [gnu-objc] Make selector order deterministic.

theraven updated this revision to Diff 160312. theraven added a comment. - Add a test case. Repository: rC Clang https://reviews.llvm.org/D50559 Files: lib/CodeGen/CGObjCGNU.cpp test/CodeGenObjC/gnu-deterministic-selectors.m Index: test/CodeGenObjC/gnu-deterministic-selectors.m ===

[PATCH] D50559: [gnu-objc] Make selector order deterministic.

This revision was automatically updated to reflect the committed changes. Closed by commit rC339668: [gnu-objc] Make selector order deterministic. (authored by theraven, committed by ). Changed prior to commit: https://reviews.llvm.org/D50559?vs=160312&id=160545#toc Repository: rC Clang htt

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

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

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

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

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

theraven added a comment. In https://reviews.llvm.org/D50144#1209758, @smeenai wrote: > 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 ru

[PATCH] D46052: GNUstep Objective-C ABI version 2

theraven marked 3 inline comments as done. theraven added inline comments. Comment at: lib/CodeGen/CGObjCGNU.cpp:439 + ArrayRef IvarOffsets, + ArrayRef IvarAlign, + ArrayRef IvarOwnership); --

[PATCH] D46052: GNUstep Objective-C ABI version 2

theraven added inline comments. Comment at: lib/CodeGen/CGObjCGNU.cpp:1056 +char c = Str[i]; +if (isalpha(c) || isnumber(c)) + StringName += c; Ka-Ka wrote: > The isnumber() function was added to cctype.h by Apple. I don't think it can >

[PATCH] D46052: GNUstep Objective-C ABI version 2

theraven added inline comments. Comment at: lib/CodeGen/CGObjCGNU.cpp:512 /// used to return an untyped selector (with the types field set to NULL). - llvm::Value *GetSelector(CodeGenFunction &CGF, Selector Sel, + virtual llvm::Value *GetSelector(CodeGenFunction &CGF, Select

[PATCH] D46052: GNUstep Objective-C ABI version 2

theraven added inline comments. Comment at: lib/CodeGen/CGObjCGNU.cpp:512 /// used to return an untyped selector (with the types field set to NULL). - llvm::Value *GetSelector(CodeGenFunction &CGF, Selector Sel, + virtual llvm::Value *GetSelector(CodeGenFunction &CGF, Select

[PATCH] D50783: [CodeGen] Merge identical block descriptor global variables

theraven added a comment. 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

[PATCH] D50783: [CodeGen] Merge identical block descriptor global variables

theraven added inline comments. Comment at: lib/CodeGen/CGBlocks.cpp:163 + std::string TypeAtEncoding = + CGM.getContext().getObjCEncodingForBlock(BlockInfo.getBlockExpr()); + Name += "e" + llvm::to_string(TypeAtEncoding.size()) + "_" + TypeAtEncoding; Spe

[PATCH] D50783: [CodeGen] Merge identical block descriptor global variables

theraven added a comment. In https://reviews.llvm.org/D50783#1221147, @ahatanak wrote: > The GNUstep documentation I found replaces '@' with '\1'. Would that fix the > problem? > > https://github.com/gnustep/libobjc2/blob/master/ABIDoc/abi.tex Yup. If this mangling is needed outside of CGObjC

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

theraven created this revision. theraven added reviewers: rjmccall, DHowett-MSFT. Herald added a subscriber: cfe-commits. Introduces funclet-based unwinding for Objective-C and fixes an issue where global blocks can't have their isa pointers initialised on Windows. After discussion with Dustin, t

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

theraven updated this revision to Diff 158997. theraven added a comment. - Address Dustin's review comments. Repository: rC Clang https://reviews.llvm.org/D50144 Files: include/clang/Driver/Options.td lib/AST/MicrosoftMangle.cpp lib/CodeGen/CGBlocks.cpp lib/CodeGen/CGException.cpp

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

theraven added inline comments. Comment at: include/clang/Driver/Options.td:1491 // Objective-C ABI options. -def fobjc_runtime_EQ : Joined<["-"], "fobjc-runtime=">, Group, Flags<[CC1Option]>, +def fobjc_runtime_EQ : Joined<["-"], "fobjc-runtime=">, Group, Flags<[CC1Option, Co

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

theraven updated this revision to Diff 159293. theraven added a comment. - Fix an issue in protocol generation. Repository: rC Clang https://reviews.llvm.org/D50144 Files: include/clang/Driver/Options.td lib/AST/MicrosoftMangle.cpp lib/CodeGen/CGBlocks.cpp lib/CodeGen/CGException.cpp

[PATCH] D44580: Sema: allow comparison between blocks & block-compatible objc types

theraven added a comment. We seem to be missing tests for the assignment part of this patch. Comment at: lib/Sema/SemaExpr.cpp:7749 +// id (or strictly compatible object type) -> T^ +if (getLangOpts().ObjC1 && RHSType->isBlockCompatibleObjCPointerType(Context)) {

[PATCH] D53547: NFC: Remove the ObjC1/ObjC2 distinction from clang (and related projects)

theraven accepted this revision. theraven added a comment. This revision is now accepted and ready to land. Looks good for me, and removing all of the code describing Objective-C 4 as ObjC1 makes me happy. Comment at: clang-tools-extra/clang-tidy/google/AvoidThrowingObjCExcep

[PATCH] D54539: [CodeGen] Replace '@' characters in block descriptors' symbol names with '\1' on ELF targets.

theraven added inline comments. Comment at: lib/CodeGen/CGObjCRuntime.h:288 + /// descriptor's symbol name. + virtual std::string getObjCEncodingForBlock(const BlockExpr *BE) const; + I'm not sure that this actually belongs in CGObjCRuntime. The runtime doesn'

[PATCH] D55544: Warning: objc-encodings-larger-than=

theraven added a comment. 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). I wonder if we also want an option in code generation to replace very long type encodings (or encodings of specifically an

[PATCH] D58724: [gnustep-objc] Make the GNUstep v2 ABI work for Windows DLLs.

theraven added a comment. This review is mostly so that @DHowett-MSFT can check that I didn't break his patches too badly in the cleanup and test that they still do allow building Objective-C DLLs on Windows. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D58724: [gnustep-objc] Make the GNUstep v2 ABI work for Windows DLLs.

theraven created this revision. theraven added a reviewer: DHowett-MSFT. Herald added a project: clang. Herald added a subscriber: cfe-commits. theraven added a comment. This review is mostly so that @DHowett-MSFT can check that I didn't break his patches too badly in the cleanup and test that th

[PATCH] D58724: [gnustep-objc] Make the GNUstep v2 ABI work for Windows DLLs.

theraven marked 2 inline comments as done. theraven added inline comments. Comment at: clang/lib/CodeGen/CGObjCGNU.cpp:188 + StringRef SymbolPrefix() { +return CGM.getTriple().isOSBinFormatCOFF() ? "_" : "._"; DHowett-MSFT wrote: > Should this be `SymbolPr

[PATCH] D58724: [gnustep-objc] Make the GNUstep v2 ABI work for Windows DLLs.

theraven marked 2 inline comments as done. theraven added inline comments. Comment at: clang/lib/CodeGen/CGObjCGNU.cpp:188 + StringRef SymbolPrefix() { +return CGM.getTriple().isOSBinFormatCOFF() ? "_" : "._"; DHowett-MSFT wrote: > theraven wrote: > > DHow

[PATCH] D58807: [CodeGen] COMDAT-fold block descriptors

theraven accepted this revision. theraven added a comment. This revision is now accepted and ready to land. Looks good to me. On ELF and Mach-O, I think we get the equivalent behaviour automatically from the ODR linkage type. I'd really like to see linkage type and COMDAT made orthogonal, but

[PATCH] D58807: [CodeGen] COMDAT-fold block descriptors

theraven added a comment. It would probably benefit from a test so that we don't regress. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58807/new/ https://reviews.llvm.org/D58807 ___ cfe-commits mailin

[PATCH] D58724: [gnustep-objc] Make the GNUstep v2 ABI work for Windows DLLs.

theraven updated this revision to Diff 15. theraven marked an inline comment as done. theraven added a comment. - Address Dustin's review comments. - [objc-gnustep] Use $ in symbol names on Windows. - Add fix from Dustin. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

[PATCH] D58724: [gnustep-objc] Make the GNUstep v2 ABI work for Windows DLLs.

theraven updated this revision to Diff 193002. theraven marked 2 inline comments as done. theraven added a comment. - Fix ownership with Twine. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58724/new/ https://reviews.llvm.org/D58724 Files: clang

[PATCH] D58807: [CodeGen] COMDAT-fold block descriptors

This revision was automatically updated to reflect the committed changes. Closed by commit rL357363: COMDAT-fold block descriptors. (authored by theraven, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D58

[PATCH] D58724: [gnustep-objc] Make the GNUstep v2 ABI work for Windows DLLs.

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 rC357364: [gnustep-objc] Make the GNUstep v2 ABI work for Windows DLLs. (authored by theraven, committed by ). Changed prio

[PATCH] D61634: [clang/llvm] Allow efficient implementation of libc's memory functions in C/C++

theraven added a comment. I wonder if a list of comma-separated names is the right approach. Would it make more sense to add a new attribute for each of the helpers, such as `#no-runtime-for-memcpy`? That should make querying simpler (one lookup in the table, rather than a lookup and a string

[PATCH] D52344: [Clang][CodeGen][ObjC]: Fix non-bridged CoreFoundation builds on ELF targets that use `-fconstant-cfstrings`.

theraven added a comment. In https://reviews.llvm.org/D52344#1242530, @rjmccall wrote: > In https://reviews.llvm.org/D52344#1242451, @kristina wrote: > > > 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 entire

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

theraven added a comment. In https://reviews.llvm.org/D47233#1129810, @DHowett-MSFT wrote: > 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

[PATCH] D52581: [AST] Revert mangling changes from r339428

theraven added a comment. > 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. I'm not sure I understand this comment. The compiler code is in LLVM and the t

[PATCH] D52581: [AST] Revert mangling changes from r339428

theraven added a comment. In https://reviews.llvm.org/D52581#1248306, @smeenai wrote: > The simplest option is something like https://reviews.llvm.org/P8109, where > we add a `.objc` discriminator when mangling the RTTI itself. It would > require the GNUStep runtime for Windows to be altered ac

[PATCH] D55802: Change CGObjC to use objc intrinsics instead of runtime methods

theraven added a comment. Herald added a project: LLVM. After some bisection, it appears that this is the revision that introduced the regression in the GNUstep Objective-C runtime test suite that I reported on the list a few weeks ago. In this is the test (compiled with `-fobjc-runtime=gnuste

[PATCH] D64493: [Driver] Don't pass --dynamic-linker to ld on Solaris

theraven added a comment. I think this was simply mirroring what gcc 4.something did on Solaris. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64493/new/ https://reviews.llvm.org/D64493 ___ cfe-commits mailing list c

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

theraven requested changes to this revision. theraven added inline comments. This revision now requires changes to proceed. Comment at: lib/AST/ASTContext.cpp:6973 QualType PointeeTy = OPT->getPointeeType(); -if (!Options.EncodingProperty() && +if (getLangOpts().ObjC

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

theraven added inline comments. Comment at: lib/AST/ASTContext.cpp:6973 QualType PointeeTy = OPT->getPointeeType(); -if (!Options.EncodingProperty() && +if (getLangOpts().ObjCRuntime.isGNUFamily() && +!Options.EncodingProperty() && ahatanak w

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

theraven accepted this revision. theraven added a comment. This revision is now accepted and ready to land. LGTM. I wonder if we have any other ugly GCC bug compatibility parts in clang's Objective-C implementation... Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D55229: [COFF] Statically link certain runtime library functions

theraven added a comment. 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. Comment at: CodeGenObjC/gnu-init.m:103 +// Make sure we do not have dllimport on th

[PATCH] D55640: [clang-tidy] Implement a check for large Objective-C type encodings 🔍

theraven added a comment. 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

[PATCH] D55869: Convert some ObjC retain/release msgSends to runtime calls

theraven added a comment. This should be fine for the GNUstep runtime (the GCC runtime doesn't support ARC at all). My main concern is that it will break already-released versions of the runtime built with a newer version of clang. I can easily enable a new flag in the next release, but doing

[PATCH] D54539: [CodeGen] Replace '@' characters in block descriptors' symbol names with '\1' on ELF targets.

theraven added a comment. Please can we either merge this or revert the original change that introduced the bug that this is fixing? It's now been 6 months since blocks generated invalid ELF symbols. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54539/new/ htt

[PATCH] D54539: [CodeGen] Replace '@' characters in block descriptors' symbol names with '\1' on ELF targets.

theraven accepted this revision. theraven added a comment. This revision is now accepted and ready to land. Looks like a much cleaner change. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54539/new/ https://reviews.llvm.org/D54539 _

[PATCH] D68108: Redeclare Objective-C property accessors inside the ObjCImplDecl in which they are synthesized.

theraven added a comment. The changes for the GNU runtimes look correct to me. We're missing a bunch of tests there, unfortunately. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68108/new/ https://reviews.llvm.org/D68108 ___ cfe-commits m

[PATCH] D45305: ObjCGNU: Fix empty v3 protocols being emitted two fields short

This revision was automatically updated to reflect the committed changes. Closed by commit rC329882: ObjCGNU: Fix empty v3 protocols being emitted two fields short (authored by theraven, committed by ). Repository: rC Clang https://reviews.llvm.org/D45305 Files: lib/CodeGen/CGObjCGNU.cpp

[PATCH] D46052: GNUstep Objective-C ABI version 2

theraven added a comment. In https://reviews.llvm.org/D46052#1078597, @rjmccall wrote: > Are you asking for a code review or a design review of the ABI? I don't think a design review is appropriate here, I am asking for a code review. > The second would be much easier to do from a design docu

[PATCH] D46052: GNUstep Objective-C ABI version 2

theraven added inline comments. Comment at: lib/CodeGen/CGObjCGNU.cpp:977 +if ((CGM.getTarget().getPointerWidth(0) == 64) && +(SL->getLength() < 9) && !isNonASCII) { + // Tiny strings are (roughly): rjmccall wrote: > Please hoist `SL->getLength()

[PATCH] D46052: GNUstep Objective-C ABI version 2

theraven added inline comments. Comment at: lib/CodeGen/CGObjCGNU.cpp:439 + ArrayRef IvarOffsets, + ArrayRef IvarAlign, + ArrayRef IvarOwnership); DHowett-MSFT wrote: > While we'r

[PATCH] D10480: Implement shared_mutex re: N4508

theraven added a comment. 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

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

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 rGd157a9bc8ba1: Add Windows Control Flow Guard checks (/guard:cf). (authored by ajpaverd, committed by theraven). Repositor

[PATCH] D80225: [Driver] Recognize -fuse-ld={bfd, gold, lld} but don't prepend "ld." or "ld64." for other values

theraven added a comment. I'm a bit nervous about this. I'm aware of at least one out-of-tree toolchain that uses this mechanism to select their proprietary linker. I'd expect an RFC and for this to be highlighted in LLVM Weekly before I'm happy that this won't break downstream consumers. R

[PATCH] D69991: Implement __attribute__((objc_direct)), __attribute__((objc_direct_members))

theraven added a comment. Herald added a reviewer: aaron.ballman. Herald added a reviewer: jdoerfert. Sorry for getting to this late, I assumed it was a runtime-agnostic feature until someone filed a bug against the GNUstep runtime saying that we didn't implement it. It would be polite to tag m

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

theraven added a comment. This feature looks generally useful. A few small suggestions: - This is really a way of transforming a formal protocol into an informal protocol. Objective-C has had a convention of informal protocols since the '80s, but they're implemented as categories on the root

[PATCH] D123898: Fix crash in ObjC codegen introduced with 5ab6ee75994d645725264e757d67bbb1c96fb2b6

theraven created this revision. theraven added reviewers: rjmccall, triplef. Herald added a project: All. theraven requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. 5ab6ee75994d645725264e757d67bbb1c96fb2b6

[PATCH] D123898: Fix crash in ObjC codegen introduced with 5ab6ee75994d645725264e757d67bbb1c96fb2b6

theraven added a comment. I'd like to. The test case from the original bug report was very large but I'm not sure how to trigger this with anything comprehensibly small. You need something where the callee is responsible for destroying the argument and I'm not sure what combination of propert

[PATCH] D123898: Fix crash in ObjC codegen introduced with 5ab6ee75994d645725264e757d67bbb1c96fb2b6

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG94c3b169785c: Fix crash in ObjC codegen introduced with… (authored by theraven). Changed prior to commit: https://reviews.llvm.org/D123898?vs=4232

[PATCH] D128190: [WinEH] Apply funclet operand bundles to nounwind intrinsics that lower to function calls

theraven accepted this revision. theraven added a comment. This revision is now accepted and ready to land. LGTM, a couple of extra comments would help. Comment at: llvm/include/llvm/IR/IntrinsicInst.h:110 + // Check if this intrinsic might lower into a regular function call

[PATCH] D72167: Add support for __declspec(guard(nocf))

This revision was automatically updated to reflect the committed changes. Closed by commit rGbdd88b7ed395: Add support for __declspec(guard(nocf)) (authored by ajpaverd, committed by theraven). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72167/new/

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

theraven accepted this revision. theraven added a comment. This revision is now accepted and ready to land. Looks good to me, thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75574/new/ https://reviews.llvm.org/D75574 _

[PATCH] D91874: [GNU ObjC] Fix a regression listing methods twice.

theraven created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. theraven requested review of this revision. Methods synthesized from declared properties were being added to the method lists twice. This came from the change to list them in the class's method

[PATCH] D91874: [GNU ObjC] Fix a regression listing methods twice.

theraven added a comment. This was caught with the GNUstep runtime's test suite, which apparently had not been run with anything newer than clang 8 until recently. With this patch, all of the runtime's tests now pass again (a few others failed in 10 but appear to have been fixed in 11 or 12).

[PATCH] D91874: [GNU ObjC] Fix a regression listing methods twice.

theraven added a comment. I'd like to get this into the 11 point release, so it would be good to have a review... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91874/new/ https://reviews.llvm.org/D91874 ___

[PATCH] D91874: [GNU ObjC] Fix a regression listing methods twice.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd1ed67037de6: [GNU ObjC] Fix a regression listing methods twice. (authored by theraven). Changed prior to commit: https://reviews.llvm.org/D91874?

[PATCH] D96816: [ObjC] Encode pointers to C++ classes as "^v" if the encoded string would otherwise include template specialization types

theraven requested changes to this revision. theraven added a comment. This revision now requires changes to proceed. I'm broadly in favour of this change, but with the GNUstep ABIs this is an ABI-breaking change and so should not be on by default. The type encoding leaks into the ABI in two wa

[PATCH] D96816: [ObjC] Encode pointers to C++ classes as "^v" if the encoded string would otherwise include template specialization types

theraven added inline comments. Comment at: clang/include/clang/Driver/Options.td:2113 +def fno_objc_encode_cxx_class_template_spec : + Flag<["-"], "fno-objc-encode-cxx-class-template-spec">, Group; defm objc_convert_messages_to_runtime_calls : BoolFOption<"objc-convert-messag

[PATCH] D96816: [ObjC] Encode pointers to C++ classes as "^v" if the encoded string would otherwise include template specialization types

theraven accepted this revision. theraven added a comment. This revision is now accepted and ready to land. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96816/new/ https://reviews.llvm.org/D96816 __

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

theraven added a comment. The error message here is very confusing: /home/theraven/snmalloc2/src/mem/../ds/../aal/../ds/defines.h:122:27: error: cannot perform a tail call to function 'error' because its signature is incompatible with the calling function [[clang::musttail]] return sn

[PATCH] D105142: RFC: Implementing new mechanism for hard register operands to inline asm as a constraint.

theraven added a comment. The code looks fine but it would be good to see some docs along with it. We're currently missing docs on inline assembly entirely and the GCC ones are somewhat... opaque when it comes to describing how constraints work. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

theraven added a comment. Here's a minimal test: void tail(int, float); __attribute__((always_inline)) void caller(float x) { [[clang::musttail]] return tail(42, x); } void outer(int x, float y) { return caller(y); } This raises this error: tail.cc:7:3:

[PATCH] D105142: RFC: Implementing new mechanism for hard register operands to inline asm as a constraint.

theraven added a comment. In D105142#2850247 , @anirudhp wrote: > In D105142#2849835 , @theraven > wrote: > >> The code looks fine but it would be good to see some docs along with it. >> We're currently missing

[PATCH] D112400: [clang][compiler-rt][atomics] Add `__c11_atomic_fetch_nand` builtin and support `__atomic_fetch_nand` libcall

theraven accepted this revision. theraven added a comment. This revision is now accepted and ready to land. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112400/new/ https://reviews.llvm.org/D112400

[PATCH] D157962: [GNU ObjC] Unconditionally emit section markers.

theraven created this revision. Herald added a project: All. theraven requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. In the GNUstep v2 ABI (on ELF), we rely on the linker-inserted section start and stop markers. These only exist when bina

[PATCH] D157962: [GNU ObjC] Unconditionally emit section markers.

theraven updated this revision to Diff 550295. theraven added a comment. Update test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157962/new/ https://reviews.llvm.org/D157962 Files: clang/lib/CodeGen/CGObjCGNU.cpp clang/test/CodeGenObjC/gnu-

[PATCH] D135273: [Clang][ObjC] Add optionality to property attribute strings.

theraven added a comment. In D135273#3936297 , @al45tair wrote: > @theraven Any chance you could glance over this and reassure us that it isn't > going to break the GNU runtime if we do this? (We're adding an extra > attribute in the property attribute

[PATCH] D71499: Add builtins for aligning and checking alignment of pointers and integers

theraven added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:14323 +Result = Builder.CreateIntrinsic( +Intrinsic::ptrmask, {Args.SrcType, SrcForMask->getType(), Args.IntType}, +{SrcForMask, NegatedMask}, nullptr, "aligned_result");

[PATCH] D45305: ObjCGNU: Fix empty v3 protocols being emitted two fields short

theraven added a comment. 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. Repository: rC Clang https://reviews.llvm.org/D45305 ___ cfe-commits mail

[PATCH] D45305: ObjCGNU: Fix empty v3 protocols being emitted two fields short

theraven added a comment. Isn't it better to test for the correct structure existing in the IR? Repository: rC Clang https://reviews.llvm.org/D45305 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/lis

[PATCH] D45305: ObjCGNU: Fix empty v3 protocols being emitted two fields short

theraven added a comment. 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 th

[PATCH] D39365: [libunwind] Change unw_word_t to always have the same size as the pointer size

theraven added a comment. This makes things worse for us. On CHERI, `[u]intptr_t` is a (`typedef` for a) built-in type that can hold a capability. Having `unw_word_t` be `uintptr_t` made LLVM's libunwind considerably easier to port than the HP unwinder would have been, because `uintptr_t` is

[PATCH] D39365: [libunwind] Change unw_word_t to always have the same size as the pointer size

theraven added a comment. In https://reviews.llvm.org/D39365#910622, @mstorsjo wrote: > In https://reviews.llvm.org/D39365#910590, @theraven wrote: > > > This makes things worse for us. On CHERI, `[u]intptr_t` is a (`typedef` > > for a) built-in type that can hold a capability. Having `unw_wor

[PATCH] D30025: [compiler-rt] [builtins] Fix building atomic.c with GCC

theraven added a comment. This code is working around something that's probably a clang bug. It would be better to fix the clang bug than add more complex workarounds. Repository: rL LLVM https://reviews.llvm.org/D30025 ___ cfe-commits mailing

[PATCH] D30025: [compiler-rt] [builtins] Fix building atomic.c with GCC

theraven added a comment. No, it's a bug in clang. Clang does not reject other functions that are used to implement builtins (if it did, compiler-rt would be a lot more difficult to build). Repository: rL LLVM https://reviews.llvm.org/D30025 _

[PATCH] D111792: [ObjC-GNUstep] Fix ivars for dll{im,ex}ported classes.

theraven created this revision. theraven requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. If a class is dllexported or imported then the ivar offset variables must be as well. Repository: rG LLVM Github Monorepo https://reviews.llvm.org

  1   2   >