In D55869#1337723, @js wrote:

In D55869#1337711, @dexonsmith wrote:

In D55869#1337706, @js wrote:

The ObjFW runtime itself does not contain anything about release, retain or autorelease - these are all part of ObjFW (as in the framework). As ObjFW also supports the Apple runtime, as well as mixing with Foundation code, it so far only provides objc_retain and objc_release when they are missing, to make ARC work. They just call into the corresponding methods on the object and do nothing else.

How will this change work from the Apple runtime? Will objc_retain/objc_release call the retain/release on the object if it implements its own? Should I drop my own retain/release implementation from my root object if I am compiling against the Apple runtime?

Yes, the idea is that the specialized runtime functions are fast paths for the common case, but they still fall back if the object has overridden them.

I am guessing not just overridden, but also a root class providing one? IOW, the fast path is used when the class does not have the retain or release method at all? In that case, LGTM as is.

Dec 20 2018, 9:33 AM

Dec 19 2018

pete updated the diff for D55869: Convert some ObjC retain/release msgSends to runtime calls.

Dec 19 2018, 10:58 PM

pete added a comment to D55869: Convert some ObjC retain/release msgSends to runtime calls.

In D55869#1335783, @rjmccall wrote:

So, once upon a time this was a problem because we were rewriting the method calls in the runtime itself. Can you explain how that's not a problem now? Do we just expect the runtime to be compiled with the right switch?

Dec 19 2018, 10:57 PM

Dec 18 2018

pete created D55869: Convert some ObjC retain/release msgSends to runtime calls.

Dec 18 2018, 4:52 PM

pete committed rL349559: Preserve the linkage for objc* intrinsics as clang will set them to….

Preserve the linkage for objc* intrinsics as clang will set them to…

Dec 18 2018, 2:45 PM

pete committed rL349558: Add nonlazybind to objc_retain/objc_release when converting from intrinsics..

Add nonlazybind to objc_retain/objc_release when converting from intrinsics.

Dec 18 2018, 2:34 PM

pete committed rL349552: Rewrite objc intrinsics to runtime methods in PreISelIntrinsicLowering instead….

Rewrite objc intrinsics to runtime methods in PreISelIntrinsicLowering instead…

Dec 18 2018, 2:23 PM

pete committed rC349535: Generate objc intrinsics instead of runtime calls as the ARC optimizer now….

Generate objc intrinsics instead of runtime calls as the ARC optimizer now…

Dec 18 2018, 12:36 PM

pete committed rL349535: Generate objc intrinsics instead of runtime calls as the ARC optimizer now….

Generate objc intrinsics instead of runtime calls as the ARC optimizer now…

Dec 18 2018, 12:36 PM

pete committed rL349534: Change the objc ARC optimizer to use the new objc.* intrinsics.

Change the objc ARC optimizer to use the new objc.* intrinsics

Dec 18 2018, 12:36 PM

pete closed D55802: Change CGObjC to use objc intrinsics instead of runtime methods.

Dec 18 2018, 12:36 PM · Restricted Project

pete closed D55348: Change the objc ARC optimizer to use the new objc.* intrinsics.

Dec 18 2018, 12:36 PM

Dec 17 2018

pete added a comment to D55802: Change CGObjC to use objc intrinsics instead of runtime methods.

In D55802#1334008, @rjmccall wrote:

You're making intrinsics with weak_external linkage? I feel like that's going to be unnecessarily awkward in the future, but okay.

Dec 17 2018, 9:45 PM · Restricted Project

pete created D55802: Change CGObjC to use objc intrinsics instead of runtime methods.

Dec 17 2018, 6:12 PM · Restricted Project

pete added inline comments to D55348: Change the objc ARC optimizer to use the new objc.* intrinsics.

Dec 17 2018, 5:57 PM

Dec 7 2018

pete added a comment to D55349: Convert some ObjC alloc msgSends to runtime calls.

In D55349#1323926, @rjmccall wrote:

LGTM.

