-Winvalid-constexpr fails with specialisation when interacting with automatic reference counting

[7f902d05-b075-416e-946c-ddba8c45e328]

Bugzilla Link	45020
Version	trunk
OS	MacOS X
CC	@nico,@zygoloid,@rjmccall

Extended Description

Specialisation of a template to support "id" (the Objective-C type) uniformly the same way as pointer to Objective-C object cause failure of "constexpr" compilation.

Small reproduction case:

=== s.mm

import <Foundation/Foundation.h>

template struct a { explicit constexpr a(__unsafeunretained T o = nil) : o(o) {}

__unsafeunretained T o; };

template struct b : a { explicit constexpr b(T attribute((ns_consumed)) o = nil) : a(o) {} };

template struct c : b<T> { explicit constexpr c(T attribute((ns_consumed)) o = nil) : b<T*>(o) {} };

template<> struct c : b { explicit constexpr c(id attribute((ns_consumed)) o = nil) : b(o) {} };

void foo() { b _0([[NSObject alloc] init]); c _1([[NSObject alloc] init]); c _2(([[NSObject alloc] init])); }

Compiling this with the following options "--std=c++14 -fobjc-arc" results in the following error:

s.mm:23:22: error: constexpr constructor never produces a constant expression [-Winvalid-constexpr] explicit constexpr c(id attribute((ns_consumed)) o = nil) : b(o) {} ^ s.mm:23:73: note: subexpression not valid in a constant expression explicit constexpr c(id attribute((ns_consumed)) o = nil) : b(o) {} ^ I would expect the "constexpr" to fail compilation for all of b, c and c or for none of them. However, it only fails for c which makes me think this is a bad interaction between specialisation, ARC (automatic reference counting) and the annotations.

The following alternative way to specialise via trait does not produce any error:

===t.mm

import <Foundation/Foundation.h>

template struct a { explicit constexpr a(__unsafeunretained T o = nil) : o(o) {}

__unsafeunretained T o; };

template struct b : a { explicit constexpr b(T attribute((ns_consumed)) o = nil) : a(o) {} };

template struct t { typedef T* tp; };

template<> struct t { typedef id tp; };

template struct c : b<typename t::tp> { typedef typename t::tp tp; explicit constexpr c(tp attribute((ns_consumed)) o = nil) : b(o) {} };

void foo() { b _0([[NSObject alloc] init]); c _1([[NSObject alloc] init]); c _2(([[NSObject alloc] init])); }

[rjmccall]

Okay, if this is being rejected in the abstract, we're probably incorrectly thinking that we can't perform a CK_ARCProduceObject (used when passing an argument to an ns_consumed parameter) as a constant expression, when in fact we can if the value is a constant literal.

[ec04fc15-fa35-46f2-80e1-5d271f2ef708]

Diagnosing constexpr functions that can never produce a constant expression is best-effort (in general this is a non-computable property, so the C++ standard allows implementations to diagnose as much or as little as they care to). In Clang we implement this by basically attempting a trial constant evaluation (skipping subexpressions we can't compute the values of), looking for things that are definitely reachable and definitely not evaluatable.

But our constant evaluator is (by design) only able to evaluate non-value-dependent portions of the AST -- the value-dependent pieces don't maintain the invariants necessary to support such evaluation, and are in general incomplete in various semantically-important ways -- so we don't do any checking for value-dependent expressions. That's why we're only able to diagnose the c case.

[rjmccall]

I think there needs to be an actual investigation into why it's working this way, and then we can figure out what to do about it.

[7f902d05-b075-416e-946c-ddba8c45e328]

Well, we legitimately can't do reference-counting operations at compile time. However, the huge caveat is that, if the value we want to retain/release is a static literal (isExpressibleAsConstantInitializer), we don't have to because those object are retain-agnostic.

I understand that. In fact, I would have expected all those constructors to be flagged as in error as they are all marked as "constexpr" and may all lead to reference counting.

What is surprising is that only one of the constructor is marked as inconsistent with "constexpr" when I would have expected all of them to be (for the reason you mentioned).

Maybe all C++ constructor that take a pointer to an instance of an Objective-C class should be marked as incompatible with "constexpr" (at least when ARC is enabled).

WDYT?

[rjmccall]

rjmccall, this is a belated follow-up to bug 27887.

Do you have an opinion on what clang's behavior should be here?

Well, we legitimately can't do reference-counting operations at compile time. However, the huge caveat is that, if the value we want to retain/release is a static literal (isExpressibleAsConstantInitializer), we don't have to because those object are retain-agnostic.

Probably the right way to handle this is more like how we handle UB in arithmetic: we should accept the expression in the abstract but reject its application to objects that aren't known to be retain-agnostic. I'm not sure if there's any situation in which we could get a non-retain-agnostic object in the constant evaluator in the first place, so that might be quite simple; but in case it isn't, I think that's the right rule to use.

We have an Apple-internal refactor that we should be able to upstream that makes testing isExpressibleAsConstantInitializer somewhat more regular.

[nico]

rjmccall, this is a belated follow-up to bug 27887.

Do you have an opinion on what clang's behavior should be here?

[llvmbot]

@llvm/issue-subscribers-clang-frontend