Page MenuHomePhabricator

erik.pilkington (Erik Pilkington)
User

Projects

User does not belong to any projects.

User Details

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

Recent Activity

Thu, Sep 19

erik.pilkington created D67775: [Sema] Split out -Wformat-type-confusion from -Wformat-pedantic .
Thu, Sep 19, 1:12 PM · Restricted Project

Wed, Sep 18

erik.pilkington committed rG5741d19f046f: [Sema] Suppress -Wformat diagnostics for bool types when printed using %hhd (authored by erik.pilkington).
[Sema] Suppress -Wformat diagnostics for bool types when printed using %hhd
Wed, Sep 18, 12:08 PM
erik.pilkington added a comment to D66856: [Sema] Suppress -Wformat diagnostics for bool types when printed using %hhd.

Ping!

Wed, Sep 18, 11:08 AM · Restricted Project, Restricted Project

Tue, Sep 17

erik.pilkington committed rG5c62152275c0: [Sema] Split of versions of -Wimplicit-{float,int}-conversion for Objective-C… (authored by erik.pilkington).
[Sema] Split of versions of -Wimplicit-{float,int}-conversion for Objective-C…
Tue, Sep 17, 2:12 PM
erik.pilkington added inline comments to D67559: [Sema] Split of versions of -Wimplicit-{float,int}-conversion for Objective-C BOOL.
Tue, Sep 17, 2:11 PM · Restricted Project, Restricted Project
erik.pilkington committed rGa1e29a3407fb: Use 'BOOL' instead of BOOL in diagnostic messages (authored by erik.pilkington).
Use 'BOOL' instead of BOOL in diagnostic messages
Tue, Sep 17, 11:03 AM
erik.pilkington added inline comments to D67559: [Sema] Split of versions of -Wimplicit-{float,int}-conversion for Objective-C BOOL.
Tue, Sep 17, 10:33 AM · Restricted Project, Restricted Project
erik.pilkington updated the diff for D67559: [Sema] Split of versions of -Wimplicit-{float,int}-conversion for Objective-C BOOL.

Address review comments.

Tue, Sep 17, 10:33 AM · Restricted Project, Restricted Project

Fri, Sep 13

erik.pilkington created D67559: [Sema] Split of versions of -Wimplicit-{float,int}-conversion for Objective-C BOOL.
Fri, Sep 13, 9:15 AM · Restricted Project, Restricted Project

Wed, Sep 11

erik.pilkington added a comment to D66856: [Sema] Suppress -Wformat diagnostics for bool types when printed using %hhd.

The reflector discussion is still happening and there are issues with ambiguities that we are pretty sure we want to correct. I've got a paper out that touches on some of this: http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2420.pdf

Wed, Sep 11, 2:58 PM · Restricted Project, Restricted Project
erik.pilkington updated the diff for D66856: [Sema] Suppress -Wformat diagnostics for bool types when printed using %hhd.

Update the diagnostic text.

Wed, Sep 11, 2:58 PM · Restricted Project, Restricted Project

Tue, Sep 10

erik.pilkington updated the diff for D66856: [Sema] Suppress -Wformat diagnostics for bool types when printed using %hhd.

Address review comments.

Tue, Sep 10, 1:44 PM · Restricted Project, Restricted Project

Mon, Sep 9

erik.pilkington added a comment to D66696: [ObjC generics] Fix not inheriting type bounds in categories/extensions..

I'm a bit curious about clients that use getCanonicalType() to get a full desugaring, instead of doing a single step. It seems like they'd still get the out of date type parameter type. Has that ever worked?

Mon, Sep 9, 2:24 PM

Wed, Aug 28

erik.pilkington committed rG6c7687ed6770: Fix a passing XFAIL test (authored by erik.pilkington).
Fix a passing XFAIL test
Wed, Aug 28, 3:38 PM
erik.pilkington added a comment to D66856: [Sema] Suppress -Wformat diagnostics for bool types when printed using %hhd.