Dec 7 2018, 9:24 PM

pete committed rL348687: Convert some ObjC msgSends to runtime calls..

Convert some ObjC msgSends to runtime calls.

Dec 7 2018, 9:17 PM

pete committed rC348687: Convert some ObjC msgSends to runtime calls..

Convert some ObjC msgSends to runtime calls.

Dec 7 2018, 9:17 PM

pete updated the diff for D55349: Convert some ObjC alloc msgSends to runtime calls.

Added test for integer argument and updated code to only accept pointer types to allocWithZone.

Dec 7 2018, 2:01 PM

pete updated the diff for D55348: Change the objc ARC optimizer to use the new objc.* intrinsics.

I added the other objc* intrinsics used by ARC in r348646.

Dec 7 2018, 1:35 PM

pete committed rL348646: Follow-up from r348441 to add the rest of the objc ARC intrinsics..

Follow-up from r348441 to add the rest of the objc ARC intrinsics.

Dec 7 2018, 1:32 PM

pete added inline comments to D55349: Convert some ObjC alloc msgSends to runtime calls.

Dec 7 2018, 10:13 AM

pete updated the diff for D55349: Convert some ObjC alloc msgSends to runtime calls.

Thanks for the review. Have fixed the comments as suggested.

Dec 7 2018, 10:12 AM

pete accepted D55442: Update the Swift version numbers reported by objdump.

LGTM

Dec 7 2018, 10:03 AM

Dec 6 2018

pete added inline comments to D55348: Change the objc ARC optimizer to use the new objc.* intrinsics.

Dec 6 2018, 10:22 AM

Dec 5 2018

pete created D55349: Convert some ObjC alloc msgSends to runtime calls.

Dec 5 2018, 6:07 PM

pete created D55348: Change the objc ARC optimizer to use the new objc.* intrinsics.

Dec 5 2018, 5:26 PM

pete committed rL348441: Add objc.* ARC intrinsics and codegen them to their runtime methods..

Add objc.* ARC intrinsics and codegen them to their runtime methods.

Dec 5 2018, 4:56 PM

pete closed D55233: Add objc_retain and objc_release intrinsics and codegen them to their runtime methods.

Dec 5 2018, 4:55 PM

pete committed rC348431: Fix title underlines being too short after r348429.

Fix title underlines being too short after r348429

Dec 5 2018, 4:05 PM

pete committed rL348431: Fix title underlines being too short after r348429.

Fix title underlines being too short after r348429

Dec 5 2018, 4:05 PM

pete committed rL348429: Update ARC docs as objc_storeStrong returns void not id.

Update ARC docs as objc_storeStrong returns void not id

Dec 5 2018, 3:53 PM

pete committed rC348429: Update ARC docs as objc_storeStrong returns void not id.

Update ARC docs as objc_storeStrong returns void not id

Dec 5 2018, 3:52 PM

pete added a comment to D55233: Add objc_retain and objc_release intrinsics and codegen them to their runtime methods.

Discovered that objc_storeStrong is actually wrong in the documentation. Changing it from returning id to returning void. Will also update the docs.

Dec 5 2018, 9:06 AM

Dec 4 2018

pete added a comment to D55233: Add objc_retain and objc_release intrinsics and codegen them to their runtime methods.

In D55233#1318897, @ahatanak wrote:

LGTM

Dec 4 2018, 11:23 AM

Dec 3 2018

pete updated the diff for D55233: Add objc_retain and objc_release intrinsics and codegen them to their runtime methods.

Updated to add a comment to Intrinsics.td, remove the ReturnedValue<> and added all of the ARC methods from https://clang.llvm.org/docs/AutomaticReferenceCounting.html#runtime-support

Dec 3 2018, 6:34 PM

pete created D55233: Add objc_retain and objc_release intrinsics and codegen them to their runtime methods.

Dec 3 2018, 1:04 PM

Jul 2 2018

pete accepted D48517: [MC] Error on a .zerofill directive in a non-virtual section.

