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

[Quuxplusone]

Bugzilla Link	PR45020
Status	NEW
Importance	P normal
Reported by	Sylvain Defresne (sdefresne@google.com)
Reported on	2020-02-25 02:37:23 -0800
Last modified on	2020-02-26 19:19:58 -0800
Version	trunk
Hardware	PC MacOS X
CC	htmldeveloper@gmail.com, llvm-bugs@lists.llvm.org, neeilans@live.com, nicolasweber@gmx.de, richard-llvm@metafoo.co.uk, rjmccall@apple.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also

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<typename T>
struct a {
  explicit constexpr a(__unsafe_unretained T o = nil) : o_(o) {}

  __unsafe_unretained T o_;
};

template<typename T>
struct b : a<T> {
  explicit constexpr b(T __attribute__((ns_consumed)) o = nil) : a<T>(o) {}
};

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

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

void foo() {
  b<id> _0([[NSObject alloc] init]);
  c<id> _1([[NSObject alloc] init]);
  c<NSObject> _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<id>(o) {}
                     ^
s.mm:23:73: note: subexpression not valid in a constant expression
  explicit constexpr c(id __attribute__((ns_consumed)) o = nil) : b<id>(o) {}
                                                                        ^
I would expect the "constexpr" to fail compilation for all of b<id>, c<id> and
c<NSObject> or for none of them. However, it only fails for c<id> 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<typename T>
struct a {
  explicit constexpr a(__unsafe_unretained T o = nil) : o_(o) {}

  __unsafe_unretained T o_;
};

template<typename T>
struct b : a<T> {
  explicit constexpr b(T __attribute__((ns_consumed)) o = nil) : a<T>(o) {}
};

template<typename T>
struct t {
  typedef T* tp;
};

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

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

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

[Quuxplusone]

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

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

[Quuxplusone]

(In reply to Nico Weber from comment #1)
> 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.

[Quuxplusone]

> 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?

[Quuxplusone]

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.

[Quuxplusone]

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.

[Quuxplusone]

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.