I'm wondering whether this should even warn pedantically. There are no format specifiers that apply directly to a _Bool datatype, so the user is left making a choice as to what specifier fits best. I think hhd and hhu are both equally reasonable types, as are the d and u groups directly -- I don't think we should warn on either of those specifiers with a _Bool argument. I could maybe see a case for pedantically diagnosing h, l, and ll prefixes with _Bool because that seems a bit type-confused to me. c as a specifier seems weird (given that false values will potentially terminate the string suddenly depending on terminal behavior, IIRC), but lc seems like type confusion again.

Wed, Aug 28, 1:01 PM · Restricted Project, Restricted Project

Tue, Aug 27

erik.pilkington created D66856: [Sema] Suppress -Wformat diagnostics for bool types when printed using %hhd.
Tue, Aug 27, 6:20 PM · Restricted Project, Restricted Project

Aug 22 2019

erik.pilkington accepted D66622: [Bugfix] fix r369705 unit test.

LGTM, thanks for fixing this!

Aug 22 2019, 3:49 PM · Restricted Project, Restricted Project

Aug 14 2019

erik.pilkington added a comment to D65022: [Sema] Always instantiate the initializer of a variable template with undeduced type (8.0 regression).

Hmm, looks like this patch fails to fix the case where we're referencing the variable in an non-odr use context, since we don't even bother to instantiate the initializer here:

Aug 14 2019, 11:17 AM · Restricted Project
erik.pilkington committed rGaa3855694ff4: [Sema][ObjC] Fix a -Wformat false positive with localizedStringForKey (authored by erik.pilkington).
[Sema][ObjC] Fix a -Wformat false positive with localizedStringForKey
Aug 14 2019, 10:00 AM

Aug 13 2019

erik.pilkington updated the diff for D27165: Add format_dynamic_key_arg attribute to improve "-Wformat" warnings for functions that load the formatting string dynamically based on a key value.

Address review comments.

Aug 13 2019, 4:40 PM · Restricted Project
erik.pilkington committed rG6c97f8898685: Add a missing header comment, NFC (authored by erik.pilkington).
Add a missing header comment, NFC
Aug 13 2019, 3:05 PM
erik.pilkington edited reviewers for D27165: Add format_dynamic_key_arg attribute to improve "-Wformat" warnings for functions that load the formatting string dynamically based on a key value, added: rjmccall; removed: manmanren.
Aug 13 2019, 2:41 PM · Restricted Project
erik.pilkington updated the diff for D27165: Add format_dynamic_key_arg attribute to improve "-Wformat" warnings for functions that load the formatting string dynamically based on a key value.

Special case localizedStringForKey instead of using an attribute.

Aug 13 2019, 2:41 PM · Restricted Project
erik.pilkington commandeered D27165: Add format_dynamic_key_arg attribute to improve "-Wformat" warnings for functions that load the formatting string dynamically based on a key value.