Sorry for the delay. LGTM.

Jul 2 2018, 10:02 AM

May 31 2018

pete accepted D47593: [MC] Fallback on DWARF when generating compact unwind on AArch64.

Nice catch. This LGTM.

May 31 2018, 9:22 AM

Apr 23 2018

pete added a comment to D30882: Add a callback for __has_include and use it for dependency scanning.

In D30882#1075461, @dexonsmith wrote:

In D30882#1075407, @ddunbar wrote:

In D30882#1074822, @dexonsmith wrote:

I don't think this is quite right. I know at least make-based incremental builds wouldn't deal well with this.

This is actually not a novel problem w.r.t. this patch. The exact same situation comes up with Makefile-included .d files and when one of the referenced headers is removed.

This is typically solved somewhere in the build system, for example Make has -include, and Ninja and llbuild have explicit support for this situation.

Of course, yes, I was wrong. Thanks for the correction.

I agree we might want to tread cautiously on how we change the .d output, but in this case I think it might be safe.

If we decide this isn't safe, then we may want to consider a new flag which tracks *all* anti-dependencies (file's for which Clang checked existence but did not exist), and include that here. The major concern with that approach is it is a much larger list, and while it would support being substantially more correct, it is also well beyond what people currently expect out of the build system + compiler generated deps approaches.

Even though this is likely safe, it seems really noisy. Consider the case where someone has __has_include(<missing>) -- we'll get an entry for every -I, -F, -isystem, and -iframework. If we're going to add that noise, I prefer a separate flag that covers all anti-dependencies.

Apr 23 2018, 9:50 AM

Mar 9 2018

pete accepted D44270: Improve caching scheme in ProvenanceAnalysis..

This all looks reasonable. Its a nice localized change to this pass. LGTM.

Mar 9 2018, 4:03 PM

May 10 2017

pete accepted D33057: [ConstantRange] Fix the early out in ConstantRange::multiply for positive numbers to really do what the comment says.

Good catch! LGTM.

May 10 2017, 12:25 PM

May 3 2017

pete accepted D32811: [IR] Abstract away ArgNo+1 attribute indexing as much as possible.

This is a great improvement over what we had. LGTM.

May 3 2017, 11:06 AM

Apr 10 2017

pete accepted D31198: [IR] Make AttributeSetNode public, avoid temporary AttributeList copies.

Sorry for forgetting to review this.

Apr 10 2017, 1:10 PM

Mar 20 2017

pete accepted D31102: Rename AttributeSet to AttributeList.

I scrolled through fairly quickly, but everything looked fine. Looks like basically renaming and fixing formatting along the way.

Mar 20 2017, 11:26 PM

Mar 13 2017

pete created D30882: Add a callback for __has_include and use it for dependency scanning.

Mar 13 2017, 1:01 AM

pete created D30881: Track skipped files in dependency scanning.

Mar 13 2017, 12:10 AM

Mar 8 2017

pete added a comment to D30723: Add red zones to BumpPtrAllocator under ASan.

In D30723#695798, @jordan_rose wrote:

Thanks, Pete! Will land soon, then.

I think we could support SpecificBumpPtrAllocator but it would require walking the objects by 'sizeof(T) + RedZoneSize'. RedZoneSize itself would also have to be set to make sure we continue to have appropriate alignment. Not something you need to do now, but could be interesting in future.

FWIW, this doesn't work as is because SpecificBumpPtrAllocator lets you allocate *arrays* of objects as well as single objects, and the arrays obviously can't have red zones between elements.

Good point. Never thought of that. Wouldn't be surprised if there's a whole bunch of users who never use arrays and could benefit, but still something to think about more.

Mar 8 2017, 11:48 AM

pete accepted D30723: Add red zones to BumpPtrAllocator under ASan.

Very cool. Justin and I looked at ASan on BumpPtrAllocator a while ago and I think this would have been useful then too. I think the reason you didn't find issues is that he fixed many of them already.

Mar 8 2017, 11:34 AM