erik.pilkington (Erik Pilkington)
User

Projects

User does not belong to any projects.

User Details

User Since
Feb 2 2016, 10:56 AM (80 w, 5 d)

Recent Activity

Thu, Aug 17

erik.pilkington updated the diff for D36777: [Sema] Don't emit -Wunguarded-availability for switch cases.

Ah, good point. This new patch does that.
Thanks,
Erik

Thu, Aug 17, 9:54 AM

Wed, Aug 16

erik.pilkington added a comment to D36492: [time-report] Add preprocessor timer.

This looks really usefull, thanks for working on this!

Wed, Aug 16, 1:43 PM

Tue, Aug 15

erik.pilkington created D36777: [Sema] Don't emit -Wunguarded-availability for switch cases.
Tue, Aug 15, 5:21 PM

Mon, Aug 14

erik.pilkington added a comment to D36200: [Sema] Improve some -Wunguarded-availability diagnostic messages.

Ping!

Mon, Aug 14, 9:50 AM

Tue, Aug 8

erik.pilkington added inline comments to D36427: [libcxxabi][demangler] Improve representation of substitutions/templates.
Tue, Aug 8, 2:40 PM
erik.pilkington updated the diff for D36427: [libcxxabi][demangler] Improve representation of substitutions/templates.

Address review comments.
Thanks!
Erik

Tue, Aug 8, 2:40 PM

Mon, Aug 7

erik.pilkington created D36427: [libcxxabi][demangler] Improve representation of substitutions/templates.
Mon, Aug 7, 1:44 PM

Wed, Aug 2

erik.pilkington added a comment to D36200: [Sema] Improve some -Wunguarded-availability diagnostic messages.

This needs a test for the fixits as well, see test/FixIt/fixit-availability*

Wed, Aug 2, 9:46 AM

Tue, Aug 1

erik.pilkington created D36200: [Sema] Improve some -Wunguarded-availability diagnostic messages.
Tue, Aug 1, 4:31 PM
erik.pilkington created D36191: [CodeGen] Don't make availability attributes imply default visibility on macos.
Tue, Aug 1, 2:21 PM

Thu, Jul 27

erik.pilkington updated the diff for D35159: [libcxxabi][demangler] Use an AST to represent the demangled name.

Use LLVM naming conventions for all the new stuff in this patch.

Thu, Jul 27, 2:04 PM

Mon, Jul 24

erik.pilkington accepted D35726: unguarded availability: add a fixit for the "annotate '...' with an availability attribute to silence" note.

LGTM, thanks for working on this!

Mon, Jul 24, 9:24 AM

Sun, Jul 23

erik.pilkington planned changes to D35781: [Sema] Make sure that -Wunguarded-availability emits notes at the right redeclaration.

On second thought, I think it makes more sense to do this right before we emit a diagnostic so we have the most recent redecl with all the inherited availability attributes until then. I'll put a diff up for that tomorrow!

Sun, Jul 23, 9:04 PM
erik.pilkington created D35781: [Sema] Make sure that -Wunguarded-availability emits notes at the right redeclaration.
Sun, Jul 23, 2:04 PM

Jul 21 2017

erik.pilkington added a comment to D35726: unguarded availability: add a fixit for the "annotate '...' with an availability attribute to silence" note.

Thanks for working on this! This looks like it would be very useful.

Jul 21 2017, 9:39 AM

Jul 17 2017

erik.pilkington updated the diff for D35159: [libcxxabi][demangler] Use an AST to represent the demangled name.

s/string_ref/StringView

Jul 17 2017, 3:34 PM

Jul 14 2017

erik.pilkington updated the diff for D35159: [libcxxabi][demangler] Use an AST to represent the demangled name.

Rebase. I don't think the issue of purging underscores from this file/libcxx should block this, if we want to discuss that cfe-dev would probably be the place. I agree that it would be nice to clang-format this file, would anyone have any problems with me committing that after this? I found the git-blame of this file to be pretty useless, almost every line just points to r184097.
Are there any other thoughts on this change?
Thanks,
Erik

Jul 14 2017, 9:44 AM

Jul 13 2017

erik.pilkington added inline comments to D35379: Add documentation for @available.
Jul 13 2017, 1:47 PM
erik.pilkington added a reviewer for D35379: Add documentation for @available: arphaman.