Stealing this (with Alex's permission). I think it makes more sense to add a special case for the method -[NSBundle localizedStringForKey:value:table:] rather then add a new general purpose attribute that'll only be used in one place.

Aug 13 2019, 2:41 PM · Restricted Project

Aug 12 2019

erik.pilkington committed rG055fcec78cf6: [Sema] Check __builtin_bit_cast operand for completeness before materializing… (authored by erik.pilkington).
[Sema] Check __builtin_bit_cast operand for completeness before materializing…
Aug 12 2019, 12:30 PM
erik.pilkington committed rG0a223d981e6d: [Sema] Require a complete type for __builtin_bit_cast operands (authored by erik.pilkington).
[Sema] Require a complete type for __builtin_bit_cast operands
Aug 12 2019, 11:32 AM

Aug 2 2019

erik.pilkington committed rG06cccc5e6f7a: Remove a dead diagnostic, NFC (authored by erik.pilkington).
Remove a dead diagnostic, NFC
Aug 2 2019, 12:27 PM
erik.pilkington created D65665: Support for attributes on @class declarations.
Aug 2 2019, 10:51 AM · Restricted Project

Jul 30 2019

erik.pilkington updated the diff for D65022: [Sema] Always instantiate the initializer of a variable template with undeduced type (8.0 regression).

Rebase on top of r367367.

Jul 30 2019, 4:53 PM · Restricted Project
erik.pilkington committed rG4cae092099df: [Sema] Actually map a variable template specialization from pattern to… (authored by erik.pilkington).
[Sema] Actually map a variable template specialization from pattern to…
Jul 30 2019, 4:39 PM
erik.pilkington committed rGbe19c48f6d6b: [Parser] Lambda capture lists can start with '*' (authored by erik.pilkington).
[Parser] Lambda capture lists can start with '*'
Jul 30 2019, 12:23 PM

Jul 26 2019

erik.pilkington created D65359: [Sema] Map from a variable template specialization in a pattern to a variable template specialization in an instantiation properly .
Jul 26 2019, 5:03 PM · Restricted Project
erik.pilkington updated the diff for D65022: [Sema] Always instantiate the initializer of a variable template with undeduced type (8.0 regression).

Fix the type of the MemberExpr/DeclRefExpr after instantiating the variable initializer.

Jul 26 2019, 4:55 PM · Restricted Project

Jul 19 2019

erik.pilkington created D65022: [Sema] Always instantiate the initializer of a variable template with undeduced type (8.0 regression).
Jul 19 2019, 2:50 PM · Restricted Project

Jul 12 2019

erik.pilkington added inline comments to D64600: [ObjC] Add an attribute that "clamps" signed char BOOLs to {0,1}.
Jul 12 2019, 2:46 PM · Restricted Project
erik.pilkington updated the diff for D64600: [ObjC] Add an attribute that "clamps" signed char BOOLs to {0,1}.

Address review comments.

Jul 12 2019, 2:46 PM · Restricted Project

Jul 11 2019

erik.pilkington created D64600: [ObjC] Add an attribute that "clamps" signed char BOOLs to {0,1}.
Jul 11 2019, 3:12 PM · Restricted Project

Jul 9 2019

erik.pilkington committed rGabffae3a563d: [ObjC] Add a warning for implicit conversions of a constant non-boolean value… (authored by erik.pilkington).
[ObjC] Add a warning for implicit conversions of a constant non-boolean value…
Jul 9 2019, 10:32 AM

Jul 8 2019

erik.pilkington updated the diff for D63912: [ObjC] Add a warning for implicit conversions of a constant non-boolean value to BOOL.

Address review comments.

Jul 8 2019, 5:12 PM · Restricted Project, Restricted Project
erik.pilkington added inline comments to D63912: [ObjC] Add a warning for implicit conversions of a constant non-boolean value to BOOL.
Jul 8 2019, 5:12 PM · Restricted Project, Restricted Project
erik.pilkington committed rGfa591c370d24: [ObjC] Add a -Wtautological-compare warning for BOOL (authored by erik.pilkington).
[ObjC] Add a -Wtautological-compare warning for BOOL
Jul 8 2019, 4:46 PM
erik.pilkington added a comment to D63856: [ObjC] Add a -Wtautological-compare warning for BOOL.

This only applies to relational operators, right? I'm a little uncomfortable with calling this "tautological" since it's not like it's *undefined behavior* to have (BOOL) 2, it's just *unwise*. But as long as we aren't warning about reasonable idioms that are intended to handle unfortunate situations — like other code that might have left a non-{0,1} value in the BOOL — I think this is fine.

I think the party line is that it is undefined behaviour (in some sense), since UBSan will happily crash if you try to load a non-boolean value from a BOOL.

What? Since when?

https://reviews.llvm.org/D27607

Interesting; I'm not sure I find that convincing. Probably the least convincing part is where it links to its own behavioral documentation as justification for doing what it does. But okay, I guess we do this.

Jul 8 2019, 4:46 PM · Restricted Project, Restricted Project
erik.pilkington added a comment to D63912: [ObjC] Add a warning for implicit conversions of a constant non-boolean value to BOOL.

Ping!

Jul 8 2019, 1:25 PM · Restricted Project, Restricted Project
erik.pilkington added a comment to D63856: [ObjC] Add a -Wtautological-compare warning for BOOL.

Ping!

Jul 8 2019, 1:25 PM · Restricted Project, Restricted Project

Jul 2 2019

erik.pilkington committed rGeee944e7f9e6: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast (authored by erik.pilkington).
[C++2a] Add __builtin_bit_cast, used to implement std::bit_cast
Jul 2 2019, 11:29 AM

Jun 28 2019

erik.pilkington added a comment to D63954: Add lifetime categories attributes.

We explicitly allow to add an annotation after
the definition of the class to allow adding annotations
to external source from by the user, e.g.

#include <vector>

namespace std {
template<typename T, typename Alloc>
class [[gsl::Owner(T)]] vector;
}
Jun 28 2019, 2:20 PM · Restricted Project, Restricted Project
erik.pilkington committed rG9a6cef74d8a9: [demangle] Support for C++2a char8_t (authored by erik.pilkington).
[demangle] Support for C++2a char8_t
Jun 28 2019, 12:57 PM

Jun 27 2019

erik.pilkington added a reviewer for D63912: [ObjC] Add a warning for implicit conversions of a constant non-boolean value to BOOL: arphaman.
Jun 27 2019, 5:31 PM · Restricted Project, Restricted Project
erik.pilkington created D63912: [ObjC] Add a warning for implicit conversions of a constant non-boolean value to BOOL.
Jun 27 2019, 5:31 PM · Restricted Project, Restricted Project
erik.pilkington added a comment to D63856: [ObjC] Add a -Wtautological-compare warning for BOOL.

This only applies to relational operators, right? I'm a little uncomfortable with calling this "tautological" since it's not like it's *undefined behavior* to have (BOOL) 2, it's just *unwise*. But as long as we aren't warning about reasonable idioms that are intended to handle unfortunate situations — like other code that might have left a non-{0,1} value in the BOOL — I think this is fine.

I think the party line is that it is undefined behaviour (in some sense), since UBSan will happily crash if you try to load a non-boolean value from a BOOL.

What? Since when?

Jun 27 2019, 11:13 AM · Restricted Project, Restricted Project

Jun 26 2019

erik.pilkington added a comment to D63856: [ObjC] Add a -Wtautological-compare warning for BOOL.

This only applies to relational operators, right? I'm a little uncomfortable with calling this "tautological" since it's not like it's *undefined behavior* to have (BOOL) 2, it's just *unwise*. But as long as we aren't warning about reasonable idioms that are intended to handle unfortunate situations — like other code that might have left a non-{0,1} value in the BOOL — I think this is fine.

Jun 26 2019, 11:31 PM · Restricted Project, Restricted Project
erik.pilkington created D63856: [ObjC] Add a -Wtautological-compare warning for BOOL.
Jun 26 2019, 5:28 PM · Restricted Project, Restricted Project
erik.pilkington committed rGd7999cbc6eb5: [ObjC] Improve error message for a malformed objc-type-name (authored by erik.pilkington).
[ObjC] Improve error message for a malformed objc-type-name
Jun 26 2019, 4:40 PM

Jun 24 2019

erik.pilkington updated the diff for D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast.

Address review comments.

Jun 24 2019, 2:47 PM · Restricted Project, Restricted Project
erik.pilkington added inline comments to D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast.
Jun 24 2019, 2:47 PM · Restricted Project, Restricted Project

Jun 19 2019

erik.pilkington added inline comments to D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast.
Jun 19 2019, 4:54 PM · Restricted Project, Restricted Project
erik.pilkington updated the diff for D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast.

Address review comments, thanks!

Jun 19 2019, 4:54 PM · Restricted Project, Restricted Project
erik.pilkington added inline comments to D62960: Add SVE opaque built-in types.
Jun 19 2019, 11:04 AM · Restricted Project, Restricted Project

Jun 18 2019

erik.pilkington committed rGcf8c6cfcdc82: [demangle] Special case clang's creative mangling of __uuidof expressions. (authored by erik.pilkington).
[demangle] Special case clang's creative mangling of __uuidof expressions.
Jun 18 2019, 4:32 PM

Jun 13 2019

erik.pilkington added a comment to D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast.

Can we also use the same bit reader/writer implementation to generalize the current implementation of __builtin_memcpy and friends? (I don't think we can remove the existing implementation, sadly, since we allow copying arrays of the same trivially-copyable type today even if it contains pointers and unions and such.)

Jun 13 2019, 12:43 PM · Restricted Project, Restricted Project
erik.pilkington updated the diff for D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast.

Address review comments.

Jun 13 2019, 12:43 PM · Restricted Project, Restricted Project

Jun 11 2019

erik.pilkington created D63176: [SemaObjC] Infer availability of stuff declared in categories from the availability of the enclosing category.
Jun 11 2019, 5:03 PM · Restricted Project

Jun 10 2019

erik.pilkington committed rG65831d049964: [demangle] Vendor extended types shouldn't be considered substitution candidates (authored by erik.pilkington).
[demangle] Vendor extended types shouldn't be considered substitution candidates
Jun 10 2019, 2:03 PM
erik.pilkington added inline comments to D62960: Add SVE opaque built-in types.
Jun 10 2019, 10:31 AM · Restricted Project, Restricted Project

Jun 5 2019

erik.pilkington added a comment to D62831: [CodeGen][ObjC] Add attribute "objc_arc_intert" to ObjC globals that are retain-agnostic.

I think this looks good from a clang perspective. No reason to land it before the dust settles on D62433 though.

Jun 5 2019, 2:41 PM · Restricted Project, Restricted Project
erik.pilkington added inline comments to D62831: [CodeGen][ObjC] Add attribute "objc_arc_intert" to ObjC globals that are retain-agnostic.
Jun 5 2019, 11:16 AM · Restricted Project, Restricted Project

Jun 3 2019

erik.pilkington accepted D62643: [CodeGen][ObjC] Convert '[self alloc]' in a class method to 'objc_alloc(self)'.

I thinks this looks good, aside from a null concern.

Jun 3 2019, 5:09 PM · Restricted Project, Restricted Project
erik.pilkington created D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast.
Jun 3 2019, 3:11 PM · Restricted Project, Restricted Project

May 31 2019

erik.pilkington committed rGabb2a93c5327: [SimplifyLibCalls] Fold more fortified functions into non-fortified variants (authored by erik.pilkington).
[SimplifyLibCalls] Fold more fortified functions into non-fortified variants
May 31 2019, 3:39 PM
erik.pilkington committed rG5234921119f9: NFC: Pull out a function to reduce some duplication (authored by erik.pilkington).
NFC: Pull out a function to reduce some duplication
May 31 2019, 3:39 PM
erik.pilkington added inline comments to D62358: [SimplifyLibCalls] Fold more fortified builtin functions into their non-fortified variants when possible.
May 31 2019, 3:39 PM · Restricted Project

May 30 2019

erik.pilkington added a comment to D62358: [SimplifyLibCalls] Fold more fortified builtin functions into their non-fortified variants when possible.

Ping!

May 30 2019, 12:43 PM · Restricted Project

May 23 2019

erik.pilkington created D62358: [SimplifyLibCalls] Fold more fortified builtin functions into their non-fortified variants when possible.
May 23 2019, 5:12 PM · Restricted Project

May 15 2019

erik.pilkington added a reviewer for D61722: [AST] Add RecoveryExpr; produce it on overload resolution failure and missing member.: erik.pilkington.
May 15 2019, 4:46 PM · Restricted Project
erik.pilkington committed rGf6c645f9fd93: [CodeGenObjC] invoke objc_autorelease, objc_retain when necessary (authored by erik.pilkington).
[CodeGenObjC] invoke objc_autorelease, objc_retain when necessary
May 15 2019, 1:14 PM
erik.pilkington created D61957: [CodeGenObjC] invoke objc_autorelease, objc_retain when necessary.
May 15 2019, 11:11 AM · Restricted Project

May 10 2019

erik.pilkington accepted D61803: [CodeGen][ObjC] Emit invoke instead of call to call `objc_release` when necessary..

LGTM, thanks!

May 10 2019, 2:12 PM · Restricted Project, Restricted Project
erik.pilkington added inline comments to D61803: [CodeGen][ObjC] Emit invoke instead of call to call `objc_release` when necessary..
May 10 2019, 1:50 PM · Restricted Project, Restricted Project
erik.pilkington committed rGf8ccf052935a: [Sema] Mark array element destructors referenced during initialization (authored by erik.pilkington).
[Sema] Mark array element destructors referenced during initialization
May 10 2019, 10:51 AM

May 9 2019

erik.pilkington updated the diff for D61165: Fix a crash where a [[no_destroy]] destructor was not emitted in an array.

Rename hasAccessibleDestructor.

May 9 2019, 4:49 PM · Restricted Project, Restricted Project
erik.pilkington added inline comments to D61165: Fix a crash where a [[no_destroy]] destructor was not emitted in an array.
May 9 2019, 4:49 PM · Restricted Project, Restricted Project
erik.pilkington added inline comments to D61165: Fix a crash where a [[no_destroy]] destructor was not emitted in an array.
May 9 2019, 4:14 PM · Restricted Project, Restricted Project
erik.pilkington added inline comments to D61165: Fix a crash where a [[no_destroy]] destructor was not emitted in an array.
May 9 2019, 3:08 PM · Restricted Project, Restricted Project
erik.pilkington updated the diff for D61165: Fix a crash where a [[no_destroy]] destructor was not emitted in an array.

Address review comments. Also remove the special case where we wouldn't check a destructor for an array in -fno-exceptions mode. This seems inconsistent with both how we're treating -fno-exceptions in general, and inconsistent with other cases of aggregate initialization (i.e. we still check struct members here).

May 9 2019, 3:08 PM · Restricted Project, Restricted Project

May 6 2019

erik.pilkington added a comment to D61165: Fix a crash where a [[no_destroy]] destructor was not emitted in an array.

Duncan pointed out eel.is/c++draft/class.init#class.base.init-13, which appears to claim that initialization ends with the execution of the constructor, excluding temporary destructors.

May 6 2019, 3:37 PM · Restricted Project, Restricted Project

May 3 2019

erik.pilkington added a comment to D61165: Fix a crash where a [[no_destroy]] destructor was not emitted in an array.

The flip side of that argument is that (1) there aren't very many users right now and (2) it's much easier to start conservative and then weaken the rule than it will be to strengthen it later. It really isn't acceptable to just turn off access/use-checking for the destructor, so if we get trapped by the choice we make here, we'll end up having to either leak or call std::terminate.

May 3 2019, 5:06 PM · Restricted Project, Restricted Project
erik.pilkington added a comment to D61165: Fix a crash where a [[no_destroy]] destructor was not emitted in an array.

I think the intuitive rule is that initialization is complete when the full-expression performing the initialization is complete because that's the normal unit of sequencing. Note that the standard does use both "constructed" and "initialized" in different contexts, although this might just be an editorial choice.

May 3 2019, 2:01 PM · Restricted Project, Restricted Project
erik.pilkington added a comment to D61165: Fix a crash where a [[no_destroy]] destructor was not emitted in an array.

It seems like the most common sense interpretation here is to just treat the initialization of G as completed at the point when the constructor finishes (this appears to be what GCC implements: https://wandbox.org/permlink/R3C9pPhoT4efAdL1).

Your example doesn't actually demonstrate that that's what GCC implements because it doesn't try to execute the declaration twice. But you're right, GCC does appear to consider the initialization complete as soon as the static object's constructor returns normally. On the other hand, GCC gets the array case here wrong: if a static local has array type, and a destructor for a temporary required by the first element initializer throws, then it does not destroy the element but also (correctly) does not mark the variable as fully initialized, and so a second attempt to run the initializer will simply construct a new object on top of an already-constructed one. This is arguably correct under the standard — the first array element is not a previously-constructed object of automatic duration — but I hope it's obvious that that's a defect.

So if it was static it would just get destroyed at exit-time, and therefore should be disable-able with no_destroy. If the standard implies that we should be doing something else, then we should do that, but I can't seem to find any reference to the rule you're describing.

Like I said, this is a poorly-specified part of the standard, because at least *some* objects with static storage duration have to be destroyed when an exception is thrown (because of aggregate initialization), but the standard wants to pretend that this isn't true. I think that allowing temporary destructors to cancel initialization uniformly across all kinds of objects is the most consistent rule,

That's only true for subobjects of an enclosing aggregate before that aggregate's initialization is complete though, right? So it doesn't seem like that much of an inconsistency, just mimicking what we would be doing if an exception was thrown in, say, the body of the ctor before the object's initialization is completed.

Conceptually yes, but formally no. The standard *could* write this rule as "all currently-initialized subobjects must be separately destroyed when an exception aborts initialization of the containing aggregate, including constructor bodies and aggregate initialization", but it doesn't actually do so; instead it has specific rules covering the behavior when an exception is thrown out of the body of a constructor, and those rules simply do not apply to aggregate initialization.

May 3 2019, 9:49 AM · Restricted Project, Restricted Project

May 2 2019

erik.pilkington added a comment to D61165: Fix a crash where a [[no_destroy]] destructor was not emitted in an array.

It seems like the most common sense interpretation here is to just treat the initialization of G as completed at the point when the constructor finishes (this appears to be what GCC implements: https://wandbox.org/permlink/R3C9pPhoT4efAdL1).

Your example doesn't actually demonstrate that that's what GCC implements because it doesn't try to execute the declaration twice. But you're right, GCC does appear to consider the initialization complete as soon as the static object's constructor returns normally. On the other hand, GCC gets the array case here wrong: if a static local has array type, and a destructor for a temporary required by the first element initializer throws, then it does not destroy the element but also (correctly) does not mark the variable as fully initialized, and so a second attempt to run the initializer will simply construct a new object on top of an already-constructed one. This is arguably correct under the standard — the first array element is not a previously-constructed object of automatic duration — but I hope it's obvious that that's a defect.

So if it was static it would just get destroyed at exit-time, and therefore should be disable-able with no_destroy. If the standard implies that we should be doing something else, then we should do that, but I can't seem to find any reference to the rule you're describing.

Like I said, this is a poorly-specified part of the standard, because at least *some* objects with static storage duration have to be destroyed when an exception is thrown (because of aggregate initialization), but the standard wants to pretend that this isn't true. I think that allowing temporary destructors to cancel initialization uniformly across all kinds of objects is the most consistent rule,

May 2 2019, 1:49 PM · Restricted Project, Restricted Project

May 1 2019

erik.pilkington added a comment to D61165: Fix a crash where a [[no_destroy]] destructor was not emitted in an array.

Hmm. You know, there's another case where the destructor can be called even for a non-array: if constructing the object requires a temporary, I believe an exception thrown from that temporary's destructor is supposed to go back and destroy the variable. (This is, sadly, not entirely clear under the standard since the object is not automatic.) Now, Clang does not actually get this correct — we abort the construction, but we don't destroy the variable — but (1) we should fix that someday and (2) in the meantime, we shouldn't implement something which will be a problem when we go to fix that.

May 1 2019, 4:53 PM · Restricted Project, Restricted Project

Apr 30 2019

erik.pilkington updated the diff for D61165: Fix a crash where a [[no_destroy]] destructor was not emitted in an array.

Add a try/catch block to the docs.

Apr 30 2019, 4:04 PM · Restricted Project, Restricted Project
erik.pilkington accepted D61147: [Sema][ObjC] Disable -Wunused-parameter for ObjC methods.

LGTM, thanks!

Apr 30 2019, 3:58 PM · Restricted Project, Restricted Project
erik.pilkington updated the diff for D61165: Fix a crash where a [[no_destroy]] destructor was not emitted in an array.

Address review comments.

Apr 30 2019, 3:06 PM · Restricted Project, Restricted Project
erik.pilkington accepted D61280: Variable auto-init: don't initialize aggregate padding of all aggregates.

LGTM, I think this makes sense.

Apr 30 2019, 1:36 PM · Restricted Project

Apr 26 2019

erik.pilkington added a comment to D52521: [Sema] DR727: Ensure we pick up enclosing template instantiation arguments for a class-scope explicit specialization..

No worries! It happens, I probably should have pinged this more aggressively.

Apr 26 2019, 3:34 PM
erik.pilkington planned changes to D61165: Fix a crash where a [[no_destroy]] destructor was not emitted in an array.

I believe at least one of the goals of nodestroy is to allow the type to potentially not provide a destructor at all, so if we're going to implicitly require the destructor anyway in certain situations, we should clearly document that, and we should be aware that we may be making the attribute less useful.

Since I believe the dominant use-case here is a true global, does only requiring the destructor for arrays in the static-local case when exceptions are enabled at least make it acceptable to do proper access checking, or is that still a source-compatibility problem for existing clients?

Apr 26 2019, 2:54 PM · Restricted Project, Restricted Project
erik.pilkington added a comment to D61165: Fix a crash where a [[no_destroy]] destructor was not emitted in an array.

By using no_destroy, you're saying that exit-time destructor doesn't matter because the OS will either reclaim the resources automatically, or its just doesn't matter since the process is going down. I don't think that implies that we can safely ignore the destructor in the middle of the program's execution.

Apr 26 2019, 2:31 PM · Restricted Project, Restricted Project
erik.pilkington added a comment to D61165: Fix a crash where a [[no_destroy]] destructor was not emitted in an array.

JF, Michael, and I were talking about this offline, and we think that the right choice of semantics for the static local case is to call the destructors.

Apr 26 2019, 2:29 PM · Restricted Project, Restricted Project
erik.pilkington added a comment to D61147: [Sema][ObjC] Disable -Wunused-parameter for ObjC methods.

I do not think the ObjC version of this diagnostic is useful as it's been explained here, and I would strongly recommend just not including such a warning for the time being.

Why? It seems to me like the vast majority of methods only declared in an @implementation are used as local helpers (even if people probably should be using functions for this), where this warning would be useful. Maybe I'm missing something? Still trying to learn more about Objective-C.

What rule are you suggesting for whether a method is "only declared in an @implementation"? Where exactly are you proposing to look for other declarations?

Objective-C does not have a formalized notion of an @implementation-private method, or really even an unambiguous convention for them (underscoring is *library*-private by convention, not class-private). The Objective-C ecosystem includes a lot of informal protocols and conventions based around discovery via naming, and Objective-C programmers do (admittedly rarely) just override methods that they know are there but which they can't see. These things make me very worried about trying to port assumptions from other languages.

Apr 26 2019, 1:47 PM · Restricted Project, Restricted Project

Apr 25 2019

erik.pilkington added a comment to D61147: [Sema][ObjC] Disable -Wunused-parameter for ObjC methods.

I do not think the ObjC version of this diagnostic is useful as it's been explained here, and I would strongly recommend just not including such a warning for the time being.

Apr 25 2019, 10:27 PM · Restricted Project, Restricted Project