Unavailable function incorrectly marked as deleted from within a scoped marked as unavailable

[Quuxplusone]

Bugzilla Link	PR40995
Status	NEW
Importance	P enhancement
Reported by	Louis Dionne (ldionne@apple.com)
Reported on	2019-03-07 12:11:55 -0800
Last modified on	2019-03-14 17:48:05 -0700
Version	trunk
Hardware	PC All
CC	llvm-bugs@lists.llvm.org, neeilans@live.com, richard-llvm@metafoo.co.uk
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also

Functions marked as unavailable appear as being deleted even when from a scope that is unavailable too:

$ cat <<EOF | clang++ -xc++ - -fsyntax-only -std=c++11
#include <type_traits>

#define UNAVAILABLE __attribute__((availability(macosx,strict,introduced=99.99)))

struct Bar { UNAVAILABLE Bar(int, int) { } };

struct UNAVAILABLE Foo { 
    // this is performed in a scope marked as unavailable
    static_assert(std::is_constructible<Bar, int, int>::value, ""); // this fires
};
EOF

I think the static_assert fires because the code is equivalent to marking Bar(int,int) = delete, which would then fail the static_assert. However, since Bar(int, int) is used (in std::is_constructible) from a context that is transitively unavailable (because Foo is unavailable), I would expect Bar(int, int) not to be marked deleted in that context.

[Quuxplusone]

I should have included the output. The output is:

<stdin>:9:1: error: static_assert failed due to requirement
'std::is_constructible<Bar, int, int>::value' ""
static_assert(std::is_constructible<Bar, int, int>::value, ""); // this fires
^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.

[Quuxplusone]

Regrettably, I think the current behavior is probably the best we can do. The
meaning of a template instantiation cannot really depend on the context in
which one of the points of instantiation appeared. Consider:

  struct UNAVAILABLE Foo {
    static_assert(std::is_constructible<Bar, int, int>::value, "");
  };
  static_assert(std::is_constructible<Bar, int, int>::value, "");

Either both the static_asserts will fire, or neither of them will. Reordering
the namespace-scope one before the class should not affect the outcome. The
only coherent answer seems to be that is_constructible is evaluated in a
context unrelated to the point of instantiation for the purpose of availability
(just as it is for the purpose of access checks).

(Because is_constructible can be used to control the behavior of templates
involving Foo, it might affect, for instance, the triviality of a member of
some other type, and therefore affect the ABI. It would be really bad if said
ABI decision depended on whether the same is_constructible specialization were
previously instantiated from an unavailable context in the same translation
unit.)

[Quuxplusone]

For this specific problem, other approaches might work: we could suppress all static_assert failures within an unavailable context. (More broadly, we could skip the bodies of unavailable classes and functions entirely, but perhaps that's a step too far.)

[Quuxplusone]

Yes, I agree this is probably the best we can do. However, I question whether availability markup should cause the function to be marked as deleted. I would actually expect std::is_constructible<Bar, int, int>::value to be true all the time, but if you actually try to use the function (maybe ODR-use, IDK), then I'd expect an error saying "oops, you can't actually call this thing because it's not available on your target".

Does that make sense?

[Quuxplusone]

The availability markup doesn't cause the function to be considered deleted, it
causes it to be considered unavailable :)

Yes, I think what you're suggesting makes sense; in essence I think what we'd
be saying is that availability errors are suppressed in SFINAE contexts, and
don't even cause substitution failure. (Almost: odr-use of an unavailable
entity still needs a diagnostic, even in SFINAE contexts.) I think that
constitutes a design change to the availability mechanism and should probably
receive some discussion on cfe-dev, but it seems like it could be a reasonable
approach.

This is potentially a breaking change. For example:

UNAVAILABLE void f(int);
template<typename T> auto g(T t) -> decltype(f(t)) {
  return f(t);
}
void g(...);

Today, g(0) calls g(...), because substitution into g<int> fails. With the
proposed change, we'd select g<int> and then hit a hard error when it tries to
call the unavailable f(int).

[Quuxplusone]

I'm confused. It looks as if availability didn't change overload resolution:

    https://wandbox.org/permlink/gQ7yAxTcnq8BVtQV

But then your example does show that it's taken into account in dependent
contexts (or something):

    https://wandbox.org/permlink/51yZEfvkKgv2uBOd

What am I missing here? Also, IMO the behavior for availability should be
exactly what you describe, i.e. that it should select g<int> and then hard
error when it _actually_ tries to call f(int). The reason is that availability
is basically a compiler-level link error. It's just an attribute that allows us
to emit a better diagnostic earlier than at link time. As such, it shouldn't
change the meaning of the program.

Now, the above holds for the strict version of availability, but perhaps not
for the non-strict version of it, which (I think) turns the functions into weak
symbols (?). Those can then be checked at runtime, etc. This seems like a
horrible feature, however the non-strict version seems closer to SFINAE in
spirit.

[Quuxplusone]

As currently implemented, unavailable functions behave very much like deleted functions: it's an error to reference them, and that error can cause non-viability of an overload in a SFINAE context. They're not quite the same, though, because use of an unavailable function from an unavailable context is not an error, whereas use of a deleted function from an unavailable context is an error.

I think the model that we want is that odr-use of an unavailable function (that is, a use that might result in a reference to the symbol) from a non-unavailable context is a hard (non-SFINAE) error. I think that matches the idea that we're giving what would be a linker error during compilation, and that we don't allow availability to be detected by SFINAE. As noted in comment#5, though, that's a significant change in the design direction of the attribute in C++, so we should discuss it somewhere more visible than this bug before committing to it.