This looks great, thanks for working on this! LGTM, but @arphaman might have some thoughts.

Jul 13 2017, 1:31 PM

Jul 12 2017

erik.pilkington added a comment to D35159: [libcxxabi][demangler] Use an AST to represent the demangled name.

Looks like this demangler's design is similar to my demangler for Microsoft name mangling scheme (https://reviews.llvm.org/D34667). Is that a coincidence? Both demanglers create AST, stringize it using print_left/print_right (I named them write_pre/write_post), and use custom memory allocator. Looks like both demangler can share more code once both land.

Jul 12 2017, 2:04 PM

Jul 9 2017

erik.pilkington closed D35158: [libcxxabi][demangler] NFC: Don't make everything a template.

Landed as r307482 & r307481, thanks! (for some reason phab wasn't automatically closing this)

Jul 9 2017, 6:14 AM
erik.pilkington added inline comments to D35159: [libcxxabi][demangler] Use an AST to represent the demangled name.
Jul 9 2017, 6:14 AM
erik.pilkington updated the diff for D35159: [libcxxabi][demangler] Use an AST to represent the demangled name.

In this new patch:

  • Fix UB @EricWF pointed out; memmove(something, nullptr, 0) was being called.
  • Add some comments
  • rename stream -> output_stream
Jul 9 2017, 6:14 AM
erik.pilkington updated the diff for D35159: [libcxxabi][demangler] Use an AST to represent the demangled name.

Forgot to add context lines!

Jul 9 2017, 6:13 AM
erik.pilkington created D35159: [libcxxabi][demangler] Use an AST to represent the demangled name.
Jul 9 2017, 6:13 AM
erik.pilkington created D35158: [libcxxabi][demangler] NFC: Don't make everything a template.
Jul 9 2017, 6:13 AM

Jul 6 2017

erik.pilkington accepted D35061: [ObjC] Avoid the -Wunguarded-availability warnings for protocol requirements in protocol/class/category declarations.

LGTM, thanks!

Jul 6 2017, 10:39 AM
erik.pilkington added inline comments to D35061: [ObjC] Avoid the -Wunguarded-availability warnings for protocol requirements in protocol/class/category declarations.
Jul 6 2017, 10:09 AM
erik.pilkington added inline comments to D35061: [ObjC] Avoid the -Wunguarded-availability warnings for protocol requirements in protocol/class/category declarations.
Jul 6 2017, 9:46 AM

Jun 28 2017

erik.pilkington updated the diff for D33816: [Sema][ObjC] Don't allow -Wunguarded-availability to be silenced with redeclarations.

Improve diagnostics for unnamed types.

Jun 28 2017, 9:20 AM

Jun 26 2017

erik.pilkington updated the diff for D33816: [Sema][ObjC] Don't allow -Wunguarded-availability to be silenced with redeclarations.

Make the diagnostic reference the context declaration instead of the offending decl if no enclosing decl is found, fixing the diagnostic bug @arphaman pointed out.

Jun 26 2017, 8:49 AM

Jun 24 2017

erik.pilkington updated the diff for D33816: [Sema][ObjC] Don't allow -Wunguarded-availability to be silenced with redeclarations.

Improve enum diagnostics, as @arphaman suggested. This causes a bit of churn throughout the availability diagnostic machinery, if it would make it at all easier to review, I would be happy to separate out these changes.
Thanks,
Erik

Jun 24 2017, 3:32 PM

Jun 23 2017

erik.pilkington added inline comments to D34556: [libcxx] Annotate c++17 aligned new/delete operators with availability attribute.
Jun 23 2017, 8:20 AM

Jun 22 2017

erik.pilkington accepted D33606: [Sema] Fix a crash-on-invalid when a template parameter list has a class definition or non-reference class type.

LGTM!

Jun 22 2017, 5:37 PM
erik.pilkington accepted D34264: Introduce -Wunguarded-availability-new, which is like -Wunguarded-availability, except that it's enabled by default for new deployment targets.

LGTM, thanks for working on this!

Jun 22 2017, 8:09 AM

Jun 19 2017

erik.pilkington added inline comments to D34264: Introduce -Wunguarded-availability-new, which is like -Wunguarded-availability, except that it's enabled by default for new deployment targets.
Jun 19 2017, 9:30 AM

Jun 16 2017

erik.pilkington added inline comments to D34264: Introduce -Wunguarded-availability-new, which is like -Wunguarded-availability, except that it's enabled by default for new deployment targets.
Jun 16 2017, 3:57 PM

Jun 15 2017

erik.pilkington added a comment to D33816: [Sema][ObjC] Don't allow -Wunguarded-availability to be silenced with redeclarations.

Ping!

Jun 15 2017, 6:18 PM
erik.pilkington added inline comments to D34264: Introduce -Wunguarded-availability-new, which is like -Wunguarded-availability, except that it's enabled by default for new deployment targets.
Jun 15 2017, 6:16 PM

Jun 13 2017

erik.pilkington added a comment to D33977: [libcxx][WIP] Experimental support for a scheduler for use in the parallel stl algorithms.

Ping!

Jun 13 2017, 9:05 AM

Jun 11 2017

erik.pilkington created D34096: [Sema][C++1z] Ensure structured binding's bindings in dependent foreach have non-null type.
Jun 11 2017, 6:31 PM

Jun 6 2017

erik.pilkington created D33977: [libcxx][WIP] Experimental support for a scheduler for use in the parallel stl algorithms.
Jun 6 2017, 10:55 PM

Jun 1 2017

erik.pilkington created D33816: [Sema][ObjC] Don't allow -Wunguarded-availability to be silenced with redeclarations.
Jun 1 2017, 6:14 PM

May 29 2017

erik.pilkington created D33661: [Sema][ObjC] Don't emit availability diagnostics for categories extending unavailable interfaces.
May 29 2017, 5:39 PM

May 28 2017

erik.pilkington edited reviewers for D33637: [libcxxabi][demangler] Fix a exponential string copying bug, added: compnerd; removed: EricWF.
May 28 2017, 2:28 PM
erik.pilkington created D33637: [libcxxabi][demangler] Fix a exponential string copying bug.
May 28 2017, 2:27 PM

May 24 2017

erik.pilkington added a comment to D33368: [libcxxabi][demangler] Fix a crash in the demangler.

Also, are you now maintaining this code?
I am trying to find someone who wants to be CC-ed to other demangler bugs automatically reported by oss-fuzz.

I don’t think I’ll accept the title of maintainer, (I only have one commit in this file!) but I have some plans to work on it and would be interested in hearing about any bugs found in the fuzzer.
Can you add erik.pilkington@gmail.com to your CC list?

May 24 2017, 2:30 PM
erik.pilkington added a comment to D33368: [libcxxabi][demangler] Fix a crash in the demangler.

r303806 removes the assertion (instead just returning first). I though this should never happen, I'm looking into this testcase to see if there is another bug here.
Thanks,
Erik

May 24 2017, 1:55 PM

May 23 2017

erik.pilkington added inline comments to D33368: [libcxxabi][demangler] Fix a crash in the demangler.
May 23 2017, 10:43 PM
erik.pilkington accepted D33450: Warn about uses of `@available` that can't suppress the -Wunguarded-availability warnings.

LGTM, thanks!

May 23 2017, 9:34 PM
erik.pilkington added a comment to D33393: [PATCH] Libcxxabi Demangler PR32890.

but having a constant like this ("7") sounds wrong. Why not 6, or 8, or 42?

7 is correct here because we inserting into a specific point into a string that was inserted into db.names just above this (but out of context from the diff). The only problem is that more stuff was inserted in the meantime.
This is horrible, but the demangler is littered with random offsets. My personal favorite is that const, volatile, and restrict aren't represented with an enum, but just with the literals 1, 2, and 4.

May 23 2017, 11:44 AM
erik.pilkington added inline comments to D33450: Warn about uses of `@available` that can't suppress the -Wunguarded-availability warnings.
May 23 2017, 10:50 AM
erik.pilkington added a comment to D33393: [PATCH] Libcxxabi Demangler PR32890.

You could append my test case as a quick-check.

Sure, I added the test case here to my patch and it works.

So, I'll go ahead and abandon this revision?

Guess so, sorry for the confusion here.

May 23 2017, 9:29 AM
erik.pilkington updated the diff for D33368: [libcxxabi][demangler] Fix a crash in the demangler.

In this new patch:

May 23 2017, 9:25 AM

May 22 2017

erik.pilkington added a comment to D33393: [PATCH] Libcxxabi Demangler PR32890.

Could you please add more context lines in any future patches? Makes it easier to review!

May 22 2017, 11:23 PM
erik.pilkington added a comment to D30837: [libcxx] Support for shared_ptr<T()>.

Ping!

May 22 2017, 4:37 PM
erik.pilkington added inline comments to D33368: [libcxxabi][demangler] Fix a crash in the demangler.
May 22 2017, 9:45 AM

May 19 2017

erik.pilkington updated the diff for D33049: [libcxx] Support for Objective-C++ tests.

This new patch checks to make sure ARC is enabled in the tests, thanks for the suggestion!

May 19 2017, 3:32 PM
erik.pilkington updated the diff for D33049: [libcxx] Support for Objective-C++ tests.

In this new patch:

  • Add support for toggling -fobjc-arc, this is done by using file extensions, ie: foo.arc.pass.mm instead of foo.pass.mm. (Thanks @dexonsmith for the suggestion!)
  • Clean up/simplify things a bit
May 19 2017, 3:05 PM
erik.pilkington created D33368: [libcxxabi][demangler] Fix a crash in the demangler.
May 19 2017, 2:15 PM
erik.pilkington updated the diff for D33250: [Sema][ObjC] Fix a bug where -Wunguarded-availability was emitted at the wrong location.

Can we ignore the TypeLocs with invalid location and instead look at ObjCPropertyRefExprs with a class receiver?

Sure, good idea. This new patch does exactly that.

May 19 2017, 1:06 PM

May 16 2017

erik.pilkington updated the diff for D33250: [Sema][ObjC] Fix a bug where -Wunguarded-availability was emitted at the wrong location.

Just noticed this can be simplified a bit, NFC compared to the last version of the diff.

May 16 2017, 8:46 PM
erik.pilkington created D33250: [Sema][ObjC] Fix a bug where -Wunguarded-availability was emitted at the wrong location.
May 16 2017, 11:17 AM

May 10 2017

erik.pilkington created D33049: [libcxx] Support for Objective-C++ tests.
May 10 2017, 9:24 AM

May 9 2017

erik.pilkington added a comment to D30837: [libcxx] Support for shared_ptr<T()>.

Ping! @mclow.lists: Do you have any thoughts here?

May 9 2017, 9:31 AM
erik.pilkington accepted D33000: Add support for pretty platform names to `@available`/`__builtin_available`.

LGTM, thanks for working on this!

May 9 2017, 8:07 AM

May 5 2017

erik.pilkington accepted D32424: Add a fix-it for -Wunguarded-availability.

LGTM, thanks!

May 5 2017, 8:46 AM

May 4 2017

erik.pilkington created D32891: [Sema][ObjC++] Objective-C++ support for __is_base_of(B, D).
May 4 2017, 6:51 PM
erik.pilkington added a comment to D32424: Add a fix-it for -Wunguarded-availability.

Hi Alex, thanks for working on this! This looks right, but I have a couple of comments.
Thanks,
Erik

May 4 2017, 9:18 AM

May 1 2017

erik.pilkington updated the diff for D30837: [libcxx] Support for shared_ptr<T()>.

Rebase n' ping!

May 1 2017, 11:19 AM

Apr 13 2017

erik.pilkington added inline comments to D30837: [libcxx] Support for shared_ptr<T()>.
Apr 13 2017, 2:14 PM
erik.pilkington updated the diff for D30837: [libcxx] Support for shared_ptr<T()>.

This new patch includes @EricWF's static_assert & test.
Thanks,
Erik

Apr 13 2017, 2:13 PM

Apr 11 2017

erik.pilkington added a comment to D30837: [libcxx] Support for shared_ptr<T()>.

Ping!

Apr 11 2017, 1:29 PM

Mar 30 2017

erik.pilkington updated the diff for D30837: [libcxx] Support for shared_ptr<T()>.

In this new patch, use an explicit specialization of std::allocator that specifically only performs a rebind. This needs to be a specialization of std::allocator so we can use allocator's allocator(const allocator<U> &) ctor from shared_ptr_pointer::__on_zero_shared_weak().
Thanks,
Erik

Mar 30 2017, 10:08 AM

Mar 23 2017

erik.pilkington added a comment to D30837: [libcxx] Support for shared_ptr<T()>.

Ping!

Mar 23 2017, 11:16 AM

Mar 14 2017

erik.pilkington added a comment to D30837: [libcxx] Support for shared_ptr<T()>.

We can't just use an arbitrary allocator type for a number of reasons:

  • You just changed the type of the control block. That's ABI breaking.

Ah, I didn't think of that. This new patch only selects allocator<int> when _Yp is a function type, which is only permitted as of this patch.

  • allocator<int> allocates ints, nothing else.

Since we only use allocator<int> for a rebind, we don't violate this rule.

  • It may mean we don't select a valid user specialization of allocator<Yp>.

In the new patch, allocator<int> is only selected if is_function<_Yp>, I don't believe that there is any valid user specialization of std::allocator<FunctionType>. This is because [namespace.std] p1:

A program may add a template specialization for any standard library template to namespace std only if [...] the specialization meets the standard library requirements for the original template [...]

In this case, the original template is the default allocator, which is supposed to have overloads of address taking a const_reference and a reference ([allocator.members] p2-3), which will be ambiguous for function types with the given definitions of const_reference and reference. Is this a valid argument?

Mar 14 2017, 5:45 AM
erik.pilkington updated the diff for D30837: [libcxx] Support for shared_ptr<T()>.

In this new patch:

  • We only select allocator<int> when _Yp is a function type, fixing the ABI break @EricWF pointed out.
  • Only try to select a different allocator if we're using the __shared_ptr_pointer layout; make_shared<void()> doesn't make sense.
  • Improve the testcase
Mar 14 2017, 5:45 AM

Mar 10 2017

erik.pilkington updated the diff for D30837: [libcxx] Support for shared_ptr<T()>.

This new patch replaces the allocator from allocator<void> to allocator<int>, I didn't realize allocator<void> was deprecated.
Thanks,
Erik

Mar 10 2017, 11:58 AM
erik.pilkington created D30837: [libcxx] Support for shared_ptr<T()>.
Mar 10 2017, 10:27 AM
erik.pilkington added a comment to D30802: [Builtin] Implement lit-test support (part 1 of 2: test cases update).

Just rebased and find that https://reviews.llvm.org/D30136 is adding TestCase support.
I'm not sure what's the TestCase is used for. Looks only for Darwin and the directory is empty.

Thanks for working on this!
I added TestCases as a workaround, once this lands I'll be more than happy to remove it and port the os_version_check_test I added to this infrastructure.

Mar 10 2017, 7:54 AM

Mar 9 2017

erik.pilkington added a comment to D30136: [compiler-rt][builtins][WIP] Add _IsOSVersionAtLeast, to be used by ObjC's @available.

@arphaman: Sure, works as intended on debian8 x86_64. Thanks for all your help here!

Mar 9 2017, 6:24 AM

Mar 4 2017

erik.pilkington updated the diff for D30136: [compiler-rt][builtins][WIP] Add _IsOSVersionAtLeast, to be used by ObjC's @available.

This new patch incorporates Alex Lorenz's testing infrastructure, and uses it to write a testcase.

Mar 4 2017, 11:32 AM

Feb 24 2017

erik.pilkington updated the diff for D30136: [compiler-rt][builtins][WIP] Add _IsOSVersionAtLeast, to be used by ObjC's @available.

Just realized I did a if (!&CFLongFunctionName) when I meant to do a if (&CFLongFunctionName). Oops!

Feb 24 2017, 1:44 PM
erik.pilkington updated the diff for D30136: [compiler-rt][builtins][WIP] Add _IsOSVersionAtLeast, to be used by ObjC's @available.

This new patch addresses Alex's comments:

  • Add more failure checks
  • Rename _IsOSVersionAtLeast -> __isOSVersionAtLeast

Additionally:

  • fall back to the function CFPropertyListCreateFromXMLData if CFPropertyListCreateWithData is unavailable.
Feb 24 2017, 12:56 PM

Feb 22 2017

erik.pilkington added inline comments to D27827: [ObjC] CodeGen support for @available on macOS.
Feb 22 2017, 12:36 PM

Feb 21 2017

erik.pilkington updated the diff for D27827: [ObjC] CodeGen support for @available on macOS.

This new patch addresses all of Alex's comments:

  • Remove ObjC from function names
  • Rename _IsOSVersionAtLeast -> __isOSVersionAtLeast
  • Improve testcase
Feb 21 2017, 5:27 PM
erik.pilkington added a comment to D30136: [compiler-rt][builtins][WIP] Add _IsOSVersionAtLeast, to be used by ObjC's @available.

@arphaman: AFAIK Gestalt() is only present on macOS (https://developer.apple.com/reference/coreservices/1471624-gestalt?language=objc), so for iOS and the like we'll need to fall back to parsing the SystemVersion.plist file. Is there any reason to prefer special casing macOS to use Gestalt?

Feb 21 2017, 1:00 PM

Feb 19 2017

erik.pilkington updated the diff for D27827: [ObjC] CodeGen support for @available on macOS.

This new patch just generates a call to the function _IsOSVersionAtLeast and branches on the result. _IsOSVersionAtLeast is going through review here: https://reviews.llvm.org/D30136.

Feb 19 2017, 8:35 AM

Feb 18 2017

erik.pilkington created D30136: [compiler-rt][builtins][WIP] Add _IsOSVersionAtLeast, to be used by ObjC's @available.
Feb 18 2017, 10:40 AM

Jan 14 2017

erik.pilkington added a comment to D28670: [ObjC] Disallow vector parameters and return values in Objective-C methods on older X86 targets.

Hi Alex, thanks for CCing me!

Jan 14 2017, 11:59 AM

Jan 4 2017

erik.pilkington abandoned D26893: [Sema] Fix assert on valid during template argument deduction.

@rsmith fixed this in r291064.

Jan 4 2017, 6:48 PM

Dec 20 2016

erik.pilkington added a comment to D24639: [Sema] Warn when returning a lambda that captures a local variable by reference.

Ping!

Dec 20 2016, 11:21 AM

Dec 16 2016

erik.pilkington added a comment to D27827: [ObjC] CodeGen support for @available on macOS.

I seem to remember that mapping from kernel versions to marketing versions was tossed around as a potential alternative to Gestalt. I think that the problem was Apple sometimes introduces (or reserves the right to introduce) new SDKs in a patch release (ie, Z in X.Y.Z), which wouldn't necessary imply a new kernel version, and would still need to be queried by @available. This makes it impossible to use kernel version in the general case (Or at least I think, it would be nice if someone internal to Apple could confirm this?). This rules out using sysctl() and the like.

Dec 16 2016, 1:37 PM

Dec 15 2016

erik.pilkington retitled D27827: [ObjC] CodeGen support for @available on macOS from to [ObjC] CodeGen support for @available on macOS.
Dec 15 2016, 1:25 PM

Dec 1 2016

erik.pilkington added a comment to D24639: [Sema] Warn when returning a lambda that captures a local variable by reference.

Ping!

Dec 1 2016, 9:29 AM

Nov 23 2016

erik.pilkington added a comment to D24639: [Sema] Warn when returning a lambda that captures a local variable by reference.

Ping!

Nov 23 2016, 11:28 AM

Nov 19 2016

erik.pilkington retitled D26893: [Sema] Fix assert on valid during template argument deduction from to [Sema] Fix assert on valid during template argument deduction.
Nov 19 2016, 9:43 PM

Nov 15 2016

erik.pilkington added inline comments to D26664: [ObjC] Prevent infinite loops when iterating over redeclaration of a method that was declared in an invalid interface.
Nov 15 2016, 9:30 AM

Nov 12 2016

erik.pilkington added inline comments to D24639: [Sema] Warn when returning a lambda that captures a local variable by reference.
Nov 12 2016, 5:31 AM
erik.pilkington updated the diff for D24639: [Sema] Warn when returning a lambda that captures a local variable by reference.

This new patch rewrites the check to just use the captures in the lambda's CXXRecordDecl, instead of searching through the returned expression to find the LambdaExpr.
Thanks!

Nov 12 2016, 5:25 AM